Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unit test attr forwarding #927

Merged
merged 1 commit into from
Nov 18, 2024
Merged

Fix unit test attr forwarding #927

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions rules/test.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ _IOS_TEST_KWARGS = [
"bundle_id",
"bundle_name",
"env",
"features",
"flaky",
"frameworks",
"infoplists",
Expand All @@ -34,7 +33,6 @@ _APPLE_BUNDLE_ATTRS = {
"bundle_id",
"bundle_name",
"families",
"features",
"frameworks",
"infoplists",
"linkopts",
Expand All @@ -45,6 +43,22 @@ _APPLE_BUNDLE_ATTRS = {
]
}

# See: https://bazel.build/reference/be/common-definitions#common-attributes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if bazel's newer version contains new common attribute, we need to update this list right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally yeah, however most of these attrs are unused

# NOTE: `testonly` arent listed because we have custom logic for them in here.
_ALWAYS_AVAILABLE_ATTRS = [
"compatible_with",
"deprecation",
"distribs",
"exec_compatible_with",
"exec_properties",
"features",
"restricted_to",
"tags",
"target_compatible_with",
"toolchains",
"visibility",
]

_DEFAULT_APPLE_TEST_RUNNER = "@build_bazel_rules_apple//apple/testing/default_runner:ios_default_runner"

def _make_runner_split(name, runner, **in_split):
Expand Down Expand Up @@ -104,7 +118,7 @@ def _make_test(name, test_rule, **kwargs):
Helper to create an individual test
"""
runner = kwargs.pop("runner", None) or _DEFAULT_APPLE_TEST_RUNNER
test_attrs = {k: v for (k, v) in kwargs.items() if k not in _APPLE_BUNDLE_ATTRS}
test_attrs = {k: v for (k, v) in kwargs.items() if (k not in _APPLE_BUNDLE_ATTRS) or (k in _ALWAYS_AVAILABLE_ATTRS)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when downstream project point to this commit, probably worth printing out what is dropped before VS after, to see if anything other than features are dropped also

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean adding a print here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but do it temp. during the testing


test_rule(
name = name,
Expand Down Expand Up @@ -159,7 +173,7 @@ def _ios_test(name, bundle_rule, test_rule, test_factory, apple_library, infopli
"""

testonly = kwargs.pop("testonly", True)
ios_test_kwargs = {arg: kwargs.pop(arg) for arg in _IOS_TEST_KWARGS if arg in kwargs}
ios_test_kwargs = {arg: kwargs.pop(arg) for arg in (_IOS_TEST_KWARGS + _ALWAYS_AVAILABLE_ATTRS) if arg in kwargs}
ios_test_kwargs["data"] = kwargs.pop("test_data", [])

test_exec_properties = kwargs.pop("test_exec_properties", None)
Expand Down Expand Up @@ -218,7 +232,7 @@ def _ios_test(name, bundle_rule, test_rule, test_factory, apple_library, infopli

# 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_attrs = {k: v for (k, v) in ios_test_kwargs.items() if (k in _APPLE_BUNDLE_ATTRS) or (k in _ALWAYS_AVAILABLE_ATTRS)}
bundle_name = bundle_attrs.pop("bundle_name", name)
bundle_rule(
name = test_bundle_name,
Expand Down