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

build/android/pylib/base_test_sharder.py

 # Copyright (c) 2012 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.
 
 
+import android_commands
 import logging
 import multiprocessing
 
 from test_result import TestResults
 
 
 def _ShardedTestRunnable(test):
   """Standalone function needed by multiprocessing.Pool."""
   log_format = '[' + test.device + '] # %(asctime)-15s: %(message)s'
   if logging.getLogger().handlers:
@@ skipping 55 matching lines @@
     Returns:
       A TestResults object.
     """
     logging.warning('*' * 80)
     logging.warning('Sharding in ' + str(len(self.attached_devices)) +
                     ' devices.')
     logging.warning('Note that the output is not synchronized.')
     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 = []
       for index, device in enumerate(self.attached_devices):
         logging.warning('*' * 80)
         logging.warning('Creating shard %d for %s', index, device)
         logging.warning('*' * 80)
         test_runner = self.CreateShardedTestRunner(device, index)
         test_runners += [test_runner]
       logging.warning('Starting...')
       pool = multiprocessing.Pool(len(self.attached_devices),
                                   SetTestsContainer,
                                   [BaseTestSharder.tests_container])
       # map can't handle KeyboardInterrupt exception. It's a python bug.
       # 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
         break
       else:
         final_results.ok += test_results.ok
         self.tests = []
         for t in test_results.GetAllBroken():
           self.tests += [t.name]
         if not self.tests:
           break
     self.OnTestsCompleted(test_runners, final_results)
     return final_results