run_chrome_public_test_apk should attempt to rebuild dependent targets when run locally |
||
Issue descriptionRepro: 1. Have some code that fails tests running on a device 2. Fix the code to pass the test 3. Run run_chrome_public_test_apk -f "ExternalNavigationDelegateImplTest.*", for example, to check that tests have been fixed Expected: A new chrome_public_apk is pushed to the device by the test script Actual: Tests run on the old code and continue to fail
,
Dec 6 2016
GN args: use_goma = true target_cpu = "arm" is_component_build = true is_debug = true goma_dir = "/usr/local/google/goma/goma" target_os = "android" $ cr build chrome to build (this builds chrome_apk, not chrome_public_apk, maybe that's the issue?) Then this to test: $ ./out_android/Debug/bin/run_chrome_public_test_apk -v -f "ExternalNavigationDelegateImplTest.*"
,
Dec 6 2016
cr build chrome could definitely be the issue, though I admittedly know very little about cr. - does cr use out_android (or do you have it configured to do so)? - What are the timestamps on the contents of out_android/Debug/apks/?
,
Dec 6 2016
Yes, cr uses out_android by default and sets the out dir variables accordingly I am guessing maybe the issue is that out_android/Debug/apks might not even have ChromePublic.apk at all since I haven't built it. What happens in that case?
,
Dec 6 2016
That definitely *should* die with an error, but I'm not sure that that's the case.
,
Dec 6 2016
Die with an error would be an improvement. But ideally, build and push apk would be even better.
,
Dec 6 2016
Building is currently outside of the scope of the test runner, but that's both an interesting and (I think) feasible idea.
,
Dec 6 2016
I checked, and it does fail with an error if the APK doesn't exist, but it's not very clear. I've uploaded https://codereview.chromium.org/2552033003/ to clarify the error message. You're sure that ChromePublic.apk/ChromePublicTest.apk don't exist in that directory? We build the runner script (here, run_chrome_public_test_apk) as part of the test apk target, so if the runner exists, both the test apk and the apk under test should also exist.
,
Dec 6 2016
No, I am not actually sure whether they existed or not. They definitely exist now. I can try to reproduce again and check.
,
Dec 6 2016
$ ls -l out_android/Debug/apks -rw-r----- 1 mariakhomenko eng 6084 Nov 30 14:31 ApkWithWebApkService.apk -rw-r----- 1 mariakhomenko eng 84317762 Dec 6 10:47 Chrome.apk -rw-r----- 1 mariakhomenko eng 84158867 Dec 5 16:09 ChromePublic.apk -rw-r----- 1 mariakhomenko eng 1821205 Dec 5 14:46 ChromePublicTest.apk -rw-r----- 1 mariakhomenko eng 2481339 Dec 2 16:46 ChromePublicTestSupport.apk -rw-r----- 1 mariakhomenko eng 257409 Sep 28 11:23 ChromeTest.apk -rw-r----- 1 mariakhomenko eng 12050386 Dec 2 16:46 ChromiumNetTestSupport.apk -rw-r----- 1 mariakhomenko eng 8227614 Nov 28 11:01 CustomTabsClientExample.apk -rw-r----- 1 mariakhomenko eng 93803 Dec 2 16:46 DexOptimizer.apk -rw-r----- 1 mariakhomenko eng 117730 Sep 28 11:08 OnDeviceInstrumentationDriver.apk Then edit source code (ExternalNavigationDelegateImpl.java) such that tests would fail. Then run $ ./out_android/Debug/bin/run_chrome_public_test_apk -f "ExternalNavigationDelegateImplTest.*" All tests pass, even though they should fail. $ ls -l out_android/Debug/apks total 189016 -rw-r----- 1 mariakhomenko eng 6084 Nov 30 14:31 ApkWithWebApkService.apk -rw-r----- 1 mariakhomenko eng 84317762 Dec 6 10:47 Chrome.apk -rw-r----- 1 mariakhomenko eng 84158867 Dec 5 16:09 ChromePublic.apk -rw-r----- 1 mariakhomenko eng 1821205 Dec 5 14:46 ChromePublicTest.apk -rw-r----- 1 mariakhomenko eng 2481339 Dec 2 16:46 ChromePublicTestSupport.apk -rw-r----- 1 mariakhomenko eng 257409 Sep 28 11:23 ChromeTest.apk -rw-r----- 1 mariakhomenko eng 12050386 Dec 2 16:46 ChromiumNetTestSupport.apk -rw-r----- 1 mariakhomenko eng 8227614 Nov 28 11:01 CustomTabsClientExample.apk -rw-r----- 1 mariakhomenko eng 93803 Dec 2 16:46 DexOptimizer.apk -rw-r----- 1 mariakhomenko eng 117730 Sep 28 11:08 OnDeviceInstrumentationDriver.apk Question: is the issue that I am not rebuilding run_chrome_public_test_apk in the middle? I wouldn't have thought to need to do that... $ ls -l out_android/Debug/bin/ total 16 drwxr-x--- 2 mariakhomenko eng 4096 Nov 23 14:26 helper -rwxr-x--- 1 mariakhomenko eng 1264 Dec 5 16:08 run_chrome_public_test_apk -rwxr-x--- 1 mariakhomenko eng 1041 Sep 28 11:10 run_chrome_test_apk -rwxr-x--- 1 mariakhomenko eng 881 Nov 15 12:08 run_components_unittests
,
Dec 6 2016
No, the issue is that you're not rebuilding chrome_public_test_apk in the middle. We don't attempt to reinstall the APK because the APK hasn't changed.
,
Dec 6 2016
It also didn't occur to me to rebuild a test apk because I didn't change any test code.
,
Dec 6 2016
er, right, sorry. chrome_public_apk, not chrome_public_test_apk.
,
Dec 6 2016
Yep, that's fair. I verified that if I did rebuild chrome_public_apk, then it's pushed. What happened is that I built chrome_apk instead of chrome_public_apk because that's what I usually build and hence the confusion.
,
Dec 6 2016
Yeah, and that's understandable. I think the best way to avoid that confusion may be to do what you suggested in #6, so I'm updating this bug to reflect that. We'll have to be careful about how it behaves on bots, though. tentatively assigning to mikecase, who did the wrapper scripts originally.
,
Dec 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0d0973803e7802215f26802941eb4bb2f0c97037 commit 0d0973803e7802215f26802941eb4bb2f0c97037 Author: catapult-deps-roller <catapult-deps-roller@chromium.org> Date: Tue Dec 06 20:28:23 2016 Roll src/third_party/catapult/ 9902da68b..cb066006a (1 commit). https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/9902da68b6de..cb066006a85e $ git log 9902da68b..cb066006a --date=short --no-merges --format='%ad %ae %s' 2016-12-06 jbudorick [devil] Clarify DeviceUtils.Install{,SplitApk} failure on missing APK. BUG=671413 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=catapult-sheriff@chromium.org Review-Url: https://codereview.chromium.org/2554993002 Cr-Commit-Position: refs/heads/master@{#436694} [modify] https://crrev.com/0d0973803e7802215f26802941eb4bb2f0c97037/DEPS |
||
►
Sign in to add a comment |
||
Comment 1 by jbudorick@chromium.org
, Dec 6 2016Labels: -Pri-3 Pri-2