chore: set test timeouts to short

I recently enabled --test_verbose_timeout_warnings and that caused a bunch of warnings in our build.
This fixes it, and adds a utility for us or others to make test-wrapping macros that set to short by default.

docs/testing.md

@@ -7,7 +7,7 @@ Helpers for making test assertions
 ## 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>)
+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>)
 </pre>
 
 Generates a test target which fails if the file doesn't contain the string.
@@ -23,5 +23,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> |
 
 

docs/utils.md

@@ -2,6 +2,45 @@
 
 Public API
 
+<a id="default_timeout"></a>
+
+## default_timeout
+
+<pre>
+default_timeout(<a href="#default_timeout-size">size</a>, <a href="#default_timeout-timeout">timeout</a>)
+</pre>
+
+Provide a sane default for *_test timeout attribute.
+
+The [test-encyclopedia](https://bazel.build/reference/test-encyclopedia) says
+> Tests may return arbitrarily fast regardless of timeout.
+> A test is not penalized for an overgenerous timeout, although a warning may be issued:
+> you should generally set your timeout as tight as you can without incurring any flakiness.
+
+However Bazel's default for timeout is medium, which is dumb given this guidance.
+
+It also says:
+> Tests which do not explicitly specify a timeout have one implied based on the test's size as follows
+Therefore if size is specified, we should allow timeout to take its implied default.
+If neither is set, then we can fix Bazel's wrong default here to avoid warnings under
+`--test_verbose_timeout_warnings`.
+
+This function can be used in a macro which wraps a testing rule.
+
+
+**PARAMETERS**
+
+
+| Name  | Description | Default Value |
+| :------------- | :------------- | :------------- |
+| <a id="default_timeout-size"></a>size |  the size attribute of a test target   |  none |
+| <a id="default_timeout-timeout"></a>timeout |  the timeout attribute of a test target   |  none |
+
+**RETURNS**
+
+"short" if neither is set, otherwise timeout
+
+
 <a id="file_exists"></a>
 
 ## file_exists

lib/BUILD.bazel

@@ -123,16 +123,6 @@ bzl_library(
     deps = ["//lib/private:run_binary"],
 )
 
-bzl_library(
-    name = "transitions",
-    srcs = ["transitions.bzl"],
-)
-
-bzl_library(
-    name = "windows_utils",
-    srcs = ["windows_utils.bzl"],
-)
-
 bzl_library(
     name = "repo_utils",
     srcs = ["repo_utils.bzl"],
@@ -169,5 +159,35 @@ bzl_library(
 bzl_library(
     name = "testing",
     srcs = ["testing.bzl"],
-    deps = ["@bazel_skylib//rules:write_file"],
+    deps = [
+        ":utils",
+        "@bazel_skylib//rules:write_file",
+    ],
+)
+
+bzl_library(
+    name = "extensions",
+    srcs = ["extensions.bzl"],
+    deps = ["@aspect_bazel_lib//lib:repositories"],
+)
+
+bzl_library(
+    name = "repositories",
+    srcs = ["repositories.bzl"],
+    deps = [
+        "//lib/private:jq_toolchain",
+        "//lib/private:yq_toolchain",
+        "@bazel_tools//tools/build_defs/repo:http.bzl",
+        "@bazel_tools//tools/build_defs/repo:utils.bzl",
+    ],
+)
+
+bzl_library(
+    name = "transitions",
+    srcs = ["transitions.bzl"],
+)
+
+bzl_library(
+    name = "windows_utils",
+    srcs = ["windows_utils.bzl"],
 )

lib/private/BUILD.bazel

@@ -153,29 +153,41 @@ bzl_library(
     ],
 )
 
-bzl_library(
-    name = "repo_utils",
-    srcs = ["repo_utils.bzl"],
-)
-
 bzl_library(
     name = "patch",
     srcs = ["patch.bzl"],
     deps = [":repo_utils"],
 )
 
-bzl_library(
-    name = "yq",
-    srcs = ["yq.bzl"],
-)
-
 bzl_library(
     name = "host_repo",
     srcs = ["host_repo.bzl"],
     deps = [":repo_utils"],
 )
 
+bzl_library(
+    name = "jq_toolchain",
+    srcs = ["jq_toolchain.bzl"],
+    deps = [":repo_utils"],
+)
+
+bzl_library(
+    name = "repo_utils",
+    srcs = ["repo_utils.bzl"],
+)
+
 bzl_library(
     name = "stamping",
     srcs = ["stamping.bzl"],
 )
+
+bzl_library(
+    name = "yq",
+    srcs = ["yq.bzl"],
+)
+
+bzl_library(
+    name = "yq_toolchain",
+    srcs = ["yq_toolchain.bzl"],
+    deps = [":repo_utils"],
+)

lib/private/diff_test.bzl

@@ -19,6 +19,7 @@ command (fc.exe) on Windows (no Bash is required).
 """
 
 load(":directory_path.bzl", "DirectoryPathInfo")
+load("//lib:utils.bzl", "default_timeout")
 
 def _runfiles_path(f):
     if f.root.path:
@@ -255,7 +256,7 @@ _diff_test = rule(
     implementation = _diff_test_impl,
 )
 
-def diff_test(name, file1, file2, **kwargs):
+def diff_test(name, file1, file2, size = None, timeout = None, **kwargs):
     """A test that compares two files.
 
     The test succeeds if the files' contents match.
@@ -264,11 +265,16 @@ def diff_test(name, file1, file2, **kwargs):
       name: The name of the test rule.
       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/utils.bzl

@@ -113,6 +113,37 @@ def _file_exists(path):
     file_glob = native.glob([file_rel], exclude_directories = 1)
     return len(file_glob) > 0
 
+def _default_timeout(size, timeout):
+    """Provide a sane default for *_test timeout attribute.
+
+    The [test-encyclopedia](https://bazel.build/reference/test-encyclopedia) says
+    > Tests may return arbitrarily fast regardless of timeout.
+    > A test is not penalized for an overgenerous timeout, although a warning may be issued:
+    > you should generally set your timeout as tight as you can without incurring any flakiness.
+
+    However Bazel's default for timeout is medium, which is dumb given this guidance.
+
+    It also says:
+    > Tests which do not explicitly specify a timeout have one implied based on the test's size as follows
+    Therefore if size is specified, we should allow timeout to take its implied default.
+    If neither is set, then we can fix Bazel's wrong default here to avoid warnings under
+    `--test_verbose_timeout_warnings`.
+
+    This function can be used in a macro which wraps a testing rule.
+
+    Args:
+        size: the size attribute of a test target
+        timeout: the timeout attribute of a test target
+
+    Returns:
+        "short" if neither is set, otherwise timeout
+    """
+
+    if size == None and timeout == None:
+        return "short"
+
+    return timeout
+
 utils = struct(
     is_external_label = _is_external_label,
     file_exists = _file_exists,
@@ -120,4 +151,5 @@ utils = struct(
     path_to_workspace_root = _path_to_workspace_root,
     propagate_well_known_tags = _propagate_well_known_tags,
     to_label = _to_label,
+    default_timeout = _default_timeout,
 )

lib/testing.bzl

@@ -1,8 +1,9 @@
 "Helpers for making test assertions"
 
 load("@bazel_skylib//rules:write_file.bzl", "write_file")
+load("//lib:utils.bzl", "default_timeout")
 
-def assert_contains(name, actual, expected):
+def assert_contains(name, actual, expected, size = None, timeout = None):
     """Generates a test target which fails if the file doesn't contain the string.
 
     Depends on bash, as it creates an sh_test target.
@@ -11,6 +12,8 @@ def assert_contains(name, actual, expected):
         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
     """
 
     test_sh = "_{}_test.sh".format(name)
@@ -29,5 +32,7 @@ def assert_contains(name, actual, expected):
         name = name,
         srcs = [test_sh],
         args = ["$(rootpath %s)" % actual],
+        size = size,
+        timeout = default_timeout(size, timeout),
         data = [actual],
     )

lib/tests/BUILD.bazel

@@ -43,6 +43,7 @@ expand_template(
 
 sh_test(
     name = "expand_template_test",
+    timeout = "short",
     srcs = ["expand_template"],
 )
 

lib/tests/copy_to_bin/BUILD.bazel

@@ -25,6 +25,7 @@ output_files(
 
 sh_test(
     name = "file1_test",
+    timeout = "short",
     srcs = ["test.sh"],
     args = [
         "lib/tests/copy_to_bin/file1",
@@ -39,6 +40,7 @@ sh_test(
 
 sh_test(
     name = "file2_test",
+    timeout = "short",
     srcs = ["test.sh"],
     args = [
         "lib/tests/copy_to_bin/file2",
@@ -55,13 +57,13 @@ sh_test(
 copy_to_bin(
     name = "copy_same_file_1",
     srcs = [
-        "file3"
+        "file3",
-    ]
+    ],
 )
 
 copy_to_bin(
     name = "copy_same_file_2",
     srcs = [
-        "file3"
+        "file3",
-    ]
+    ],
 )

lib/tests/copy_to_directory/BUILD.bazel

@@ -155,6 +155,7 @@ copy_to_directory(
 
 sh_test(
     name = "case_6_test",
+    timeout = "short",
     srcs = ["case6.sh"],
     data = ["case_6"],
 )
@@ -437,4 +438,4 @@ diff_test(
     name = "case_20_test",
     file1 = "case_20",
     file2 = ":expected_20",
-)
+)

lib/tests/copy_to_directory_action/BUILD.bazel

@@ -1,3 +1,4 @@
+load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
 load("//lib:diff_test.bzl", "diff_test")
 load("//lib:copy_to_directory.bzl", "copy_to_directory")
 load("//lib:copy_to_bin.bzl", "copy_to_bin")
@@ -42,3 +43,9 @@ diff_test(
     file1 = ":pkg",
     file2 = ":expected_pkg",
 )
+
+bzl_library(
+    name = "other_info",
+    srcs = ["other_info.bzl"],
+    visibility = ["//visibility:public"],
+)

lib/tests/expand_make_vars_test.bzl

@@ -2,6 +2,7 @@
 See https://docs.bazel.build/versions/main/skylark/testing.html#for-testing-starlark-utilities
 """
 
+load("@bazel_skylib//lib:partial.bzl", "partial")
 load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest")
 load("//lib:expand_make_vars.bzl", "expand_variables")
 
@@ -35,4 +36,4 @@ def _variables_test_impl(ctx):
 t0_test = unittest.make(_variables_test_impl)
 
 def expand_make_vars_test_suite():
-    unittest.suite("make_vars_tests", t0_test)
+    unittest.suite("make_vars_tests", partial.make(t0_test, timeout = "short"))

lib/tests/glob_match_test.bzl

@@ -1,5 +1,6 @@
 """unit tests for glob_match"""
 
+load("@bazel_skylib//lib:partial.bzl", "partial")
 load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest")
 load("//lib:glob_match.bzl", "glob_match")
 
@@ -151,17 +152,17 @@ mixed_wrapper_globstar_test = unittest.make(_mixed_wrapping_globstar)
 def glob_match_test_suite():
     unittest.suite(
         "glob_match_tests",
-        star_test,
+        partial.make(star_test, timeout = "short"),
-        globstar_test,
+        partial.make(globstar_test, timeout = "short"),
-        qmark_test,
+        partial.make(qmark_test, timeout = "short"),
-        qmark_qmark_test,
+        partial.make(qmark_qmark_test, timeout = "short"),
-        wrapped_qmark_test,
+        partial.make(wrapped_qmark_test, timeout = "short"),
-        mixed_wrapped_qmark_test,
+        partial.make(mixed_wrapped_qmark_test, timeout = "short"),
-        ending_star_test,
+        partial.make(ending_star_test, timeout = "short"),
-        wrapping_star_test,
+        partial.make(wrapping_star_test, timeout = "short"),
-        wrapped_star_test,
+        partial.make(wrapped_star_test, timeout = "short"),
-        starting_star_test,
+        partial.make(starting_star_test, timeout = "short"),
-        mixed_trailing_globstar_test,
+        partial.make(mixed_trailing_globstar_test, timeout = "short"),
-        mixed_leading_globstar_test,
+        partial.make(mixed_leading_globstar_test, timeout = "short"),
-        mixed_wrapper_globstar_test,
+        partial.make(mixed_wrapper_globstar_test, timeout = "short"),
     )

lib/tests/paths_test.bzl

@@ -1,5 +1,6 @@
 """unit tests for paths"""
 
+load("@bazel_skylib//lib:partial.bzl", "partial")
 load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest")
 load("//lib/private:paths.bzl", "paths")
 
@@ -227,8 +228,8 @@ workspace_path_test = unittest.make(_workspace_path_test_impl, attrs = _ATTRS)
 def paths_test_suite():
     unittest.suite(
         "paths_tests",
-        relative_file_test,
+        partial.make(relative_file_test, timeout = "short"),
-        manifest_path_test,
+        partial.make(manifest_path_test, timeout = "short"),
-        output_relative_path_test,
+        partial.make(output_relative_path_test, timeout = "short"),
-        workspace_path_test,
+        partial.make(workspace_path_test, timeout = "short"),
     )

lib/tests/stamping/BUILD.bazel

@@ -1,3 +1,4 @@
+load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
 load(":stamp_aware_rule.bzl", "my_stamp_aware_rule")
 load("//lib:run_binary.bzl", "run_binary")
 
@@ -32,3 +33,10 @@ run_binary(
     stamp = -1,
     tool = "stamper",
 )
+
+bzl_library(
+    name = "stamp_aware_rule",
+    srcs = ["stamp_aware_rule.bzl"],
+    visibility = ["//visibility:public"],
+    deps = ["//lib:stamping"],
+)

lib/tests/utils_test.bzl

@@ -89,16 +89,18 @@ def file_exists_test():
 def utils_test_suite():
     to_label_test(name = "to_label_tests", relative_asserts = {
         utils.to_label(":utils_test.bzl"): "//lib/tests:utils_test.bzl",
-    })
+    }, timeout = "short")
 
     is_external_label_test(
         name = "is_external_label_tests",
         external_as_string = utils.is_external_label("@foo//some/label"),
         internal_with_workspace_as_string = utils.is_external_label("@aspect_bazel_lib//some/label"),
+        timeout = "short",
     )
 
     propagate_well_known_tags_test(
         name = "propagate_well_known_tags_tests",
+        timeout = "short",
     )
 
     file_exists_test()

lib/tests/write_source_files/write_source_file_test.bzl

@@ -186,4 +186,5 @@ def write_source_file_test(name, in_file, out_file):
         write_source_file_target = name + "_updater",
         in_file = in_file,
         out_file = out_file,
+        timeout = "short",
     )

lib/utils.bzl

@@ -8,3 +8,4 @@ path_to_workspace_root = utils.path_to_workspace_root
 propagate_well_known_tags = utils.propagate_well_known_tags
 to_label = utils.to_label
 file_exists = utils.file_exists
+default_timeout = utils.default_timeout