Skip to content

Commit

Permalink
fix: Set size to a default value as well as timeout. (#839)
Browse files Browse the repository at this point in the history
* fix: Set size to a default value as well as timeout.

Currently, we are unable to run our `write_source_files` tests in our pre-upload checks, because we have `--test_size_filter=small`, and setting `size` will attempt to set it on both the run rule and the test rule, the former being invalid.

* code review feedback

* chore: fix one more test that should use size for defaulting

---------

Co-authored-by: Alex Eagle <alex@aspect.dev>
  • Loading branch information
matts1 and alexeagle authored Jul 19, 2024
1 parent db5556d commit 59453e5
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 18 deletions.
5 changes: 2 additions & 3 deletions docs/diff_test.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions docs/testing.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions lib/private/diff_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ The rule uses a Bash command (diff) on Linux/macOS/non-Windows, and a cmd.exe
command (fc.exe) on Windows (no Bash is required).
"""

load("//lib:utils.bzl", "default_timeout")
load(":directory_path.bzl", "DirectoryPathInfo")

def _runfiles_path(f):
Expand Down Expand Up @@ -107,7 +106,7 @@ _diff_test = rule(
implementation = _diff_test_impl,
)

def diff_test(name, file1, file2, size = None, timeout = None, **kwargs):
def diff_test(name, file1, file2, size = "small", **kwargs):
"""A test that compares two files.
The test succeeds if the files' contents match.
Expand All @@ -117,15 +116,12 @@ def diff_test(name, file1, file2, size = None, timeout = None, **kwargs):
file1: Label of the file to compare to <code>file2</code>.
file2: Label of the file to compare to <code>file1</code>.
size: standard attribute for tests
timeout: standard attribute for tests. Defaults to "short" if both timeout and size are unspecified.
**kwargs: The <a href="https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes-tests">common attributes for tests</a>.
"""

_diff_test(
name = name,
file1 = file1,
file2 = file2,
size = size,
timeout = default_timeout(size, timeout),
**kwargs
)
1 change: 1 addition & 0 deletions lib/private/write_source_file.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ To create an update *only* this file, run:
message = message,
visibility = kwargs.get("visibility"),
tags = kwargs.get("tags"),
size = "small",
)
else:
if suggested_update_target == None:
Expand Down
7 changes: 2 additions & 5 deletions lib/testing.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ load("@bazel_skylib//rules:write_file.bzl", "write_file")
load("//lib:diff_test.bzl", "diff_test")
load("//lib:jq.bzl", "jq")
load("//lib:params_file.bzl", "params_file")
load("//lib:utils.bzl", "default_timeout")

def assert_contains(name, actual, expected, size = None, timeout = None, **kwargs):
def assert_contains(name, actual, expected, size = "small", **kwargs):
"""Generates a test target which fails if the file doesn't contain the string.
Depends on bash, as it creates an sh_test target.
Expand All @@ -16,8 +15,7 @@ def assert_contains(name, actual, expected, size = None, timeout = None, **kwarg
name: target to create
actual: Label of a file
expected: a string which should appear in the file
size: the size attribute of the test target
timeout: the timeout attribute of the test target
size: standard attribute for tests
**kwargs: additional named arguments for the resulting sh_test
"""

Expand Down Expand Up @@ -45,7 +43,6 @@ def assert_contains(name, actual, expected, size = None, timeout = None, **kwarg
srcs = [test_sh],
args = ["$(rootpath %s)" % expected_file, "$(rootpath %s)" % actual],
size = size,
timeout = default_timeout(size, timeout),
data = [actual, expected_file],
**kwargs
)
Expand Down
4 changes: 2 additions & 2 deletions lib/tests/write_source_files/write_source_file_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ _write_source_file_test = rule(
test = True,
)

def write_source_file_test(name, in_file, out_file, check_that_out_file_exists = True):
def write_source_file_test(name, in_file, out_file, check_that_out_file_exists = True, size = "small"):
"""Stamp a write_source_files executable and a test to run against it"""

_write_source_file(
Expand All @@ -190,5 +190,5 @@ def write_source_file_test(name, in_file, out_file, check_that_out_file_exists =
write_source_file_target = name + "_updater",
in_file = in_file,
out_file = out_file,
timeout = "short",
size = size,
)

0 comments on commit 59453e5

Please sign in to comment.