Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2712613003:
Add swarmed version of VR tests to FYI bot (Closed)
DescriptionAdd swarmed version of VR tests to FYI bot
Adds a swarmed version of chrome_public_test_vr_apk to Android VR Tests.
Currently only runs on a single 5X with N, but will update dimensions over
time as more OSes besides N are supported.
Instrumentation tests are still kept around since there are no Pixel
devices available in swarming, and the builder has a Pixel attached.
BUG=642946, 671373
Review-Url: https://codereview.chromium.org/2712613003
Cr-Commit-Position: refs/heads/master@{#452628}
Committed: https://chromium.googlesource.com/chromium/src/+/8d42011f9c4c3f748efb8bb554a62cbb2ec8b474
Patch Set 1 #
Total comments: 7
Messages
Total messages: 18 (7 generated)
Description was changed from ========== Add swarmed version of VR tests to FYI bot Adds a swarmed version of chrome_public_test_vr_apk to Android VR Tests. Currently only runs on a single 5X with N, but will update dimensions over time as more OSes besides N are supported. BUG=642946,671373 ========== to ========== Add swarmed version of VR tests to FYI bot Adds a swarmed version of chrome_public_test_vr_apk to Android VR Tests. Currently only runs on a single 5X with N, but will update dimensions over time as more OSes besides N are supported. Instrumentation tests are still kept around since there are no Pixel devices available in swarming, and the builder has a Pixel attached. BUG=642946,671373 ==========
https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... File testing/buildbot/chromium.fyi.json (right): https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... testing/buildbot/chromium.fyi.json:226: "--shared-prefs-file=chrome/android/shared_preference_files/test/vr_cardboard_skipdon_setupcomplete.json", --shared-prefs-file is currently only used in test_runner.py if the test is an instrumentation test. Will it need to be changed to also support gtests, or will swarming somehow determine that the underlying APK is actually an instrumentation test and use that? https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... testing/buildbot/chromium.fyi.json:257: "test": "chrome_public_test_vr_apk" Not sure if this will cause issues since it'll create multiple steps with the same name (instrumentation_tests don't support the "name" field like gtest_tests). Might have to add support for that, or would it work to just stick this in a gtest with can_use_on_swarming_builders set to false?
bsheedy@chromium.org changed reviewers: + sky@chromium.org
PTAL
sky@chromium.org changed reviewers: + dpranke@chromium.org - sky@chromium.org
sky->dpranke
dpranke@chromium.org changed reviewers: + bpastene@chromium.org, jbudorick@chromium.org
lgtm, but bpastene@ or jbudorick@ are probably better reviewers for the android-specific parts.
lgtm https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... File testing/buildbot/chromium.fyi.json (right): https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... testing/buildbot/chromium.fyi.json:226: "--shared-prefs-file=chrome/android/shared_preference_files/test/vr_cardboard_skipdon_setupcomplete.json", On 2017/02/23 01:37:01, bsheedy wrote: > --shared-prefs-file is currently only used in test_runner.py if the test is an > instrumentation test. Will it need to be changed to also support gtests, or will > swarming somehow determine that the underlying APK is actually an > instrumentation test and use that? This is fine. I can explain in more detail why next week if you'd like. https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... testing/buildbot/chromium.fyi.json:257: "test": "chrome_public_test_vr_apk" On 2017/02/23 01:37:01, bsheedy wrote: > Not sure if this will cause issues since it'll create multiple steps with the > same name (instrumentation_tests don't support the "name" field like > gtest_tests). Might have to add support for that, or would it work to just stick This is an unusual setup, though I *think* it'll still work as-is. The recipe system can handle multiple steps w/ the same name. > this in a gtest with can_use_on_swarming_builders set to false? No, I don't think this would work.
Thanks for the quick review despite being OOO. https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... File testing/buildbot/chromium.fyi.json (right): https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... testing/buildbot/chromium.fyi.json:226: "--shared-prefs-file=chrome/android/shared_preference_files/test/vr_cardboard_skipdon_setupcomplete.json", On 2017/02/23 19:45:05, jbudorick is ooo until monday wrote: > On 2017/02/23 01:37:01, bsheedy wrote: > > --shared-prefs-file is currently only used in test_runner.py if the test is an > > instrumentation test. Will it need to be changed to also support gtests, or > will > > swarming somehow determine that the underlying APK is actually an > > instrumentation test and use that? > > This is fine. I can explain in more detail why next week if you'd like. Sure - I'll be messing around with this stuff enough in the future that understanding what's going on will be helpful.
The CQ bit was checked by bsheedy@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... File testing/buildbot/chromium.fyi.json (right): https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... testing/buildbot/chromium.fyi.json:236: "device_os": "NRD91N", Just so you're aware, you're triggering this test onto a swarming pool that's already pretty over-subscribed: https://chromium-swarm.appspot.com/botlist?c=id&c=os&c=task&c=status&f=device... Expect long wait times or expired tasks as-is.
https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... File testing/buildbot/chromium.fyi.json (right): https://codereview.chromium.org/2712613003/diff/1/testing/buildbot/chromium.f... testing/buildbot/chromium.fyi.json:236: "device_os": "NRD91N", On 2017/02/23 19:54:00, bpastene wrote: > Just so you're aware, you're triggering this test onto a swarming pool that's > already pretty over-subscribed: > https://chromium-swarm.appspot.com/botlist?c=id&c=os&c=task&c=status&f=device... > > Expect long wait times or expired tasks as-is. Thanks for the notification. Are there any other pools running N that are less busy?
On 2017/02/23 20:44:40, bsheedy wrote: > Thanks for the notification. Are there any other pools running N that are less > busy? Nope, that's the only one. I'll throw another bot into that pool when I get a chance later today.
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1487879309500450, "parent_rev":
"3ba5676657840e4d18244871f2ffd4ee354874a1", "commit_rev":
"8d42011f9c4c3f748efb8bb554a62cbb2ec8b474"}
Message was sent while issue was closed.
Description was changed from ========== Add swarmed version of VR tests to FYI bot Adds a swarmed version of chrome_public_test_vr_apk to Android VR Tests. Currently only runs on a single 5X with N, but will update dimensions over time as more OSes besides N are supported. Instrumentation tests are still kept around since there are no Pixel devices available in swarming, and the builder has a Pixel attached. BUG=642946,671373 ========== to ========== Add swarmed version of VR tests to FYI bot Adds a swarmed version of chrome_public_test_vr_apk to Android VR Tests. Currently only runs on a single 5X with N, but will update dimensions over time as more OSes besides N are supported. Instrumentation tests are still kept around since there are no Pixel devices available in swarming, and the builder has a Pixel attached. BUG=642946,671373 Review-Url: https://codereview.chromium.org/2712613003 Cr-Commit-Position: refs/heads/master@{#452628} Committed: https://chromium.googlesource.com/chromium/src/+/8d42011f9c4c3f748efb8bb554a6... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/8d42011f9c4c3f748efb8bb554a6... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
