Report apptest_runner device not found infra failures.

Make AndroidShell.InitShell return an exit code.
(infra exit code for infra errors and 'device not found')

Inline CleanShell in InitShell; store --origin internally.
Move gdb port forwarding to StartShell; minor refactoring.

BUG=493900
TEST=Mojo/Mandoline python scripts work as expected. Bots yield purple builds on "device not found" errors.
R=sky@chromium.org

Review URL: https://codereview.chromium.org/1143223005

Cr-Commit-Position: refs/heads/master@{#333186}

build/android/test_runner.py

@@ -1026,8 +1026,7 @@ def main():
     logging.exception('Error occurred.')
     if e.is_infra_error:
       return constants.INFRA_EXIT_CODE
-    else:
-      return constants.ERROR_EXIT_CODE
+    return constants.ERROR_EXIT_CODE
   except: # pylint: disable=W0702
     logging.exception('Unrecognized error occurred.')
     return constants.ERROR_EXIT_CODE

mandoline/tools/android_run_mandoline.py

@@ -9,7 +9,7 @@ import os
 import sys
 
 sys.path.insert(0, os.path.join(os.path.abspath(os.path.dirname(__file__)),
-                                '../../mojo/tools'))
+                                os.pardir, os.pardir, 'mojo', 'tools'))
 
 from mopy.android import AndroidShell
 from mopy.config import Config
@@ -38,9 +38,7 @@ def main():
                   is_debug=runner_args.debug,
                   apk_name="Mandoline.apk")
   shell = AndroidShell(config)
-  args.extend(shell.PrepareShellRun(None, runner_args.device, runner_args.gdb))
-
-  shell.CleanLogs()
+  shell.InitShell(None, runner_args.device)
   p = shell.ShowLogs()
   shell.StartShell(args, sys.stdout, p.terminate, runner_args.gdb)
   return 0

mojo/tools/android_mojo_shell.py

@@ -40,9 +40,7 @@ def main():
                   is_debug=runner_args.debug,
                   apk_name="MojoRunner.apk")
   shell = AndroidShell(config)
-  args.extend(shell.PrepareShellRun(runner_args.origin, runner_args.device))
-
-  shell.CleanLogs()
+  shell.InitShell(runner_args.origin, runner_args.device)
   p = shell.ShowLogs()
   shell.StartShell(args, sys.stdout, p.terminate)
   return 0

mojo/tools/apptest_runner.py

@@ -8,6 +8,7 @@
 import argparse
 import json
 import logging
+import os
 import sys
 import time
 
@@ -39,11 +40,12 @@ def main():
   logger.debug("Test list: %s" % test_list)
 
   shell = None
-  extra_args = []
   if config.target_os == Config.OS_ANDROID:
     from mopy.android import AndroidShell
     shell = AndroidShell(config)
-    extra_args.extend(shell.PrepareShellRun('localhost'))
+    result = shell.InitShell()
+    if result != 0:
+      return result
 
   tests = []
   passed = []
@@ -52,7 +54,7 @@ def main():
     test = test_dict["test"]
     test_name = test_dict.get("name", test)
     test_type = test_dict.get("type", "gtest")
-    test_args = test_dict.get("args", []) + extra_args
+    test_args = test_dict.get("args", [])
 
     print "Running %s...%s" % (test_name, ("\n" if args.verbose else "")),
     sys.stdout.flush()

mojo/tools/mopy/android.py

@@ -23,6 +23,7 @@ from pylib import constants
 from pylib.base import base_test_runner
 from pylib.device import device_errors
 from pylib.device import device_utils
+from pylib.utils import base_error
 from pylib.utils import apk_helper
 
 
@@ -54,6 +55,7 @@ class AndroidShell(object):
     self.adb_path = constants.GetAdbPath()
     self.paths = Paths(config)
     self.device = None
+    self.shell_args = []
     self.target_package = apk_helper.GetPackageName(self.paths.apk_path)
     # This is used by decive_utils.Install to check if the apk needs updating.
     constants.SetOutputDirectory(self.paths.build_dir)
@@ -131,45 +133,42 @@ class AndroidShell(object):
       result.append(self._StartHttpServerForOriginMapping(value))
     return [MAPPING_PREFIX + ','.join(result)]
 
-  def PrepareShellRun(self, origin=None, device=None, gdb=False):
+  def InitShell(self, origin='localhost', device=None):
     """
-    Prepares for StartShell: runs adb as root and installs the apk as needed.
-    If the origin specified is 'localhost', a local http server will be set up
-    to serve files from the build directory along with port forwarding.
-    |device| is the device to run on, if multiple devices are connected.
-    Returns arguments that should be appended to shell argument list.
+    Runs adb as root, starts an origin server, and installs the apk as needed.
+    |origin| is the origin for mojo: URLs; if its value is 'localhost', a local
+    http server will be set up to serve files from the build directory.
+    |device| is the target device to run on, if multiple devices are connected.
+    Returns 0 on success or a non-zero exit code on a terminal failure.
     """
-    devices = device_utils.DeviceUtils.HealthyDevices()
-    if device:
-      self.device = next((d for d in devices if d == device), None)
-      if not self.device:
-        raise device_errors.DeviceUnreachableError(device)
-    elif devices:
-      self.device = devices[0]
-    else:
-      raise device_errors.NoDevicesError()
-
-    logging.getLogger().debug("Using device: %s", self.device)
-    self.device.EnableRoot()
-
-    # TODO(msw): Install fails often, retry as needed; http://crbug.com/493900
     try:
+      devices = device_utils.DeviceUtils.HealthyDevices()
+      if device:
+        self.device = next((d for d in devices if d == device), None)
+        if not self.device:
+          raise device_errors.DeviceUnreachableError(device)
+      elif devices:
+        self.device = devices[0]
+      else:
+        raise device_errors.NoDevicesError()
+
+      logging.getLogger().debug("Using device: %s", self.device)
+      # Clean the logs on the device to avoid displaying prior activity.
+      subprocess.check_call(self._CreateADBCommand(['logcat', '-c']))
+      self.device.EnableRoot()
       self.device.Install(self.paths.apk_path)
-    except device_errors.CommandFailedError as e:
-      logging.getLogger().error("APK install failed:\n%s", str(e))
-      self.device.Install(self.paths.apk_path)
+    except base_error.BaseError as e:
+      # Report "device not found" as infra failures. See http://crbug.com/493900
+      print "Exception in AndroidShell.InitShell:\n%s" % str(e)
+      if e.is_infra_error or "error: device not found" in str(e):
+        return constants.INFRA_EXIT_CODE
+      return constants.ERROR_EXIT_CODE
 
-    extra_args = []
     if origin is 'localhost':
       origin = self._StartHttpServerForDirectory(self.paths.build_dir)
     if origin:
-      extra_args.append("--origin=" + origin)
-
-    if gdb:
-      # Remote debugging needs a port forwarded.
-      self.device.adb.Forward('tcp:5039', 'tcp:5039')
-
-    return extra_args
+      self.shell_args.append("--origin=" + origin)
+    return 0
 
   def _GetProcessId(self, process):
     """Returns the process id of the process on the remote device."""
@@ -229,11 +228,15 @@ class AndroidShell(object):
     local_gdb_process.wait()
     signal.signal(signal.SIGINT, signal.SIG_DFL)
 
-  def StartShell(self, arguments, stdout, on_application_stop, gdb=False):
+  def StartShell(self, arguments, stdout, on_fifo_closed, gdb=False):
     """
-    Starts the shell with the given arguments, directing output to |stdout|.
-    The |arguments| list must contain the "--origin=" arg from PrepareShellRun.
+    Starts the shell with the given |arguments|, directing output to |stdout|.
+    |on_fifo_closed| will be run if the FIFO can't be found or when it's closed.
+    |gdb| is a flag that attaches gdb to the device's remote process on startup.
     """
+    assert self.device
+    arguments += self.shell_args
+
     cmd = self._CreateADBCommand([
            'shell',
            'am',
@@ -245,7 +248,9 @@ class AndroidShell(object):
 
     logcat_process = None
     if gdb:
-      arguments += ['--wait-for-debugger']
+      arguments.append('--wait-for-debugger')
+      # Remote debugging needs a port forwarded.
+      self.device.adb.Forward('tcp:5039', 'tcp:5039')
       logcat_process = self.ShowLogs(stdout=subprocess.PIPE)
 
     fifo_path = "/data/data/%s/stdout.fifo" % self.target_package
@@ -253,15 +258,14 @@ class AndroidShell(object):
         ['shell', 'rm', '-f', fifo_path]))
     arguments.append('--fifo-path=%s' % fifo_path)
     max_attempts = 200 if '--wait-for-debugger' in arguments else 5
-    self._ReadFifo(fifo_path, stdout, on_application_stop, max_attempts)
+    self._ReadFifo(fifo_path, stdout, on_fifo_closed, max_attempts)
 
     # Extract map-origin args and add the extras array with commas escaped.
     parameters = [a for a in arguments if not a.startswith(MAPPING_PREFIX)]
     map_parameters = [a for a in arguments if a.startswith(MAPPING_PREFIX)]
     parameters += self._StartHttpServerForOriginMappings(map_parameters)
     parameters = [p.replace(',', '\,') for p in parameters]
-    if parameters:
-      cmd += ['--esa', 'org.chromium.mojo.shell.extras', ','.join(parameters)]
+    cmd += ['--esa', 'org.chromium.mojo.shell.extras', ','.join(parameters)]
 
     atexit.register(self.StopShell)
     with open(os.devnull, 'w') as devnull:
@@ -274,10 +278,6 @@ class AndroidShell(object):
     """Stops the mojo shell."""
     self.device.ForceStop(self.target_package)
 
-  def CleanLogs(self):
-    """Cleans the logs on the device."""
-    subprocess.check_call(self._CreateADBCommand(['logcat', '-c']))
-
   def ShowLogs(self, stdout=sys.stdout):
     """Displays the mojo shell logs and returns the process reading the logs."""
     logcat = subprocess.Popen(self._CreateADBCommand([