Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2409973002:
chromium.android: Fully swarm Lollipop Phone tester (Closed)
Descriptionchromium.android: Fully swarm Lollipop Phone tester
BUG=654047
Committed: https://crrev.com/9d888f7681394f3a0f5341b4206516624b3d1e66
Cr-Commit-Position: refs/heads/master@{#424988}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Review #Patch Set 3 : Move instrumentation tests to gtests #
Total comments: 6
Patch Set 4 : Review #Patch Set 5 : Actually delete the right things this time #Patch Set 6 : JSON fix #Messages
Total messages: 25 (8 generated)
hinoka@chromium.org changed reviewers: + bpastene@chromium.org, stip@chromium.org
based off of https://codereview.chromium.org/2338353002 holy crap this is wordy
yeah, I have plans on how to compress this with a generator but I haven't gotten to it yet :) lgtm. looking at https://chromium-swarm.appspot.com/restricted/bots?dimensions=os%3AAndroid%0D... we actually have one bot with hammerheads on LMY48I. unfortunately it only has one device on it. so you could technically commit this now, but it will break since the tests will run ~5x slower and hit the hard timeouts we have specified. I'd recommend holding off until we move build41-b1 onto the swarming pool. we should also specify android_devices:4 to make sure it gets scheduled on that bot
lgtm, but I'm a little worried that the triggered swarming tasks might expire if they all get run on the same swarming bot (ignoring dedupe) Might need to either stagger the triggering or maybe throw some more capacity into the L-phone pool with the extra hardware we've got coming https://codereview.chromium.org/2409973002/diff/1/testing/buildbot/chromium.a... File testing/buildbot/chromium.android.json (left): https://codereview.chromium.org/2409973002/diff/1/testing/buildbot/chromium.a... testing/buildbot/chromium.android.json:2908: "instrumentation_tests": [ Still need these ones.
https://codereview.chromium.org/2409973002/diff/1/testing/buildbot/chromium.a... File testing/buildbot/chromium.android.json (left): https://codereview.chromium.org/2409973002/diff/1/testing/buildbot/chromium.a... testing/buildbot/chromium.android.json:2908: "instrumentation_tests": [ On 2016/10/11 19:06:36, bpastene wrote: > Still need these ones. They were removed in the reference CL. Doesn't this mean we still need to run the buildbot on a machine with a device then?
https://codereview.chromium.org/2409973002/diff/1/testing/buildbot/chromium.a... File testing/buildbot/chromium.android.json (left): https://codereview.chromium.org/2409973002/diff/1/testing/buildbot/chromium.a... testing/buildbot/chromium.android.json:2908: "instrumentation_tests": [ On 2016/10/12 02:26:49, hinoka wrote: > On 2016/10/11 19:06:36, bpastene wrote: > > Still need these ones. > > They were removed in the reference CL. Doesn't this mean we still need to run > the buildbot on a machine with a device then? Ahh, sorry; I should have been more clear. When swarmed, these instrumentation tests can be treated as gtests (big thanks to stip for unifying that.) If you look at the reference CL, they weren't dropped but moved up to the list of gtest_tests with the relevant swarming bits. (probably would have been easier to spot if there wasn't so much damn boiler plate)
blerg, I thought I had posted this but forgot to hit 'publish' check out where android_webview_test_apk is in Android Tests https://codereview.chromium.org/2409973002/diff/1/testing/buildbot/chromium.a... File testing/buildbot/chromium.android.json (left): https://codereview.chromium.org/2409973002/diff/1/testing/buildbot/chromium.a... testing/buildbot/chromium.android.json:2908: "instrumentation_tests": [ On 2016/10/11 19:06:36, bpastene wrote: > Still need these ones. good catch, thought those were reflected above. ryan, just add these into the gtest list above, see Android Tests in https://chromium.googlesource.com/chromium/src/+/master/testing/buildbot/chro... for example
Aha, that makes a lot more sense, done.
lgtm w/ a small change and a nit https://codereview.chromium.org/2409973002/diff/40001/testing/buildbot/chromi... File testing/buildbot/chromium.android.json (right): https://codereview.chromium.org/2409973002/diff/40001/testing/buildbot/chromi... testing/buildbot/chromium.android.json:3633: "override_isolate_target": "android_webview_test_apk", nit: alphabetize. however, this should be automatic if you run `testing/buildbot/manage.py -w` upload should have yelled at you to do this lol https://codereview.chromium.org/2409973002/diff/40001/testing/buildbot/chromi... testing/buildbot/chromium.android.json:3665: "apk_under_test": "Blimp.apk", you can delete the apk_under_test line https://codereview.chromium.org/2409973002/diff/40001/testing/buildbot/chromi... testing/buildbot/chromium.android.json:3699: "test_apk": "BlimpTest.apk" you can delete test_apk as well
https://codereview.chromium.org/2409973002/diff/40001/testing/buildbot/chromi... File testing/buildbot/chromium.android.json (right): https://codereview.chromium.org/2409973002/diff/40001/testing/buildbot/chromi... testing/buildbot/chromium.android.json:3633: "override_isolate_target": "android_webview_test_apk", On 2016/10/13 07:26:26, stip wrote: > nit: alphabetize. however, this should be automatic if you run > `testing/buildbot/manage.py -w` > > upload should have yelled at you to do this lol ran manage.py, didn't sort anything...? https://codereview.chromium.org/2409973002/diff/40001/testing/buildbot/chromi... testing/buildbot/chromium.android.json:3665: "apk_under_test": "Blimp.apk", On 2016/10/13 07:26:26, stip wrote: > you can delete the apk_under_test line Done. https://codereview.chromium.org/2409973002/diff/40001/testing/buildbot/chromi... testing/buildbot/chromium.android.json:3699: "test_apk": "BlimpTest.apk" On 2016/10/13 07:26:26, stip wrote: > you can delete test_apk as well Done.
hinoka@chromium.org changed reviewers: + bauerb@chromium.org
+cc emea android sheriff fyi
The CQ bit was checked by hinoka@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bpastene@chromium.org, stip@chromium.org Link to the patchset: https://codereview.chromium.org/2409973002/#ps80001 (title: "Actually delete the right things this time")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by hinoka@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bpastene@chromium.org, stip@chromium.org Link to the patchset: https://codereview.chromium.org/2409973002/#ps100001 (title: "JSON fix")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== chromium.android: Fully swarm Lollipop Phone tester BUG=654047 ========== to ========== chromium.android: Fully swarm Lollipop Phone tester BUG=654047 Committed: https://crrev.com/9d888f7681394f3a0f5341b4206516624b3d1e66 Cr-Commit-Position: refs/heads/master@{#424988} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/9d888f7681394f3a0f5341b4206516624b3d1e66 Cr-Commit-Position: refs/heads/master@{#424988}
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:100001) has been created in https://codereview.chromium.org/2419583003/ by johnme@chromium.org. The reason for reverting is: This seems to have broken compile on Android arm builder (dbg): https://build.chromium.org/p/chromium.android/builders/Android%20arm%20Builde... The compile error is "ninja: error: unknown target 'android_webview_test_apk_apk', did you mean 'android_webview_test_apk__apk'?", and this is the only patch on the blamelist that relates to android_webview_test_apk. |
