fix: Set size to a default value as well as timeout. (#839)
* 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>
diff --git a/docs/diff_test.md b/docs/diff_test.md
index de63d9d..c4038fb 100644
--- a/docs/diff_test.md
+++ b/docs/diff_test.md
@@ -14,7 +14,7 @@
## diff_test
<pre>
-diff_test(<a href="#diff_test-name">name</a>, <a href="#diff_test-file1">file1</a>, <a href="#diff_test-file2">file2</a>, <a href="#diff_test-size">size</a>, <a href="#diff_test-timeout">timeout</a>, <a href="#diff_test-kwargs">kwargs</a>)
+diff_test(<a href="#diff_test-name">name</a>, <a href="#diff_test-file1">file1</a>, <a href="#diff_test-file2">file2</a>, <a href="#diff_test-size">size</a>, <a href="#diff_test-kwargs">kwargs</a>)
</pre>
A test that compares two files.
@@ -30,8 +30,7 @@
| <a id="diff_test-name"></a>name | The name of the test rule. | none |
| <a id="diff_test-file1"></a>file1 | Label of the file to compare to <code>file2</code>. | none |
| <a id="diff_test-file2"></a>file2 | Label of the file to compare to <code>file1</code>. | none |
-| <a id="diff_test-size"></a>size | standard attribute for tests | <code>None</code> |
-| <a id="diff_test-timeout"></a>timeout | standard attribute for tests. Defaults to "short" if both timeout and size are unspecified. | <code>None</code> |
+| <a id="diff_test-size"></a>size | standard attribute for tests | <code>"small"</code> |
| <a id="diff_test-kwargs"></a>kwargs | The <a href="https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes-tests">common attributes for tests</a>. | none |
diff --git a/docs/testing.md b/docs/testing.md
index e2c5436..74a3142 100644
--- a/docs/testing.md
+++ b/docs/testing.md
@@ -29,7 +29,7 @@
## assert_contains
<pre>
-assert_contains(<a href="#assert_contains-name">name</a>, <a href="#assert_contains-actual">actual</a>, <a href="#assert_contains-expected">expected</a>, <a href="#assert_contains-size">size</a>, <a href="#assert_contains-timeout">timeout</a>, <a href="#assert_contains-kwargs">kwargs</a>)
+assert_contains(<a href="#assert_contains-name">name</a>, <a href="#assert_contains-actual">actual</a>, <a href="#assert_contains-expected">expected</a>, <a href="#assert_contains-size">size</a>, <a href="#assert_contains-kwargs">kwargs</a>)
</pre>
Generates a test target which fails if the file doesn't contain the string.
@@ -45,8 +45,7 @@
| <a id="assert_contains-name"></a>name | target to create | none |
| <a id="assert_contains-actual"></a>actual | Label of a file | none |
| <a id="assert_contains-expected"></a>expected | a string which should appear in the file | none |
-| <a id="assert_contains-size"></a>size | the size attribute of the test target | <code>None</code> |
-| <a id="assert_contains-timeout"></a>timeout | the timeout attribute of the test target | <code>None</code> |
+| <a id="assert_contains-size"></a>size | standard attribute for tests | <code>"small"</code> |
| <a id="assert_contains-kwargs"></a>kwargs | additional named arguments for the resulting sh_test | none |
diff --git a/lib/private/diff_test.bzl b/lib/private/diff_test.bzl
index 68d0b95..e1ea14e 100644
--- a/lib/private/diff_test.bzl
+++ b/lib/private/diff_test.bzl
@@ -21,7 +21,6 @@
command (fc.exe) on Windows (no Bash is required).
"""
-load("//lib:utils.bzl", "default_timeout")
load(":directory_path.bzl", "DirectoryPathInfo")
def _runfiles_path(f):
@@ -107,7 +106,7 @@
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.
@@ -117,15 +116,12 @@
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
)
diff --git a/lib/private/write_source_file.bzl b/lib/private/write_source_file.bzl
index f5e85d2..c2b2a5c 100644
--- a/lib/private/write_source_file.bzl
+++ b/lib/private/write_source_file.bzl
@@ -122,6 +122,7 @@
message = message,
visibility = kwargs.get("visibility"),
tags = kwargs.get("tags"),
+ size = "small",
)
else:
if suggested_update_target == None:
diff --git a/lib/testing.bzl b/lib/testing.bzl
index 1b6feb2..d508218 100644
--- a/lib/testing.bzl
+++ b/lib/testing.bzl
@@ -5,9 +5,8 @@
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.
@@ -16,8 +15,7 @@
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
"""
@@ -45,7 +43,6 @@
srcs = [test_sh],
args = ["$(rootpath %s)" % expected_file, "$(rootpath %s)" % actual],
size = size,
- timeout = default_timeout(size, timeout),
data = [actual, expected_file],
**kwargs
)
diff --git a/lib/tests/write_source_files/write_source_file_test.bzl b/lib/tests/write_source_files/write_source_file_test.bzl
index 0bebe6c..d242067 100644
--- a/lib/tests/write_source_files/write_source_file_test.bzl
+++ b/lib/tests/write_source_files/write_source_file_test.bzl
@@ -172,7 +172,7 @@
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(
@@ -190,5 +190,5 @@
write_source_file_target = name + "_updater",
in_file = in_file,
out_file = out_file,
- timeout = "short",
+ size = size,
)
