Change Android retry logic - always retry 3 times.

build/android/pylib/base/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 logging
 import multiprocessing
 
 from pylib import android_commands
 from pylib.base.test_result import TestResults
@@ skipping 36 matching lines @@
     self.attached_devices = attached_devices
     # Worst case scenario: a device will drop offline per run, so we need
     # to retry until we're out of devices.
 
     # TODO(frankf): There are two sources of flakiness:
     # 1. Device flakiness
     # 2. Test/product flakiness
     # We should differentiate between these. Otherwise, blindly retrying tests
     # might mask test/product flakiness. For type 2, we should follow the
     # general chrome best practices.
-    self.retries = len(self.attached_devices)
+    self.retries = 3

[frankf, 2013/02/14 03:42:15]

Do you have some data why the tot bot was flaky? Referring to the TODO above,
was it device or test/product flakiness? The original reason for adding retries
at all was to mitigate only device flakiness. There has been a lot of discussion
(chromium-dev and offline) regarding not masking flakiness, but either
fixing/disabling tests or fixing the product as the case may be.

[Isaac (away), 2013/02/14 06:38:50]

I agree this code would be better if we distinguished between the two.  But most
bots have 3 or more devices, so they already are doing the retries.  What I am
trying to avoid here is different behavior based on number of phones attached. 
Currently the main builder masks flakiness because it has 7 phones.

[frankf, 2013/02/14 06:43:48]

Let's define it as a constant at the top of the module.

[Isaac (away), 2013/02/14 06:53:10]

Done.

[Yaron, 2013/02/14 16:59:27]

It wasn't the bot that was flaky (it only had one device). The other bots just
masked it. Min landed https://gerrit-int.chromium.org/#/c/32469/ which should
fix one issue and 147905 can continue to track the flakiness.

     self.tests = []
     self.build_type = build_type
 
   def CreateShardedTestRunner(self, device, index):
     """Factory function to create a suite-specific test runner.
 
     Args:
       device: Device serial where this shard will run
       index: Index of this device in the pool.
 
@@ skipping 31 matching lines @@
       test_runners = []
 
       # Try to create N shards, and retrying on failure.
       try:
         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]
       except android_commands.errors.DeviceUnresponsiveError as e:

[cjhopman, 2013/02/14 03:06:34]

What happens if 4 devices are offline? It appears that no tests will run and the
build will be green.

[Isaac (away), 2013/02/14 03:17:46]

attached_devices is initially set to only the online devices.  The question --
do more than 3 devices drop offline while pushing test binaries.  I'd say we can
just fail in that case.

[cjhopman, 2013/02/14 15:37:07]

I would be ok with just failing in that case, but it looks like we wouldn't
fail, instead we would run no tests and pass. That behavior is much worse.

         logging.critical('****Failed to create a shard: [%s]', e)
         self.attached_devices.remove(device)
         continue
 
       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.
@@ skipping 27 matching lines @@
         for t in test_results.GetAllBroken():
           self.tests += [t.name]
         if not self.tests:
           break
     else:
       # We ran out retries, possibly out of healthy devices.
       # There's no recovery at this point.
       raise Exception('Unrecoverable error while retrying test runs.')
     self._KillHostForwarder()
     return final_results