Enable unittest.suite to accept partial calls of test rules (#276)

* Enable unittest.suite to accept partial calls of rules

This permits using `unittest.suite` with test rules that have nondefault
attributes, while retaining compatibility with current usage.

For instance, this permits setting a `timeout` on each test in a
`unittest.suite`.  Previously, all tests in a `unittest.suite` would
have the default timeout, with no good way to alter this.  This
made it hard to eliminate all the warnings produced from using the
`--test_verbose_timeout_warnings` bazel option.

While timeouts were the motivation, the solution here is not specific
to timeouts. It will permit arbitrary additional arguments to the test
rules in a `unittest.suite`.

Fixes #98

* Respond to review feedback.

* Document a breaking change in bazel that this code needs to be aware of.

6 files changed, 72 insertions, 6 deletions

diff --git a/lib/BUILD b/lib/BUILD
index 26a6120..7e7a9a4 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -71,6 +71,7 @@ bzl_library(
     srcs = ["unittest.bzl"],
     deps = [
         ":new_sets",
+        ":partial",
         ":sets",
         ":types",
     ],
diff --git a/lib/partial.bzl b/lib/partial.bzl
index e2f24b7..8bfba7f 100644
--- a/lib/partial.bzl
+++ b/lib/partial.bzl
@@ -19,6 +19,11 @@ Partial function objects allow some parameters are bound before the call.
 Similar to https://docs.python.org/3/library/functools.html#functools.partial.
 """
 
+# create instance singletons to avoid unnecessary allocations
+_a_dict_type = type({})
+_a_tuple_type = type(())
+_a_struct_type = type(struct())
+
 def _call(partial, *args, **kwargs):
     """Calls a partial created using `make`.
 
@@ -124,7 +129,29 @@ def _make(func, *args, **kwargs):
     """
     return struct(function = func, args = args, kwargs = kwargs)
 
+def _is_instance(v):
+    """Returns True if v is a partial created using `make`.
+
+    Args:
+      v: The value to check.
+
+    Returns:
+      True if v was created by `make`, False otherwise.
+    """
+    # Note that in bazel 3.7.0 and earlier, type(v.function) is the same
+    # as the type of a function even if v.function is a rule. But we
+    # cannot rely on this in later bazels due to breaking change
+    # https://github.com/bazelbuild/bazel/commit/e379ece1908aafc852f9227175dd3283312b4b82
+    #
+    # Since this check is heuristic anyway, we simply check for the
+    # presence of a "function" attribute without checking its type.
+    return type(v) == _a_struct_type \
+        and hasattr(v, "function") \
+        and hasattr(v, "args") and type(v.args) == _a_tuple_type \
+        and hasattr(v, "kwargs") and type(v.kwargs) == _a_dict_type
+
 partial = struct(
     make = _make,
     call = _call,
+    is_instance = _is_instance,
 )
diff --git a/lib/unittest.bzl b/lib/unittest.bzl
index 5b70df5..87dfd27 100644
--- a/lib/unittest.bzl
+++ b/lib/unittest.bzl
@@ -20,6 +20,7 @@ assertions used to within tests.
 """
 
 load(":new_sets.bzl", new_sets = "sets")
+load(":partial.bzl", "partial")
 load(":types.bzl", "types")
 
 # The following function should only be called from WORKSPACE files and workspace macros.
@@ -232,10 +233,10 @@ def _suite(name, *test_rules):
     writing a macro in your `.bzl` file to instantiate all targets, and calling
     that macro from your BUILD file so you only have to load one symbol.
 
-    For the case where your unit tests do not take any (non-default) attributes --
-    i.e., if your unit tests do not test rules -- you can use this function to
-    create the targets and wrap them in a single test_suite target. In your
-    `.bzl` file, write:
+    You can use this function to create the targets and wrap them in a single
+    test_suite target. If a test rule requires no arguments, you can simply list
+    it as an argument. If you wish to supply attributes explicitly, you can do so
+    using `partial.make()`. For instance, in your `.bzl` file, you could write:
 
     ```
     def your_test_suite():
@@ -243,7 +244,7 @@ def _suite(name, *test_rules):
           "your_test_suite",
           your_test,
           your_other_test,
-          yet_another_test,
+          partial.make(yet_another_test, timeout = "short"),
       )
     ```
 
@@ -269,7 +270,10 @@ def _suite(name, *test_rules):
     test_names = []
     for index, test_rule in enumerate(test_rules):
         test_name = "%s_test_%d" % (name, index)
-        test_rule(name = test_name)
+        if partial.is_instance(test_rule):
+            partial.call(test_rule, name = test_name)
+        else:
+            test_rule(name = test_name)
         test_names.append(test_name)
 
     native.test_suite(
diff --git a/tests/partial_tests.bzl b/tests/partial_tests.bzl
index 6d778c3..73a579b 100644
--- a/tests/partial_tests.bzl
+++ b/tests/partial_tests.bzl
@@ -76,9 +76,27 @@ def _make_call_test(ctx):
 
 make_call_test = unittest.make(_make_call_test)
 
+def _is_instance_test(ctx):
+    """Unit test for partial.is_instance."""
+    env = unittest.begin(ctx)
+
+    # We happen to use make_call_test here, but it could be any valid test rule.
+    asserts.true(env, partial.is_instance(partial.make(make_call_test)))
+    asserts.true(env, partial.is_instance(partial.make(make_call_test, timeout = "short")))
+    asserts.true(env, partial.is_instance(partial.make(make_call_test, timeout = "short", tags = ["foo"])))
+    asserts.false(env, partial.is_instance(None))
+    asserts.false(env, partial.is_instance({}))
+    asserts.false(env, partial.is_instance(struct(foo = 1)))
+    asserts.false(env, partial.is_instance(struct(function = "not really function")))
+
+    return unittest.end(env)
+
+is_instance_test = unittest.make(_is_instance_test)
+
 def partial_test_suite():
     """Creates the test targets and test suite for partial.bzl tests."""
     unittest.suite(
         "partial_tests",
         make_call_test,
+        is_instance_test,
     )
diff --git a/tests/unittest_test.sh b/tests/unittest_test.sh
index baed490..9455d8e 100755
--- a/tests/unittest_test.sh
+++ b/tests/unittest_test.sh
@@ -73,6 +73,7 @@ exports_files(["*.bzl"])
 EOF
   ln -sf "$(rlocation bazel_skylib/lib/dicts.bzl)" lib/dicts.bzl
   ln -sf "$(rlocation bazel_skylib/lib/new_sets.bzl)" lib/new_sets.bzl
+  ln -sf "$(rlocation bazel_skylib/lib/partial.bzl)" lib/partial.bzl
   ln -sf "$(rlocation bazel_skylib/lib/sets.bzl)" lib/sets.bzl
   ln -sf "$(rlocation bazel_skylib/lib/types.bzl)" lib/types.bzl
   ln -sf "$(rlocation bazel_skylib/lib/unittest.bzl)" lib/unittest.bzl
diff --git a/tests/unittest_tests.bzl b/tests/unittest_tests.bzl
index 0b992f9..7d30e2a 100644
--- a/tests/unittest_tests.bzl
+++ b/tests/unittest_tests.bzl
@@ -14,6 +14,7 @@
 
 """Unit tests for unittest.bzl."""
 
+load("//lib:partial.bzl", "partial")
 load("//lib:unittest.bzl", "analysistest", "asserts", "unittest")
 
 ###################################
@@ -42,6 +43,19 @@ def _basic_passing_test(ctx):
 
 basic_passing_test = unittest.make(_basic_passing_test)
 
+#################################################
+####### basic_passing_short_timeout_test ########
+#################################################
+def _basic_passing_short_timeout_test(ctx):
+    """Unit tests for a basic library verification test."""
+    env = unittest.begin(ctx)
+
+    asserts.equals(env, ctx.attr.timeout, "short")
+
+    return unittest.end(env)
+
+basic_passing_short_timeout_test = unittest.make(_basic_passing_short_timeout_test)
+
 ###################################
 ####### change_setting_test #######
 ###################################
@@ -240,6 +254,7 @@ def unittest_passing_tests_suite():
     unittest.suite(
         "unittest_tests",
         basic_passing_test,
+        partial.make(basic_passing_short_timeout_test, timeout = "short"),
     )
 
     change_setting_test(