This is a less aggressive patch than the description below. The original was reverted for breaking the Android tests bot (https://codereview.chromium.org/16299003).

Removes check install step and moves it into the device status check. This will allow us to examine Android install speeds, which have recently caused timeout problems when installing large APK's. Unlike the original CL, low install speed only causes a warning (i.e. does not fail the step). The step fails if the battery of an online device is less than 5%.

BUG=230970, 224004, 242237

---------------------------------------------------
Original CL:
Move CheckInstall to device status and fail on low install speed and battery.

Removes the recently added CheckInstall step and moves the checking to the
device status check step. Recently, devices will install very slowly causing
tests to fail with device problems. We have also seen battery issues. The new
device status check step will fail if any devices install unreasonably slow
(i.e. < 800 KB/s) or have critically low battery level (i.e. < 5%).

BUG=230970, 224004, 242237

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203587

Review URL: https://chromiumcodereview.appspot.com/16110005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204013 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 28 insertions, 16 deletions

diff --git a/build/android/buildbot/bb_device_steps.py b/build/android/buildbot/bb_device_steps.py
index f9e2883..6ec9c87 100755
--- a/build/android/buildbot/bb_device_steps.py
+++ b/build/android/buildbot/bb_device_steps.py
@@ -162,15 +162,6 @@ def RunChromeDriverTests():
   RunCmd(['chrome/test/chromedriver/run_buildbot_steps.py',
           '--android-package=%s' % constants.CHROMIUM_TEST_SHELL_PACKAGE])
 
-
-def CheckInstall():
-  """Build bot step to see if adb install works on attached devices. """
-  buildbot_report.PrintNamedStep('Check device install')
-  # This step checks if apks can be installed on the devices.
-  args = ['--apk', 'build/android/CheckInstallApk-debug.apk']
-  RunCmd(['build/android/adb_install_apk.py'] + args, halt_on_failure=True)
-
-
 def InstallApk(options, test, print_step=False):
   """Install an apk to all phones.
 
@@ -286,9 +277,6 @@ def MainTestWrapper(options):
   target = options.factory_properties.get('target', 'Debug')
   RunCmd(['build/android/provision_devices.py', '-t', target])
 
-  # Check to see if devices can install apks.
-  CheckInstall()
-
   if options.install:
     test_obj = INSTRUMENTATION_TESTS[options.install]
     InstallApk(options, test_obj, print_step=True)
diff --git a/build/android/device_status_check.py b/build/android/device_status_check.py
index 15ab493..47a63a0 100755
--- a/build/android/device_status_check.py
+++ b/build/android/device_status_check.py
@@ -24,7 +24,8 @@ def DeviceInfo(serial):
     serial: The serial of the attached device to construct info about.
 
   Returns:
-    Tuple of device type, build id and report as a string.
+    Tuple of device type, build id, report as a string, error messages, and
+    boolean indicating whether or not device can be used for testing.
   """
 
   def AdbShellCmd(cmd):
@@ -38,6 +39,13 @@ def DeviceInfo(serial):
   setup_wizard_disabled = AdbShellCmd(
       'getprop ro.setupwizard.mode') == 'DISABLED'
   battery = AdbShellCmd('dumpsys battery')
+  install_output = GetCmdOutput(['build/android/adb_install_apk.py', '--apk',
+                                 'build/android/CheckInstallApk-debug.apk'])
+  install_speed_found = re.findall('(\d+) KB/s', install_output)
+  if install_speed_found:
+    install_speed = int(install_speed_found[0])
+  else:
+    install_speed = 'Unknown'
   if 'Error' in battery:
     ac_power = 'Unknown'
     battery_level = 'Unknown'
@@ -55,17 +63,26 @@ def DeviceInfo(serial):
                                              '| grep Device'
                                              "| awk '{print $4}'")[-6:],
             '  Wifi IP: %s' % AdbShellCmd('getprop dhcp.wlan0.ipaddress'),
+            '  Install Speed: %s KB/s' % install_speed,
             '']
 
   errors = []
   if battery_level < 5:
-    errors += ['Device critically low in battery.']
+    errors += ['Device critically low in battery. Do not use for testing.']
   if not setup_wizard_disabled:
     errors += ['Setup wizard not disabled. Was it provisioned correctly?']
   if device_product_name == 'mantaray' and ac_power != 'true':
     errors += ['Mantaray device not connected to AC power.']
+  if install_speed < 800:
+    errors += ['Device install speed too low. Do not use for testing.']
 
-  return device_type, device_build, '\n'.join(report), errors
+  # TODO(navabi): Determine if device status check step should fail on slow
+  # install speed. The original CL caused the step to fail but was reverted
+  # because it caused too many early failures. Determine if it was just flake.
+  # Also, do not fail on 'Unknown' caused by offline device, because other
+  # devices can still be used for tests.
+  fail_step = (battery_level == 'Unknown' or battery_level >= 5)
+  return device_type, device_build, '\n'.join(report), errors, fail_step
 
 
 def CheckForMissingDevices(options, adb_online_devs):
@@ -167,7 +184,8 @@ def main():
   devices = android_commands.GetAttachedDevices()
   types, builds, reports, errors = [], [], [], []
   if devices:
-    types, builds, reports, errors = zip(*[DeviceInfo(dev) for dev in devices])
+    types, builds, reports, errors, fail_step_lst = zip(*[DeviceInfo(dev)
+                                                          for dev in devices])
 
   err_msg = CheckForMissingDevices(options, devices) or []
 
@@ -189,6 +207,12 @@ def main():
     print msg
     SendDeviceStatusAlert(msg)
 
+  if False in fail_step_lst:
+    # TODO(navabi): Build fails on device status check step if there exists any
+    # devices with critically low battery or install speed. Remove those devices
+    # from testing, allowing build to continue with good devices.
+    return 1
+
   if not devices:
     return 1