From 3fdaa96da16fa97225ca7cb02b9d119c746db59d Mon Sep 17 00:00:00 2001 From: Derek Ostrander Date: Fri, 9 Dec 2022 13:49:38 -0500 Subject: [PATCH] Pass product module name into test_rule --- patches/BUILD.bazel | 0 patches/rules_apple_module_name.patch | 84 ++++++++++++++ patches/xctestrunner_module_name.patch | 147 +++++++++++++++++++++++++ rules/repositories.bzl | 14 +++ rules/test.bzl | 6 + 5 files changed, 251 insertions(+) create mode 100644 patches/BUILD.bazel create mode 100644 patches/rules_apple_module_name.patch create mode 100644 patches/xctestrunner_module_name.patch diff --git a/patches/BUILD.bazel b/patches/BUILD.bazel new file mode 100644 index 00000000..e69de29b diff --git a/patches/rules_apple_module_name.patch b/patches/rules_apple_module_name.patch new file mode 100644 index 00000000..2e0cc88e --- /dev/null +++ b/patches/rules_apple_module_name.patch @@ -0,0 +1,84 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Derek Ostrander +Date: Fri, 9 Dec 2022 13:55:16 -0500 +Subject: Add product module name as an optional arg for the test rule + +If there are modules under the tests that are named differently then the target itself it will cause issues with the xctestrun file. Specifically using Swift the test_filters do not work unless the module names line up with the test target name. Guessing because Swift's full class names are `MODULE.CLASS` so they do the magic under the hood. + +diff --git a/apple/internal/rule_factory.bzl b/apple/internal/rule_factory.bzl +index 33d4312f..b196a577 100644 +--- a/apple/internal/rule_factory.bzl ++++ b/apple/internal/rule_factory.bzl +@@ -205,6 +205,12 @@ AppleTestRunnerInfo provider. + "test_filter": attr.string( + doc = """ + Test filter string that will be passed into the test runner to select which tests will run. ++""", ++ default = "", ++ ), ++ "module_name": attr.string( ++ doc = """ ++Module name of the underlying test to be run. + """, + default = "", + ), +diff --git a/apple/internal/testing/apple_test_rule_support.bzl b/apple/internal/testing/apple_test_rule_support.bzl +index 8392862a..49ff1f65 100644 +--- a/apple/internal/testing/apple_test_rule_support.bzl ++++ b/apple/internal/testing/apple_test_rule_support.bzl +@@ -102,7 +102,7 @@ This aspect propagates a `CoverageFilesInfo` provider. + implementation = _coverage_files_aspect_impl, + ) + +-def _get_template_substitutions(test_type, test_bundle, test_environment, test_host = None, test_filter = None): ++def _get_template_substitutions(test_type, test_bundle, test_environment, test_host = None, test_filter = None, product_module_name = None): + """Dictionary with the substitutions to be applied to the template script.""" + subs = {} + +@@ -114,6 +114,7 @@ def _get_template_substitutions(test_type, test_bundle, test_environment, test_h + subs["test_type"] = test_type.upper() + subs["test_env"] = ",".join([k + "=" + v for (k, v) in test_environment.items()]) + subs["test_filter"] = test_filter or "" ++ subs["product_module_name"] = product_module_name or "" + + return {"%(" + k + ")s": subs[k] for k in subs} + +@@ -176,6 +177,7 @@ def _apple_test_rule_impl(ctx, test_type): + test_environment, + test_host = test_host_archive, + test_filter = ctx.attr.test_filter, ++ product_module_name = ctx.attr.module_name, + ), + is_executable = True, + ) +diff --git a/apple/testing/default_runner/ios_test_runner.template.sh b/apple/testing/default_runner/ios_test_runner.template.sh +index 0804007c..0fbf190f 100644 +--- a/apple/testing/default_runner/ios_test_runner.template.sh ++++ b/apple/testing/default_runner/ios_test_runner.template.sh +@@ -52,6 +52,11 @@ TMP_DIR="$(mktemp -d "${TMPDIR:-/tmp}/test_runner_work_dir.XXXXXX")" + trap 'rm -rf "${TMP_DIR}"' ERR EXIT + runner_flags+=("--work_dir=${TMP_DIR}") + ++PRODUCT_MODULE_NAME="%(product_module_name)s" ++if [[ -n "$PRODUCT_MODULE_NAME" ]]; then ++ runner_flags+=("--product_module_name=${PRODUCT_MODULE_NAME}") ++fi ++ + TEST_BUNDLE_PATH="%(test_bundle_path)s" + + if [[ "$TEST_BUNDLE_PATH" == *.xctest ]]; then +diff --git a/test/ios_test_runner_unit_test.sh b/test/ios_test_runner_unit_test.sh +index 64002d38..55fb26ee 100755 +--- a/test/ios_test_runner_unit_test.sh ++++ b/test/ios_test_runner_unit_test.sh +@@ -201,6 +201,7 @@ objc_library( + + ios_unit_test( + name = "PassingUnitTest", ++ module_name = "pass_unit_test_lib", + infoplists = ["PassUnitTest-Info.plist"], + deps = [":pass_unit_test_lib"], + minimum_os_version = "9.0", +-- +2.36.1 + diff --git a/patches/xctestrunner_module_name.patch b/patches/xctestrunner_module_name.patch new file mode 100644 index 00000000..aaeb3e3a --- /dev/null +++ b/patches/xctestrunner_module_name.patch @@ -0,0 +1,147 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Derek Ostrander +Date: Fri, 9 Dec 2022 13:34:25 -0500 +Subject: =?UTF-8?q?Add=20optional=20parameter=20of=20product=5Fmodule=5Fna?= + =?UTF-8?q?me=0AThis=20will=20be=20forwarded=20into=20the=20xctestrun=20fi?= + =?UTF-8?q?le=20in=20the=20test=20name=20doesn't=20accurately=20reflect=20?= + =?UTF-8?q?the=20module=20underneath?= + + +diff --git a/test_runner/ios_test_runner.py b/test_runner/ios_test_runner.py +index c85800d..b4b67a3 100644 +--- a/test_runner/ios_test_runner.py ++++ b/test_runner/ios_test_runner.py +@@ -117,7 +117,8 @@ def _AddPrepareSubParser(subparsers): + test_bundle=args.test_bundle_path, + xctestrun_file_path=args.xctestrun, + test_type=args.test_type, +- signing_options=_GetJson(args.signing_options_json_path)) ++ signing_options=_GetJson(args.signing_options_json_path), ++ product_module_name=args.product_module_name) + session.SetLaunchOptions(_GetJson(args.launch_options_json_path)) + + test_parser = subparsers.add_parser( +@@ -153,7 +154,8 @@ def _AddTestSubParser(subparsers): + test_bundle=args.test_bundle_path, + xctestrun_file_path=args.xctestrun, + test_type=args.test_type, +- signing_options=_GetJson(args.signing_options_json_path)) ++ signing_options=_GetJson(args.signing_options_json_path), ++ product_module_name=args.product_module_name) + session.SetLaunchOptions(_GetJson(args.launch_options_json_path)) + return session.RunTest(args.id) + +@@ -172,6 +174,11 @@ def _AddTestSubParser(subparsers): + help='The platform of the device. The value can be ios_device or ' + 'ios_simulator.' + ) ++ optional_arguments.add_argument( ++ '--product_module_name', ++ help='The product module name that will be set in the xctestrun file.' ++ ) ++ + test_parser.set_defaults(func=_Test) + + +@@ -197,7 +204,8 @@ def _AddSimulatorTestSubParser(subparsers): + test_bundle=args.test_bundle_path, + xctestrun_file_path=args.xctestrun, + test_type=args.test_type, +- signing_options=_GetJson(args.signing_options_json_path)) ++ signing_options=_GetJson(args.signing_options_json_path), ++ product_module_name=args.product_module_name) + session.SetLaunchOptions(_GetJson(args.launch_options_json_path)) + if not hostless: + try: +diff --git a/test_runner/xctest_session.py b/test_runner/xctest_session.py +index 1e879db..1e08e70 100644 +--- a/test_runner/xctest_session.py ++++ b/test_runner/xctest_session.py +@@ -78,7 +78,8 @@ class XctestSession(object): + # TODO(albertdai): Support bundle id as the value of app_under_test and + # test_bundle. + def Prepare(self, app_under_test=None, test_bundle=None, +- xctestrun_file_path=None, test_type=None, signing_options=None): ++ xctestrun_file_path=None, test_type=None, ++ signing_options=None, product_module_name=None): + """Prepares the test session. + + If xctestrun_file is not provided, will use app under test and test bundle +@@ -94,6 +95,8 @@ class XctestSession(object): + test_type: ios_constants.TestType. The type of test bundle. + signing_options: dict, the signing app options. See + ios_constants.SIGNING_OPTIONS_JSON_HELP for details. ++ product_module_name: string, the name of the module that is being tested. ++ This will be forwarded into the xctestrun file. + + Raises: + ios_errors.IllegalArgumentError: +@@ -141,7 +144,7 @@ class XctestSession(object): + if test_type != ios_constants.TestType.LOGIC_TEST: + xctestrun_factory = xctestrun.XctestRunFactory( + app_under_test_dir, test_bundle_dir, self._sdk, self._device_arch, +- test_type, signing_options, self._work_dir) ++ test_type, signing_options, self._work_dir, product_module_name) + self._xctestrun_obj = xctestrun_factory.GenerateXctestrun() + else: + self._logic_test_bundle = test_bundle_dir +diff --git a/test_runner/xctestrun.py b/test_runner/xctestrun.py +index 4390489..f1c2fe1 100644 +--- a/test_runner/xctestrun.py ++++ b/test_runner/xctestrun.py +@@ -275,7 +275,8 @@ class XctestRunFactory(object): + sdk=ios_constants.SDK.IPHONESIMULATOR, + device_arch=ios_constants.ARCH.X86_64, + test_type=ios_constants.TestType.XCUITEST, +- signing_options=None, work_dir=None): ++ signing_options=None, work_dir=None, ++ product_module_name=None): + """Initializes the XctestRun object. + + If arg work_dir is provided, the original app under test file and test +@@ -292,6 +293,7 @@ class XctestRunFactory(object): + signing_options: dict, the signing app options. See + ios_constants.SIGNING_OPTIONS_JSON_HELP for details. + work_dir: string, work directory which contains run files. ++ product_module_name: string, forwarded into the xctestrun. + + Raises: + IllegalArgumentError: when the sdk or test type is not supported. +@@ -302,6 +304,7 @@ class XctestRunFactory(object): + self._sdk = sdk + self._device_arch = device_arch + self._test_type = test_type ++ self._product_module_name = product_module_name + if self._sdk == ios_constants.SDK.IPHONEOS: + self._on_device = True + self._signing_options = signing_options +@@ -499,7 +502,7 @@ class XctestRunFactory(object): + 'DYLD_LIBRARY_PATH': '__TESTROOT__:%s/usr/lib' % developer_path + } + self._xctestrun_dict = { +- 'ProductModuleName': self._test_name.replace("-", "_"), ++ 'ProductModuleName': self._product_module_name or self._test_name.replace("-", "_"), + 'IsUITestBundle': True, + 'SystemAttachmentLifetime': 'keepNever', + 'TestBundlePath': self._test_bundle_dir, +@@ -666,7 +669,7 @@ class XctestRunFactory(object): + 'DYLD_LIBRARY_PATH': '__TESTROOT__:%s/usr/lib:' % developer_path + } + self._xctestrun_dict = { +- 'ProductModuleName': self._test_name.replace("-", "_"), ++ 'ProductModuleName': self._product_module_name or self._test_name.replace("-", "_"), + 'TestHostPath': self._app_under_test_dir, + 'TestBundlePath': self._test_bundle_dir, + 'IsAppHostedTestBundle': True, +@@ -687,7 +690,7 @@ class XctestRunFactory(object): + 'DYLD_LIBRARY_PATH': dyld_framework_path + } + self._xctestrun_dict = { +- 'ProductModuleName': self._test_name.replace("-", "_"), ++ 'ProductModuleName': self._product_module_name or self._test_name.replace("-", "_"), + 'TestBundlePath': self._test_bundle_dir, + 'TestHostPath': xcode_info_util.GetXctestToolPath(self._sdk), + 'TestingEnvironmentVariables': test_envs, +-- +2.36.1 + diff --git a/rules/repositories.bzl b/rules/repositories.bzl index 7c9184cc..4313f307 100644 --- a/rules/repositories.bzl +++ b/rules/repositories.bzl @@ -65,6 +65,8 @@ def rules_ios_dependencies(): ref = "f99c3cb7e472ecd68b81ea8dab97609a4b75db06", project = "bazelbuild", repo = "rules_apple", + patch_args = ["-p1"], + patches = ["@build_bazel_rules_ios//patches:rules_apple_module_name.patch"], sha256 = "5e82a98a591efda772a5ee96ed17bcad38338aafeba6055daab04a5d6c13ea50", ) @@ -149,3 +151,15 @@ swift_binary( ) xcbuildkit_dependencies() + + ## We want to explcitily override this dependency to add our own patches to it. This will be updated when rules_apple is updated. + http_archive( + name = "xctestrunner", + urls = [ + "https://github.com/google/xctestrunner/archive/e0bc4b29976cf000794e9e796cb8a584b0c443bc.tar.gz", + ], + patch_args = ["-p1"], + patches = ["@build_bazel_rules_ios//patches:xctestrunner_module_name.patch"], + strip_prefix = "xctestrunner-e0bc4b29976cf000794e9e796cb8a584b0c443bc", + sha256 = "6cd157ae7523d024eeb7ec05811979e9c191597f061a80244041374e10ebca13", + ) diff --git a/rules/test.bzl b/rules/test.bzl index a49b6d94..f5f017df 100644 --- a/rules/test.bzl +++ b/rules/test.bzl @@ -77,6 +77,10 @@ def _ios_test(name, test_rule, test_suite_rule, apple_library, infoplists_by_bui xcconfig_by_build_setting = kwargs.get("xcconfig_by_build_setting", {}), ) + ## We can't just put this in _IOS_TEST_KWARGS as this is also used in the kwargs + ## of apple_library. + module_name = kwargs.get("module_name") + if split_name_to_kwargs and len(split_name_to_kwargs) > 0: tests = [] for suffix, split_kwargs in split_name_to_kwargs.items(): @@ -105,6 +109,7 @@ def _ios_test(name, test_rule, test_suite_rule, apple_library, infoplists_by_bui tests.append(test_name) split_rule( name = test_name, + module_name = module_name, deps = [dep_name], frameworks = frameworks, testonly = testonly, @@ -124,6 +129,7 @@ def _ios_test(name, test_rule, test_suite_rule, apple_library, infoplists_by_bui rule( name = name, + module_name = module_name, deps = [dep_name], frameworks = frameworks, infoplists = select(infoplists),