From 5a423f2ae5e3b92ac133535cc17ab123d40291f7 Mon Sep 17 00:00:00 2001 From: Jerry Marino Date: Wed, 9 Aug 2023 17:44:52 -0700 Subject: [PATCH] [Test] Break massive ios_test function into factory Add the ability to override stuff. Test rules were impossible to extend and difficult reasoning about how it worked with rules_apple - which led to issues implementing new features like shard by class. Test rules simply accept a `test_suite_factory`. On the default one, which you can override parts if necesary. _The default one could be much generic - but at some point, I expect people to just make their own `test_suite_factory` VS adding too many abstractions to it_ Last: normalize all into a single bundle - otherwise it would duplicate tons of data and be un-useable at scale. --- rules/test.bzl | 172 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 117 insertions(+), 55 deletions(-) diff --git a/rules/test.bzl b/rules/test.bzl index 82434aa46..e82b83d17 100644 --- a/rules/test.bzl +++ b/rules/test.bzl @@ -1,4 +1,4 @@ -load("@build_bazel_rules_apple//apple:ios.bzl", rules_apple_ios_ui_test = "ios_ui_test", rules_apple_ios_ui_test_suite = "ios_ui_test_suite", rules_apple_ios_unit_test = "ios_unit_test", rules_apple_ios_unit_test_suite = "ios_unit_test_suite") +load("@build_bazel_rules_apple//apple/internal/testing:ios_rules.bzl", _ios_internal_ui_test_bundle = "ios_internal_ui_test_bundle", _ios_internal_unit_test_bundle = "ios_internal_unit_test_bundle", _ios_ui_test = "ios_ui_test", _ios_unit_test = "ios_unit_test",) load("@bazel_skylib//lib:types.bzl", "types") load("//rules:library.bzl", "apple_library") load("//rules:plists.bzl", "process_infoplists") @@ -25,14 +25,95 @@ _IOS_TEST_KWARGS = [ "visibility", ] -def _ios_test(name, test_rule, test_suite_rule, apple_library, infoplists_by_build_setting = {}, split_name_to_kwargs = {}, internal_test_deps = [], **kwargs): +_APPLE_BUNDLE_ATTRS = { + x: None + for x in [ + "additional_contents", + "deps", + "bundle_id", + "bundle_name", + "families", + "frameworks", + "infoplists", + "linkopts", + "minimum_os_version", + "provisioning_profile", + "resources", + "test_host", + ] +} + +_DEFAULT_APPLE_TEST_RUNNER = "@build_bazel_rules_apple//apple/testing/default_runner:ios_default_runner" + +def _make_runner_split(name, runner, **in_split): + split = {} + split.update(in_split) + split["runner"] = runner + return split + +def _make_named_split(name, split_kwargs, **in_split): + split = {} + split.update(in_split) + split.update(split_kwargs) + return split + +def _make_test_suite_splits(factory, name, **in_kwargs): + """ + Helper function to split up a test for named splits and runners splits + + At the end of the day, we need to able to control how many tests / bundles + there are for sharding by class, otherwise it would recompile many times. + + Finally - you can set the splits to be whatever you want. + """ + if "runners" in in_kwargs and "runner" in in_kwargs: + fail("cannot use runner/s attribute in both split and top level kwargs for %s" % test_name) + + split_name_to_kwargs = in_kwargs.get("split_name_to_kwargs", {}) + splits = {} + if split_name_to_kwargs and len(split_name_to_kwargs) > 0: + for suffix, split_kwargs in split_name_to_kwargs.items(): + test_name = "{}_{}".format(name, suffix) + splits[test_name] = factory.make_named_split(name, split_kwargs, **in_kwargs) + + runners = in_kwargs.get("runners", []) + if runners and len(runners) > 0: + splits_by_runners = {} + if len(splits) == 0: + splits[name] = in_kwargs + for runner in runners: + runner_name = runner.rsplit(":", 1)[-1] + for in_test_name, in_split in splits: + test_name = "{}_{}".format(in_test_name, runner_name) + splits_by_runners[test_name] = factory.make_runner_split(name, runner, **in_split) + splits = splits_by_runners + return splits + +def _make_test_suite(factory, name, test_rule, **test_kwargs): + splits = factory.make_splits(factory, name, test_kwargs) + tests = [] + for split_name, split in splits.items(): + tests.append(split_name) + test_rule(split_name, split) + test_suite_visibility = test_kwargs.get("visibility", None) + test_suite_tags = test_kwargs.get("tags", []) + native.test_suite(name = name, tests = tests, visibility = test_suite_visibility, tags = test_suite_tags) + +default_test_suite_factory = struct( + make_test_suite = _make_test_suite, + make_test_suite_splits = _make_test_suite_splits, + make_runner_split = _make_runner_split, + make_named_split = _make_named_split +) + +def _ios_test(name, bundle_rule, test_rule, test_suite_factory, apple_library, infoplists_by_build_setting = {}, split_name_to_kwargs = {}, internal_test_deps = [], **kwargs): """ Builds and packages iOS Unit/UI Tests. Args: name: The name of the unit test. test_rule: The underlying rules_apple test rule. - test_suite_rule: The underlying rules_apple test suite rule. + bundle_rule: The underlying rules_apple test suite rule. apple_library: The macro used to package sources into a library. infoplists_by_build_setting: A dictionary of infoplists grouped by bazel build setting. @@ -84,61 +165,42 @@ def _ios_test(name, test_rule, test_suite_rule, apple_library, infoplists_by_bui xcconfig_by_build_setting = kwargs.get("xcconfig_by_build_setting", {}), ) - if split_name_to_kwargs and len(split_name_to_kwargs) > 0: - tests = [] - for suffix, split_kwargs in split_name_to_kwargs.items(): - test_name = "{}_{}".format(name, suffix) - - all_kwargs = {} - all_kwargs.update(ios_test_kwargs) - all_kwargs.update(split_kwargs) - - if runner and ("runner" in all_kwargs or "runners" in all_kwargs): - fail("cannot use runner/s attribute in both split and top level kwargs for %s" % test_name) - - if not runner: - split_runner = all_kwargs.pop("runner", all_kwargs.pop("runners", None)) - else: - split_runner = runner - - split_rule = test_rule - if split_runner: - if types.is_list(runner): - all_kwargs["runners"] = split_runner - split_rule = test_suite_rule - else: - all_kwargs["runner"] = split_runner + # Set this to a single __internal__ test bundle. + test_bundle_name = name + ".__internal__.__test_bundle" + bundle_attrs = {k: v for (k, v) in ios_test_kwargs.items() if k in _APPLE_BUNDLE_ATTRS} + bundle_rule( + name = test_bundle_name, + bundle_name = name, + test_bundle_output = "{}.zip".format(name), + testonly = True, + frameworks = frameworks, + infoplists = select(infoplists), + deps = [dep_name] + internal_test_deps, + **bundle_attrs + ) - tests.append(test_name) - split_rule( + if "runners" in ios_test_kwargs or "split_name_to_kwargs" in ios_test_kwargs: + def _split_rule(test_name, **split_kwargs): + test_attrs = {k: v for (k, v) in split_kwargs.items() if k not in _APPLE_BUNDLE_ATTRS} + runner = split_kwargs.pop("runner", None) or _DEFAULT_APPLE_TEST_RUNNER + test_rule( name = test_name, - deps = [dep_name] + internal_test_deps, - frameworks = frameworks, + deps = [":{}".format(test_bundle_name)], testonly = testonly, - infoplists = select(infoplists), - **all_kwargs + **test_attrs ) - test_suite_visibility = ios_test_kwargs.get("visibility", None) - test_suite_tags = ios_test_kwargs.get("tags", []) - native.test_suite(name = name, tests = tests, visibility = test_suite_visibility, tags = test_suite_tags) + test_suite_factory.make_test_suite(test_suite_factory, name, test_rule, **ios_test_kwargs) else: - rule = test_rule - if runner: - if types.is_list(runner): - ios_test_kwargs["runners"] = runner - rule = test_suite_rule - else: - ios_test_kwargs["runner"] = runner - - rule( + test_attrs = {k: v for (k, v) in ios_test_kwargs.items() if k not in _APPLE_BUNDLE_ATTRS} + runner = kwargs.pop("runner", None) or _DEFAULT_APPLE_TEST_RUNNER + test_rule( name = name, - deps = [dep_name] + internal_test_deps, - frameworks = frameworks, - infoplists = select(infoplists), - **ios_test_kwargs + deps = [":{}".format(test_bundle_name)], + runner = runner, + **test_attrs ) -def ios_unit_test(name, apple_library = apple_library, **kwargs): +def ios_unit_test(name, apple_library = apple_library, test_suite_factory = default_test_suite_factory, **kwargs): """ Builds and packages iOS Unit Tests. @@ -147,9 +209,9 @@ def ios_unit_test(name, apple_library = apple_library, **kwargs): apple_library: The macro used to package sources into a library. **kwargs: Arguments passed to the apple_library and ios_unit_test rules as appropriate. """ - _ios_test(name, rules_apple_ios_unit_test, rules_apple_ios_unit_test_suite, apple_library, **kwargs) + _ios_test(name, _ios_internal_unit_test_bundle, _ios_unit_test, test_suite_factory, apple_library, **kwargs) -def ios_ui_test(name, apple_library = apple_library, **kwargs): +def ios_ui_test(name, apple_library = apple_library, test_suite_factory = default_test_suite_factory, **kwargs): """ Builds and packages iOS UI Tests. @@ -160,9 +222,9 @@ def ios_ui_test(name, apple_library = apple_library, **kwargs): """ if not kwargs.get("test_host", None): fail("test_host is required for ios_ui_test.") - _ios_test(name, rules_apple_ios_ui_test, rules_apple_ios_ui_test_suite, apple_library, **kwargs) + _ios_test(name, _ios_internal_ui_test_bundle, _ios_ui_test, test_suite_factory, apple_library, **kwargs) -def ios_unit_snapshot_test(name, apple_library = apple_library, **kwargs): +def ios_unit_snapshot_test(name, apple_library = apple_library, test_suite_factory = default_test_suite_factory, **kwargs): """ Builds and packages iOS Unit Snapshot Tests. @@ -171,4 +233,4 @@ def ios_unit_snapshot_test(name, apple_library = apple_library, **kwargs): apple_library: The macro used to package sources into a library. **kwargs: Arguments passed to the apple_library and ios_unit_test rules as appropriate. """ - _ios_test(name, rules_apple_ios_unit_test, rules_apple_ios_unit_test_suite, apple_library, internal_test_deps = ["@bazel_tools//tools/cpp/runfiles"], **kwargs) + _ios_test(name, _ios_internal_unit_test_bundle, _ios_unit_test, test_suite_factory, apple_library, internal_test_deps = ["@bazel_tools//tools/cpp/runfiles"], **kwargs)