Retry tests on other bots if the device is unresponsive/offline

build/android/pylib/base_test_sharder.py

Index: build/android/pylib/base_test_sharder.py
diff --git a/build/android/pylib/base_test_sharder.py b/build/android/pylib/base_test_sharder.py
index 48206c202ce1774a402da8bbc98f2aa9a5269491..25bde3c9bb09a57b609b174d49a479fd7d80e9ae 100644
--- a/build/android/pylib/base_test_sharder.py
+++ b/build/android/pylib/base_test_sharder.py
@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 
+import android_commands
 import logging
 import multiprocessing
 
@@ -78,7 +79,9 @@ class BaseTestSharder(object):
     logging.warning('Look for the "Final result" banner in the end.')
     logging.warning('*' * 80)
     final_results = TestResults()
-    for retry in xrange(self.retries):
+    rounds = self.retries
+    retry = 0
+    while retry < rounds:
       logging.warning('Try %d of %d', retry + 1, self.retries)
       self.SetupSharding(self.tests)
       test_runners = []
@@ -96,8 +99,17 @@ class BaseTestSharder(object):
       # So use map_async instead.
       async_results = pool.map_async(_ShardedTestRunnable, test_runners)
       results_lists = async_results.get(999999)
+
+      if (TestResults.HasDeviceExcept(results_lists) and
+          retry == rounds - 1):
+        rounds += 1

[bulach, 2012/10/23 09:30:56]

hmm.. this may never finish: if we keep getting device exception, the while
retry < rounds would never exit given rounds +=1 here.. maybe remove this block,
and let it fall through and just use the standard retry in case of failure?

we'll need to switch the bots to use this param, if they're not using it yet..

[yongsheng, 2012/10/23 12:58:14]

oh, yes. 
So what if retry equals to rounds - 1? then the failed cases won't be executed 
since there is no next retry. Suppose self.retries = 1 and one device gets
offline, then tests on this device won't be re-run.

[yongsheng, 2012/10/23 13:08:35]

if one device raise exception, then it's excluded in the list according to your
suggestion in the below comment. and if device list is empty, the loop is done.

[bulach, 2012/10/23 15:01:53]

oh, sorry, I think the current semantics (before your patch!!) for retry here is
broken :)
perhaps we could implement the following:
- if there are are no failures or is the last round, break the loop..

- else: remove devices that had exceptions from the list (if any), and try
again..

this is guaranteed to terminate, and to retry if necessary.

what do you think?

how does that sound?

[yongsheng, 2012/10/24 01:41:52]

my only concern is that if there are failures(due to device exceptions) and it's
the last round (and there are attached devices), so we just ignore the failures
and don't retry them. If you think it's acceptable, I'll follw up your
suggestion here. 

[bulach, 2012/10/24 10:43:49]

I think that'd be fine, at least it's deterministic :)
otherwise, if we have two devices, and they keep alternating the failures, we'd
never exit the loop...

+
       test_results = TestResults.FromTestResults(results_lists)
-      if retry == self.retries - 1:
+      # Re-check the attached devices for some devices may
+      # become offline
+      self.attached_devices = android_commands.GetAttachedDevices()

[bulach, 2012/10/23 09:30:56]

nice! for extra safety, maybe we could do something like:
# Re-check the attached devices for some devices may
# become offline.
retry_devices = set(android_commands.GetAttachedDevices())
# Remove devices that had exceptions.
retry_devices -= TestResults.DeviceExceptions(results_lists)
# Retry on devices that didn't have any exception.
self.attached_devices = list(retry_devices)

[yongsheng, 2012/10/23 12:58:14]

great improvement. thanks. I'll follow your suggestion.

+      if (retry == rounds - 1 or
+          len(self.attached_devices) == 0):
         all_passed = final_results.ok + test_results.ok
         final_results = test_results
         final_results.ok = all_passed