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>

docs/diff_test.md

@@ -14,7 +14,7 @@ command (fc.exe) on Windows (no Bash is required).
 ## 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 @@ The test succeeds if the files' contents match.
 | <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 &lt;code&gt;file2&lt;/code&gt;.   |  none |
 | <a id="diff_test-file2"></a>file2 |  Label of the file to compare to &lt;code&gt;file1&lt;/code&gt;.   |  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 &lt;a href="https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes-tests"&gt;common attributes for tests&lt;/a&gt;.   |  none |
 
 

docs/testing.md

@@ -29,7 +29,7 @@ Assert that an archive file contains at least the given file entries.
 ## 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 @@ Depends on bash, as it creates an sh_test target.
 | <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 |
 
 

lib/private/diff_test.bzl

@@ -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):
@@ -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.
@@ -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
     )

lib/private/write_source_file.bzl

@@ -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:

lib/testing.bzl

@@ -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.
@@ -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
     """
 
@@ -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
     )

lib/tests/write_source_files/write_source_file_test.bzl

@@ -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(
@@ -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,
     )