[Android] Prepare the test runner for @RetryOnFailure.

build/android/pylib/local/device/local_device_test_run.py

 # Copyright 2014 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 fnmatch
 import functools
 import imp
 import logging
 import signal
 import thread
@@ skipping 142 matching lines @@
         logging.info('FINISHED TRY #%d/%d', tries, self._env.max_tries)
         if tests:
           logging.info('%d failed tests remain.', len(tests))
         else:
           logging.info('All tests completed.')
 
     return results
 
   def _GetTestsToRetry(self, tests, try_results):
 
-    def is_failure(test_result):
+    def is_failure_result(test_result):
       return (
           test_result is None
           or test_result.GetType() not in (
               base_test_result.ResultType.PASS,
               base_test_result.ResultType.SKIP))
 
     all_test_results = {r.GetName(): r for r in try_results.GetAll()}
 
-    def should_retry(name):
+    def test_failed(name):
       # When specifying a test filter, names can contain trailing wildcards.
       # See local_device_gtest_run._ExtractTestsFromFilter()
       if name.endswith('*'):

[mikecase (-- gone --), 2016/07/01 19:00:37]

I dont think I understand this. I get that a * can be in the test filter, but
why would a * appear in the all_test_results list. Shouldnt that be a list of
the tests that actually ran?

[jbudorick, 2016/07/11 16:32:51]

Yeah, this is a bit complicated because of an optimization here:
https://chromium.googlesource.com/chromium/src/+/master/build/android/pylib/l...

* wouldn't appear in all_test_results, but test_failed isn't being called on the
contents of all_test_results. It's being called on the names of all the tests in
tests, i.e., whatever is returned from _GetTests. Because of that optimization,
that can be something that ends with *.

I'm not entirely thrilled about the existence of that optimization given the
complications it introduces, but it does make gtest execution a few seconds
faster for devs running, e.g., -f FooTest.*

-        return any(fnmatch.fnmatch(n, name) and is_failure(t)
+        return any(fnmatch.fnmatch(n, name) and is_failure_result(t)
                    for n, t in all_test_results.iteritems())
-      return is_failure(all_test_results.get(name))
+      return is_failure_result(all_test_results.get(name))
 
-    return [t for t in tests if should_retry(self._GetUniqueTestName(t))]
+    failed_tests = (t for t in tests if test_failed(self._GetUniqueTestName(t)))
+
+    return [t for t in failed_tests if self._ShouldRetry(t)]
 
   def GetTool(self, device):
     if not str(device) in self._tools:
       self._tools[str(device)] = valgrind_tools.CreateTool(
           self._env.tool, device)
     return self._tools[str(device)]
 
   def _CreateShards(self, tests):
     raise NotImplementedError
 
-  # pylint: disable=no-self-use
   def _GetUniqueTestName(self, test):
+    # pylint: disable=no-self-use
     return test
 
+  def _ShouldRetry(self, test):
+    # pylint: disable=no-self-use,unused-argument
+    return True
+
   def _GetTests(self):
     raise NotImplementedError
 
   def _RunTest(self, device, test):
     raise NotImplementedError
 
   def _ShouldShard(self):
     raise NotImplementedError