Updates the test runner script exit codes

build/android/pylib/base/shard.py

Index: build/android/pylib/base/shard.py
diff --git a/build/android/pylib/base/shard.py b/build/android/pylib/base/shard.py
index 89b84f6f92bbc692a664389ded150ac5b60b39c4..e7f40a05c71d4adf1508c3ca388109db8d587f7f 100644
--- a/build/android/pylib/base/shard.py
+++ b/build/android/pylib/base/shard.py
@@ -8,6 +8,7 @@ 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
@@ -92,7 +93,7 @@ class _TestCollection(object):
     """Add an test to the collection.
 
     Args:
-      item: A test to add.
+      test: A test to add.
     """
     with self._lock:
       self._tests.append(test)
@@ -117,7 +118,7 @@ class _TestCollection(object):
 
 
 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
@@ -150,12 +151,6 @@ def _RunTestsFromQueue(runner, test_collection, out_results, watcher,
       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.
@@ -212,9 +207,15 @@ def _RunAllTests(runners, tests, num_retries, timeout=None):
           [r, tests_collection, results, watcher, num_retries],
           name=r.device[-4:])
        for r in runners])
-  workers.StartAll()
-  workers.JoinAll(watcher)
   run_results = base_test_result.TestRunResults()
+  workers.StartAll()
+
+  # 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
@@ -250,6 +251,7 @@ def _CreateRunners(runner_factory, devices, timeout=None):
 
 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.