FTL: Support providing a --test-targets filter
#403
Conversation
spencertransier
left a comment
spencertransier
left a comment
There was a problem hiding this comment.
Approved with just a curious question from me about one of the changes 🙂
| }.compact.flat_map { |k, v| ["--#{k}", v] } | ||
| command = Shellwords.join(['gcloud', 'firebase', 'test', 'android', 'run', *params]) |
There was a problem hiding this comment.
@AliSoftware Can you explain the benefits of switching to this syntax from the previous syntax? I'm not questioning it, just curious 😃
There was a problem hiding this comment.
This syntax allowed me to inject the compact call on the Hash before transforming it into an array of key-value-pairs.
The effect of that .compact is to remove any entry from the Hash with a nil value, which was necessary to completely remove --test-targets <value> from the params if the user didn't provide any value for it. Otherwise we'd be passing --test-targets "" (or actually even worse, just --test-targets without a corresponding value next to it) to the gcloud command, instead of skipping that param entirely when it's not provided by the user.
There was a problem hiding this comment.
Makes sense, thank you for explaining!
|
Thank you for this PR @AliSoftware! ⭐ |
# Conflicts: # CHANGELOG.md
Co-authored-by: Gio Lodi <[email protected]>
Why?
After migrating the code able to trigger Firebase Test Lab from CircleCI and our related CircleCI orb to move it to a fastlane action in release-toolkit during the Buildkite migration, we forgot to add an option to filter the test targets, like we had and used to use in e.g. WordPress Android to exclude screenshots from the instrumented/connected tests
How
Add an optional
test_targetsparameter (akaConfigItem) to theandroid_firebase_testaction which is then passed to the invocation ofgcloudTo test
The unit tests (
spec/firebase_test_runner_spec.rb) have been updated to cover for cases with or without atest_targetsparameter being provided, and for testing the proper escaping of the filter if one is provided.So if CI is green, it should be all good to go.
What's next
To workaround the fact that we couldn't filter the test targets during instrumented tests, @pachlava had to temporarily
@Ignorethe Screenshot tests in WordPress Android (wordpress-mobile/WordPress-Android#16970).Once this lands, we should then:
release-toolkitandroid_firebase_testinbuild_and_run_instrumented_teststo addtest_targets: 'notPackage org.wordpress.android.ui.screenshots'to the parametersScreenshotTestWordPress-Android#16970 to un-ignore screenshot tests (and thus be able to progress on the work on Android screenshots)