[Android] Blacklist devices on failures during environment set up + t…

…ear down.

Requires https://codereview.chromium.org/2144103002/

BUG=627939

Review-Url: https://codereview.chromium.org/2144823003
Cr-Commit-Position: refs/heads/master@{#407594}

build/android/pylib/local/device/local_device_environment.py

@@ -3,12 +3,14 @@
 # found in the LICENSE file.
 
 import datetime
+import functools
 import logging
 import os
 import shutil
 import tempfile
 import threading
 
+from devil import base_error
 from devil.android import device_blacklist
 from devil.android import device_errors
 from devil.android import device_list
@@ -25,6 +27,50 @@ def _DeviceCachePath(device):
   return os.path.join(constants.GetOutDirectory(), file_name)
 
 
+def handle_shard_failures(f):
+  """A decorator that handles device failures for per-device functions.
+
+  Args:
+    f: the function being decorated. The function must take at least one
+      argument, and that argument must be the device.
+  """
+  return handle_shard_failures_with(None)(f)
+
+
+# TODO(jbudorick): Refactor this to work as a decorator or context manager.
+def handle_shard_failures_with(on_failure):
+  """A decorator that handles device failures for per-device functions.
+
+  This calls on_failure in the event of a failure.
+
+  Args:
+    f: the function being decorated. The function must take at least one
+      argument, and that argument must be the device.
+    on_failure: A binary function to call on failure.
+  """
+  def decorator(f):
+    @functools.wraps(f)
+    def wrapper(dev, *args, **kwargs):
+      try:
+        return f(dev, *args, **kwargs)
+      except device_errors.CommandTimeoutError:
+        logging.exception('Shard timed out: %s(%s)', f.__name__, str(dev))
+      except device_errors.DeviceUnreachableError:
+        logging.exception('Shard died: %s(%s)', f.__name__, str(dev))
+      except base_error.BaseError:
+        logging.exception('Shard failed: %s(%s)', f.__name__, str(dev))
+      except SystemExit:
+        logging.exception('Shard killed: %s(%s)', f.__name__, str(dev))
+        raise
+      if on_failure:
+        on_failure(dev, f.__name__)
+      return None
+
+    return wrapper
+
+  return decorator
+
+
 class LocalDeviceEnvironment(environment.Environment):
 
   def __init__(self, args, _error_func):
@@ -67,19 +113,21 @@ def SetUp(self):
     if not self._devices:
       raise device_errors.NoDevicesError
 
-    if self._enable_device_cache:
-      for d in self._devices:
+    if self._logcat_output_file:
+      self._logcat_output_dir = tempfile.mkdtemp()
+
+    @handle_shard_failures_with(on_failure=self.BlacklistDevice)
+    def prepare_device(d):
+      if self._enable_device_cache:
         cache_path = _DeviceCachePath(d)
         if os.path.exists(cache_path):
           logging.info('Using device cache: %s', cache_path)
           with open(cache_path) as f:
             d.LoadCacheData(f.read())
           # Delete cached file so that any exceptions cause it to be cleared.
           os.unlink(cache_path)
-    if self._logcat_output_file:
-      self._logcat_output_dir = tempfile.mkdtemp()
-    if self._logcat_output_dir:
-      for d in self._devices:
+
+      if self._logcat_output_dir:
         logcat_file = os.path.join(
             self._logcat_output_dir,
             '%s_%s' % (d.adb.GetDeviceSerial(),
@@ -89,6 +137,8 @@ def SetUp(self):
         self._logcat_monitors.append(monitor)
         monitor.Start()
 
+    self.parallel_devices.pMap(prepare_device)
+
   @property
   def blacklist(self):
     return self._blacklist
@@ -121,17 +171,27 @@ def tool(self):
 
   #override
   def TearDown(self):
-    # Write the cache even when not using it so that it will be ready the first
-    # time that it is enabled. Writing it every time is also necessary so that
-    # an invalid cache can be flushed just by disabling it for one run.
-    for d in self._devices:
+    @handle_shard_failures_with(on_failure=self.BlacklistDevice)
+    def tear_down_device(d):
+      # Write the cache even when not using it so that it will be ready the
+      # first time that it is enabled. Writing it every time is also necessary
+      # so that an invalid cache can be flushed just by disabling it for one
+      # run.
       cache_path = _DeviceCachePath(d)
       with open(cache_path, 'w') as f:
         f.write(d.DumpCacheData())
         logging.info('Wrote device cache: %s', cache_path)
+
+    self.parallel_devices.pMap(tear_down_device)
+
     for m in self._logcat_monitors:
-      m.Stop()
-      m.Close()
+      try:
+        m.Stop()
+        m.Close()
+      except base_error.BaseError:
+        logging.exception('Failed to stop logcat monitor for %s',
+                          m.adb.GetDeviceSerial())
+
     if self._logcat_output_file:
       file_utils.MergeFiles(
           self._logcat_output_file,

build/android/pylib/local/device/local_device_gtest_run.py

@@ -232,7 +232,7 @@ def TestPackage(self):
 
   #override
   def SetUp(self):
-    @local_device_test_run.handle_shard_failures_with(
+    @local_device_environment.handle_shard_failures_with(
         on_failure=self._env.BlacklistDevice)
     def individual_device_set_up(dev):
       def install_apk():
@@ -313,7 +313,7 @@ def _GetTests(self):
     # Even when there's only one device, it still makes sense to retrieve the
     # test list so that tests can be split up and run in batches rather than all
     # at once (since test output is not streamed).
-    @local_device_test_run.handle_shard_failures_with(
+    @local_device_environment.handle_shard_failures_with(
         on_failure=self._env.BlacklistDevice)
     def list_tests(dev):
       raw_test_list = self._delegate.Run(
@@ -363,7 +363,7 @@ def _RunTest(self, device, test):
 
   #override
   def TearDown(self):
-    @local_device_test_run.handle_shard_failures
+    @local_device_environment.handle_shard_failures
     def individual_device_tear_down(dev):
       for s in self._servers.get(str(dev), []):
         s.TearDown()

build/android/pylib/local/device/local_device_instrumentation_test_run.py

@@ -13,6 +13,7 @@
 from devil.utils import reraiser_thread
 from pylib import valgrind_tools
 from pylib.base import base_test_result
+from pylib.local.device import local_device_environment
 from pylib.local.device import local_device_test_run
 
 
@@ -67,7 +68,7 @@ def substitute_device_root(d, device_root):
       else:
         return d
 
-    @local_device_test_run.handle_shard_failures_with(
+    @local_device_environment.handle_shard_failures_with(
         self._env.BlacklistDevice)
     def individual_device_set_up(dev, host_device_tuples):
       def install_apk():
@@ -148,7 +149,7 @@ def create_flag_changer():
         self._test_instance.GetDataDependencies())
 
   def TearDown(self):
-    @local_device_test_run.handle_shard_failures_with(
+    @local_device_environment.handle_shard_failures_with(
         self._env.BlacklistDevice)
     def individual_device_tear_down(dev):
       if str(dev) in self._flag_changers:

build/android/pylib/local/device/local_device_perf_test_run.py

@@ -25,6 +25,7 @@
 from pylib import constants
 from pylib.base import base_test_result
 from pylib.constants import host_paths
+from pylib.local.device import local_device_environment
 from pylib.local.device import local_device_test_run
 
 
@@ -78,7 +79,7 @@ def __init__(
     self._timeout = timeout
     self._heart_beat = HeartBeat(self)
 
-  @local_device_test_run.handle_shard_failures
+  @local_device_environment.handle_shard_failures
   def RunTestsOnShard(self):
     results = base_test_result.TestRunResults()
     for test in self._tests:

build/android/pylib/local/device/local_device_test_run.py

@@ -3,20 +3,18 @@
 # found in the LICENSE file.
 
 import fnmatch
-import functools
 import imp
 import logging
 import signal
 import thread
 import threading
 
-from devil import base_error
-from devil.android import device_errors
 from devil.utils import signal_handler
 from pylib import valgrind_tools
 from pylib.base import base_test_result
 from pylib.base import test_run
 from pylib.base import test_collection
+from pylib.local.device import local_device_environment
 
 
 def IncrementalInstall(device, apk_helper, installer_script):
@@ -41,49 +39,6 @@ def IncrementalInstall(device, apk_helper, installer_script):
                     permissions=None)  # Auto-grant permissions from manifest.
 
 
-def handle_shard_failures(f):
-  """A decorator that handles device failures for per-device functions.
-
-  Args:
-    f: the function being decorated. The function must take at least one
-      argument, and that argument must be the device.
-  """
-  return handle_shard_failures_with(None)(f)
-
-
-def handle_shard_failures_with(on_failure):
-  """A decorator that handles device failures for per-device functions.
-
-  This calls on_failure in the event of a failure.
-
-  Args:
-    f: the function being decorated. The function must take at least one
-      argument, and that argument must be the device.
-    on_failure: A binary function to call on failure.
-  """
-  def decorator(f):
-    @functools.wraps(f)
-    def wrapper(dev, *args, **kwargs):
-      try:
-        return f(dev, *args, **kwargs)
-      except device_errors.CommandTimeoutError:
-        logging.exception('Shard timed out: %s(%s)', f.__name__, str(dev))
-      except device_errors.DeviceUnreachableError:
-        logging.exception('Shard died: %s(%s)', f.__name__, str(dev))
-      except base_error.BaseError:
-        logging.exception('Shard failed: %s(%s)', f.__name__, str(dev))
-      except SystemExit:
-        logging.exception('Shard killed: %s(%s)', f.__name__, str(dev))
-        raise
-      if on_failure:
-        on_failure(dev, f.__name__)
-      return None
-
-    return wrapper
-
-  return decorator
-
-
 class LocalDeviceTestRun(test_run.TestRun):
 
   def __init__(self, env, test_instance):
@@ -96,7 +51,7 @@ def RunTests(self):
 
     exit_now = threading.Event()
 
-    @handle_shard_failures
+    @local_device_environment.handle_shard_failures
     def run_tests_on_device(dev, tests, results):
       for test in tests:
         if exit_now.isSet():