Add module name to the testrunner #1785
Conversation
dostrander
requested review from
BalestraPatrick,
brentleyjones,
keith,
segiddins and
thii
as code owners
|
can you throw a value for this attr into one of the test targets so that it actually gets caught by CI? also note that this has to hit master before this branch, we won't ever merge this branch back into master |
Will do. Also open to possibly changing this name to |
dostrander
force-pushed
the
dostrander/add-product-module-name
branch
2 times, most recently
from
38b3efa to
abc1a78
Compare
| @@ -115,6 +115,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 "" | |||
There was a problem hiding this comment.
Doesn't the test_filter address each test by the PRODUCT_MODULE_NAME? Perhaps can you parse this from test_filter if it it exists. The thing I liked about this is being able to filter different module names. Say you've got a number of modules in the test:
bazel test :MyTest --test_filters MyFirst.swiftmodule:foo
bazel test :MyTest --test_filters MySecond.swiftmodule:bar
Then - inside of xctestrunner - you'd simply set PRODUCT_MODULE in the xctestrun based on the filter - and be sure to get the right one. Probably validate they set that..
The downside of adding this at the rule level is you can't filter dynamically like that.
PS : I wonder if this is a regression - or perhaps test_filter only ever workd for objc!
There was a problem hiding this comment.
Also do you imagine that we'd ever want to feed .xctestrun ( or partials ) into this rule? That might be another thing to contend with - similar our plist composition stuff in rules_ios you might be able to pass overrides.
I would still question if you can leverage the build graph too: assuming that you pack ( filterable ) tests into the first .swiftmodule file - you can set the PRODUCT_MODULE_NAME based on the first direct SwiftInfo
Obviously these are 2 different solutions. I liked the dep graph way - and might be a nice default - it has some inflexibility but it's probably easier for 99% of tests out there
There was a problem hiding this comment.
Hey PTAL again i've pivoted to using the swiftmodule's you were talking about here
There was a problem hiding this comment.
Ah cool glad it works - this makes Bazel easier to use in general IMO
dostrander
force-pushed
the
dostrander/add-product-module-name
branch
from
abc1a78 to
a00c1af
Compare
test/ios_test_runner_unit_test.sh
Outdated
| @@ -209,6 +209,7 @@ objc_library( | |||
|
|
|||
| ios_unit_test( | |||
| name = "PassingUnitTest", | |||
| module_name = "pass_unit_test_lib", | |||
There was a problem hiding this comment.
I imagine we should remove this attribute now
| @@ -135,6 +141,11 @@ def _apple_test_rule_impl(ctx, test_type): | |||
| test_bundle_target = ctx.attr.deps[0] | |||
|
|
|||
| test_bundle = test_bundle_target[AppleTestInfo].test_bundle | |||
| swift_module_list = test_bundle_target[AppleTestInfo].swift_modules.to_list() | |||
|
|
|||
| test_swift_module_file = swift_module_list[len(swift_module_list) - 1] | |||
There was a problem hiding this comment.
Should this do a lenght check - if there is less than 1 swiftmodule it's out of bounds?
| @@ -115,6 +115,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 "" | |||
There was a problem hiding this comment.
Ah cool glad it works - this makes Bazel easier to use in general IMO
Unless the xctestrun specifies a product module name some feature such as filtering tests will not work in Swift files.
dostrander
force-pushed
the
dostrander/add-product-module-name
branch
from
a00c1af to
c1e7efa
Compare
dostrander
changed the title
* Grab module name from swiftmodule files (#1) bazelbuild#1785 * Use bazel-ios/xctestrunner (#2)
* Grab module name from swiftmodule files (#1) bazelbuild#1785 * Use bazel-ios/xctestrunner (#2)
|
@dostrander bump - I think there's a few test failures that would prevent us from landing this re: bazel-ios/rules_ios#663 |
This can't realyl be merged until google/xctestrunner#50 is merged unless we do a patch |
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.CLASSso they do the magic under the hood.We'll likely need to wait until google/xctestrunner#50 is merged