New issue
Advanced search Search tips

Issue 853280 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Test runner & apk_operations.py should filter by architecture when auto-selecting devices

Project Member Reported by agrieve@chromium.org, Jun 15 2018

Issue description

If you have one arm32 device attached via USB, and one x86 emulator running, then running these commands will always fail:

out/Debug/bin/run_base_unittests
out/Debug/bin/chrome_public_apk run

The first fails because it tries to install an x86 apk on an arm device (or vice versa).
The second one fails because it wants you to choose which device to run on (even though it should be obvious).

To fix this:
1. Add GetArchitectures() to ApkHelper:
https://cs.chromium.org/chromium/src/third_party/catapult/devil/devil/android/apk_helper.py?sq=package:chromium&g=0
  * Return a list of architectures, as discovered by lib/* directories that exist.
2. Add an architectures=[] parameter to DeviceUtils.HealthyDevices:
https://cs.chromium.org/chromium/src/third_party/catapult/devil/devil/android/device_utils.py?rcl=31767de5cf0a4baa96c28c90663276e165d86473&l=2744
3. Pass the parameter in test_runner.py and apk_operations.py.

Some subtleties here:
* arm64 devices should be return when arch=arm32, but not the other way around.
* x64 devices should be returned when arch=x86, but not the other way around.
* If arch= filters out all devices, we should give a good error message in the NoDevicesError(msg="") explaining that there are devices, but that they are of incompatible architectures.
 
Not sure about #2 vs filtering on what HealthyDevices returns, but otherwise SGTM.
Components: -Infra>Client>Android Infra>Client>Chrome
re: #1 - HealthDevices() currently:
1) Selects by ANDROID_SERIAL / args (or all devices)
2) Filters by blacklisted
3) Checks for no devices / multiple devices and throws an error (with nice message) if it's not what we want.

The problem with filtering after-the-fact is that HealthyDevices would throw an error in #3, even if there is only one device of the desired architecture.

I think the architecture filtering fits with its mandate though, it's the API you call to get intelligent selection and good error messages. Otherwise, you'd just call adb_wrapper.AdbWrapper.Devices() directly.

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/829fb0c2903fd725326beffcdf4620029d7ce9f9

commit 829fb0c2903fd725326beffcdf4620029d7ce9f9
Author: Calder Kitagawa <ckitagawa@chromium.org>
Date: Thu Jun 21 17:50:33 2018

[Build] Filter HealthyDevices by ABI

This adds a filter to DeviceUtils.HealthyDevices, which limit
output to devices supporting a specified list of ABIs. This filter is
imposed by the abis kwarg and is optional. If a filter is imposed it
will warn about attached devices that don't support the ABI of the
APK. apk_helper.py unzips the APK to extract the ABIs supported by the
APK.

Followup CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1106283

Bug: chromium:853280
Change-Id: Ic36cb18169c31dc3d0ec2b7ff9cb37455445e19b
No-Presubmit: true # lint failing on other files.
Reviewed-on: https://chromium-review.googlesource.com/1106544
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>

[modify] https://crrev.com/829fb0c2903fd725326beffcdf4620029d7ce9f9/devil/devil/android/device_utils.py
[modify] https://crrev.com/829fb0c2903fd725326beffcdf4620029d7ce9f9/devil/devil/android/apk_helper_test.py
[modify] https://crrev.com/829fb0c2903fd725326beffcdf4620029d7ce9f9/devil/devil/android/apk_helper.py
[modify] https://crrev.com/829fb0c2903fd725326beffcdf4620029d7ce9f9/devil/devil/android/device_utils_test.py

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e1ce67756664b9cc3d65fdebf0da135cc8f8a90

commit 0e1ce67756664b9cc3d65fdebf0da135cc8f8a90
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Jun 21 23:04:08 2018

Roll src/third_party/catapult 53842b6cbc1f..7a75f465f541 (4 commits)

https://chromium.googlesource.com/catapult.git/+log/53842b6cbc1f..7a75f465f541


git log 53842b6cbc1f..7a75f465f541 --date=short --no-merges --format='%ad %ae %s'
2018-06-21 simonhatch@chromium.org Revert "Dashboard - Raise error on duplicate histograms in /add_histograms"
2018-06-21 ckitagawa@chromium.org [Build] Filter HealthyDevices by ABI
2018-06-21 simonhatch@chromium.org Dashboard - Properly attach ownership data to alerts.
2018-06-21 benjhayden@chromium.org Add dashboard/dashboard/spa/OWNERS


Created with:
  gclient setdep -r src/third_party/catapult@7a75f465f541

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:853280
TBR=sullivan@chromium.org

Change-Id: I15c924f040d46935274f8527d60f55c926f6e850
Reviewed-on: https://chromium-review.googlesource.com/1110301
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#569410}
[modify] https://crrev.com/0e1ce67756664b9cc3d65fdebf0da135cc8f8a90/DEPS

Owner: agrieve@chromium.org
Looks like this won't get done before the end of my internship. Re-assigning to agrieve. The outstanding CL is available for takeover at:

https://chromium-review.googlesource.com/c/chromium/src/+/1106283
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad8aa26a0a06ec8bbe44f89ac0be79438301fed9

commit ad8aa26a0a06ec8bbe44f89ac0be79438301fed9
Author: Andrew Grieve <agrieve@chromium.org>
Date: Mon Sep 17 21:23:26 2018

apk_operations.py: Don't require --device when only one matches target_cpu

E.g. If one device is attached (arm32), and one emulator is running
(x86), then just use the correct one base on the apk's target_cpu.

Bug: 853280
Change-Id: Idc9904e098617f20700234f1b5e93a2291552cb5
Reviewed-on: https://chromium-review.googlesource.com/1228512
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591820}
[modify] https://crrev.com/ad8aa26a0a06ec8bbe44f89ac0be79438301fed9/build/android/apk_operations.py

Labels: DevX
Labels: QuickFix

Sign in to add a comment