From ed7f03cde643bc0e17d521a3d7f45f1aeaa25c5c Mon Sep 17 00:00:00 2001 From: David Sanderson <32687193+dws-uber@users.noreply.github.com> Date: Thu, 12 Nov 2020 21:04:39 -0500 Subject: [PATCH] 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. --- lib/BUILD | 1 + lib/partial.bzl | 27 +++++++++++++++++++++++++++ lib/unittest.bzl | 16 ++++++++++------ tests/partial_tests.bzl | 18 ++++++++++++++++++ tests/unittest_test.sh | 1 + tests/unittest_tests.bzl | 15 +++++++++++++++ 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/lib/BUILD b/lib/BUILD index 26a61201..7e7a9a4b 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 e2f24b79..8bfba7f0 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 5b70df5a..87dfd277 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 6d778c31..73a579bb 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 baed4909..9455d8e1 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 0b992f9a..7d30e2a5 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(