-
Notifications
You must be signed in to change notification settings - Fork 119
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.
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
Add additional-app-test-apks support #542
Conversation
09c92a7
to
44d0b39
Compare
Codecov Report
@@ Coverage Diff @@
## master #542 +/- ##
============================================
+ Coverage 79.4% 79.45% +0.04%
- Complexity 566 576 +10
============================================
Files 77 79 +2
Lines 2122 2146 +24
Branches 279 277 -2
============================================
+ Hits 1685 1705 +20
+ Misses 266 263 -3
- Partials 171 178 +7 |
44d0b39
to
f1ca31e
Compare
## Include additional app/test apk pairs in the run. If app is omitted, then the top level app is used for that pair. | ||
# additional-app-test-apks: | ||
# - app: ../test_app/apks/app-debug.apk | ||
# test: ../test_app/apks/app-debug-androidTest.apk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you thought of making test
a list, so you don't have a bunch of usages without app
?
Something like
- app: ../app.apk
test:
- testapk1.apk
- testapk2.apk
- app: app2.apk
test:
- testfinal.apk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prototyped adding list support, in particular this complicated the code when parsing from CLI.
I think for the first version of this feature, the more verbose format is easier to maintain. The test run loop also becomes more complex as we have yet another list to iterate through.
- app: ../app.apk
test: test1.apk
- app: ../app.apk
test: test2.apk
I'm open to revisiting this based on feedback in the future.
Did firebase add support for multiple test apks? |
Nope. We talked with the FTL team and this isn't a roadmap priority for them. I think modules on Android are a good use case for this feature. I've been working with American Express to add support for this use case directly in Flank to unblock them. |
Haven't gone through the code, but are you merging all test apks into a single one? |
Previously flank only allowed one test/app apk. In the new design, Flank accepts an array of test/app apks. The apks aren't merged. If you have one app and a dozen test apks for that single app, now you can get the results within a single flank run. |
Oh of course. You can just trigger multiple test lab runs, each with a different test APK. |
I tested these changes locally, and flank is not working as expected. Most of the shard results are reporting no tests, and I don't think it is an issue with my apks because I ran all additional apks individually with the current version and the run yields the right results. <?xml version='1.0' encoding='UTF-8' ?>
<testsuite name="" tests="1" failures="1" errors="0" skipped="0" time="0.544" timestamp="2019-04-23T17:16:05" hostname="localhost">
<properties />
<testcase name="" classname="no tests found" time="0.0">
<failure>java.lang.ClassNotFoundException: Invalid name: no tests found
at java.lang.Class.classForName(Native Method)
at java.lang.Class.forName(Class.java:324)
at androidx.test.internal.runner.TestLoader.doCreateRunner(TestLoader.java:72)
at androidx.test.internal.runner.TestLoader.getRunnersFor(TestLoader.java:104)
at androidx.test.internal.runner.TestRequestBuilder.build(TestRequestBuilder.java:789)
at androidx.test.runner.AndroidJUnitRunner.buildRequest(AndroidJUnitRunner.java:543)
at androidx.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:386)
at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1879)
</failure>
</testcase>
</testsuite> |
Thanks. I'll look into it |
f1ca31e
to
74021e9
Compare
I'm unable to reproduce the failure using my apks. Pinged on Slack with more details on how to debug the problem. |
@bootstraponline I found the problem. The Matrix is receiving test targets that are not part of the test apk. |
Fix #511