Updates the test runner script exit codes

build/android/pylib/base/shard.py

 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Implements test sharding logic."""
 
 import logging
 import threading
 
 from pylib import android_commands
+from pylib import constants
 from pylib import forwarder
 from pylib.utils import reraiser_thread
 from pylib.utils import watchdog_timer
 
 import base_test_result
 
 
 DEFAULT_TIMEOUT = 7 * 60  # seven minutes
 
 
@@ skipping 64 matching lines @@
         try:
           return self._tests.pop(0)
         except IndexError:
           # Another thread beat us to the avaliable test, wait again.
           self._item_avaliable_or_all_done.clear()
 
   def add(self, test):
     """Add an test to the collection.
 
     Args:
-      item: A test to add.
+      test: A test to add.
     """
     with self._lock:
       self._tests.append(test)
       self._item_avaliable_or_all_done.set()
       self._tests_in_progress += 1
 
   def test_completed(self):
     """Indicate that a test has been fully handled."""
     with self._lock:
       self._tests_in_progress -= 1
       if self._tests_in_progress == 0:
         # All tests have been handled, signal all waiting threads.
         self._item_avaliable_or_all_done.set()
 
   def __iter__(self):
     """Iterate through tests in the collection until all have been handled."""
     while True:
       r = self._pop()
       if r is None:
         break
       yield r
 
 
 def _RunTestsFromQueue(runner, test_collection, out_results, watcher,
-    num_retries):
+                       num_retries):
   """Runs tests from the test_collection until empty using the given runner.
 
   Adds TestRunResults objects to the out_results list and may add tests to the
   out_retry list.
 
   Args:
     runner: A TestRunner object used to run the tests.
     test_collection: A _TestCollection from which to get _Test objects to run.
     out_results: A list to add TestRunResults to.
     watcher: A watchdog_timer.WatchdogTimer object, used as a shared timeout.
@@ skipping 12 matching lines @@
       if retry and test.tries <= num_retries:
         # Retry non-passing results, only record passing results.
         pass_results = base_test_result.TestRunResults()
         pass_results.AddResults(result.GetPass())
         out_results.append(pass_results)
         logging.warning('Will retry test, try #%s.' % test.tries)
         test_collection.add(_Test(test=retry, tries=test.tries))
       else:
         # All tests passed or retry limit reached. Either way, record results.
         out_results.append(result)
-    except android_commands.errors.DeviceUnresponsiveError:
-      # Device is unresponsive, stop handling tests on this device and ensure
-      # current test gets runs by another device. Don't reraise this exception
-      # on the main thread.
-      test_collection.add(test)

[frankf, 2013/07/03 21:26:37]

This decreases robustness. Currently, if we catch this exception, we add the
tests back to queue to get retried. 

[gkanwar, 2013/07/03 23:15:18]

I'm not sure I fully understand -- is this decreasing robustness by not adding
the tests back to the queue to be retried? The except clause below should catch
this exception and do that. The only change is whether we re-raise or silently
ignore the exception.

[frankf, 2013/07/04 00:46:19]

Ah, it makes sense.

On 2013/07/03 23:15:18, gkanwar wrote:

-      return
     except:
       # An unhandleable exception, ensure tests get run by another device and
       # reraise this exception on the main thread.
       test_collection.add(test)
       raise
     finally:
       # Retries count as separate tasks so always mark the popped test as done.
       test_collection.test_completed()
 
 
@@ skipping 36 matching lines @@
                   (len(tests), len(runners)))
   tests_collection = _TestCollection([_Test(t) for t in tests])
   results = []
   watcher = watchdog_timer.WatchdogTimer(timeout)
   workers = reraiser_thread.ReraiserThreadGroup(
       [reraiser_thread.ReraiserThread(
           _RunTestsFromQueue,
           [r, tests_collection, results, watcher, num_retries],
           name=r.device[-4:])
        for r in runners])
+  run_results = base_test_result.TestRunResults()
   workers.StartAll()
-  workers.JoinAll(watcher)
-  run_results = base_test_result.TestRunResults()
+
+  # Catch DeviceUnresponsiveErrors and set a warning exit code
+  try:
+    workers.JoinAll(watcher)
+  except android_commands.errors.DeviceUnresponsiveError as e:
+    run_results.exit_code = constants.WARNING_EXIT_CODE

[frankf, 2013/07/03 21:26:37]

Perhaps a singleton object that flags this exception happened is cleaner in this
case, than propagating the exit_code through the results object. If we're
returning a warning code, we should also do logging.error() of which device is
unresponsive.

[gkanwar, 2013/07/03 23:15:18]

That's a good point. Updated to use a singleton instead.

Logging added when we set this exit code here.

+
   for r in results:
     run_results.AddTestRunResults(r)
   return run_results
 
 
 def _CreateRunners(runner_factory, devices, timeout=None):
   """Creates a test runner for each device and calls SetUp() in parallel.
 
   Note: if a device is unresponsive the corresponding TestRunner will not be
     included in the returned list.
@@ skipping 15 matching lines @@
                                       [runner_factory, d, runners, counter],
                                       name=d[-4:])
        for d in devices])
   threads.StartAll()
   threads.JoinAll(watchdog_timer.WatchdogTimer(timeout))
   return runners
 
 
 def _TearDownRunners(runners, timeout=None):
   """Calls TearDown() for each test runner in parallel.
+
   Args:
     runners: a list of TestRunner objects.
     timeout: watchdog timeout in seconds, defaults to the default timeout.
   """
   threads = reraiser_thread.ReraiserThreadGroup(
       [reraiser_thread.ReraiserThread(r.TearDown, name=r.device[-4:])
        for r in runners])
   threads.StartAll()
   threads.JoinAll(watchdog_timer.WatchdogTimer(timeout))
 
@@ skipping 28 matching lines @@
   runners = _CreateRunners(runner_factory, devices, setup_timeout)
   try:
     return _RunAllTests(runners, tests, num_retries, test_timeout)
   finally:
     try:
       _TearDownRunners(runners, setup_timeout)
     except android_commands.errors.DeviceUnresponsiveError as e:
       logging.warning('Device unresponsive during TearDown: [%s]', e)
     finally:
       forwarder.Forwarder.KillHost(build_type)