Clean up Android gtest runners.

build/android/run_tests.py

Index: build/android/run_tests.py
diff --git a/build/android/run_tests.py b/build/android/run_tests.py
index 2818dfe07106f35802063e623e705a94741af3b1..6f2a7b7036651d7031c9deee4455163b4f849c9b 100755
--- a/build/android/run_tests.py
+++ b/build/android/run_tests.py
@@ -27,9 +27,7 @@
 3.2.5  Chrome is installed on device.
 4. Run the binary in the device and stream the log to the host.
 4.1. Optionally, filter specific tests.
-4.2. Optionally, rebaseline: run the available tests and update the
-     suppressions file for failures.
-4.3. If we're running a single test suite and we have multiple devices
+4.2. If we're running a single test suite and we have multiple devices
      connected, we'll shard the tests.
 5. Clean up the device.
 
@@ -59,18 +57,16 @@ import subprocess
 import sys
 import time
 
+import emulator
 from pylib import android_commands
-from pylib.base_test_sharder import BaseTestSharder
 from pylib import buildbot_report
 from pylib import cmd_helper
-from pylib import constants
 from pylib import debug_info
-import emulator
 from pylib import ports
 from pylib import run_tests_helper
 from pylib import test_options_parser
+from pylib.base_test_sharder import BaseTestSharder
 from pylib.single_test_runner import SingleTestRunner
-from pylib.test_result import BaseTestResult, TestResults
 
 
 _TEST_SUITES = ['base_unittests',
@@ -89,7 +85,7 @@ _TEST_SUITES = ['base_unittests',
 
 
 def FullyQualifiedTestSuites(exe, option_test_suite, build_type):
-  """Return a fully qualified list
+  """Get a list of absolute paths to test suite targets.
 
   Args:
     exe: if True, use the executable-based test runner.
@@ -189,7 +185,7 @@ class TestSharder(BaseTestSharder):
   """Responsible for sharding the tests on the connected devices."""
 
   def __init__(self, attached_devices, test_suite, gtest_filter,
-               test_arguments, timeout, rebaseline, performance_test,
+               test_arguments, timeout, performance_test,
                cleanup_test_files, tool, log_dump_name, fast_and_loose,
                build_type, in_webkit_checkout):
     BaseTestSharder.__init__(self, attached_devices, build_type)
@@ -198,55 +194,61 @@ class TestSharder(BaseTestSharder):
     self.gtest_filter = gtest_filter or ''
     self.test_arguments = test_arguments
     self.timeout = timeout
-    self.rebaseline = rebaseline
     self.performance_test = performance_test
     self.cleanup_test_files = cleanup_test_files
     self.tool = tool
     self.log_dump_name = log_dump_name
     self.fast_and_loose = fast_and_loose
-    self.build_type = build_type
     self.in_webkit_checkout = in_webkit_checkout
-    self.tests = []
+    self.all_tests = []
     if not self.gtest_filter:
       # No filter has been specified, let's add all tests then.
-      self.tests, self.attached_devices = self._GetTests()
+      self.all_tests = self._GetAllEnabledTests()
+    self.tests = self.all_tests
+
+  def _GetAllEnabledTests(self):
+    """Returns a list of all enabled tests.
 
-  def _GetTests(self):
-    """Returns a tuple of (all_tests, available_devices).
+    Obtains a list of enabled tests from the test package on the device,
+    then filters it again using the diabled list on the host.

[bulach, 2012/12/13 14:55:21]

I dislike this combination :)
the while + try is tricky enough, and there's no need to make it even more
complex by combining with the list from the host here...
keep this function as only getting things from the device itself, then create
another wrapper to filter the list..

also, in general, such changes could be made in separate patches, that is, one
for removing the baseline option, another one for improving these other bits..

[frankf, 2012/12/13 19:54:04]

Done.

On 2012/12/13 14:55:21, bulach wrote:

 
-    Tries to obtain the list of available tests.
     Raises Exception if all devices failed.
     """
     available_devices = list(self.attached_devices)
     while available_devices:
       try:
-        logging.info('Obtaining tests from %s', available_devices[-1])
-        all_tests = self._GetTestsFromDevice(available_devices[-1])
-        return all_tests, available_devices
+        current_device = available_devices[-1]
+        logging.info('Obtaining tests from %s', current_device)
+        test_runner = SingleTestRunner(
+            current_device,
+            self.test_suite,
+            self.gtest_filter,
+            self.test_arguments,
+            self.timeout,
+            self.performance_test,
+            self.cleanup_test_files,
+            self.tool,
+            0,
+            not not self.log_dump_name,

[Yaron, 2012/12/13 00:04:34]

cute

+            self.fast_and_loose,
+            self.build_type,
+            self.in_webkit_checkout)
+        # The executable/apk needs to be copied before we can call GetAllTests.
+        test_runner.test_package.StripAndCopyExecutable()
+        all_tests = test_runner.test_package.GetAllTests()
+        disabled_list = test_runner.GetDisabledTests()
+        # Only includes tests that do not have any match in the disabled list.
+        all_tests = filter(lambda t:
+                           not any([fnmatch.fnmatch(t, disabled_pattern)
+                                    for disabled_pattern in disabled_list]),
+                           all_tests)
+        return all_tests
       except Exception as e:
-        logging.info('Failed obtaining tests from %s %s',
-                     available_devices[-1], e)
+        logging.warning('Failed obtaining tests from %s %s',
+                        current_device, e)
         available_devices.pop()
-    raise Exception('No device available to get the list of tests.')
 
-  def _GetTestsFromDevice(self, device):
-    test = SingleTestRunner(device, self.test_suite, self.gtest_filter,
-                            self.test_arguments, self.timeout, self.rebaseline,
-                            self.performance_test, self.cleanup_test_files,
-                            self.tool, 0,
-                            not not self.log_dump_name, self.fast_and_loose,
-                            self.build_type, self.in_webkit_checkout)
-    # The executable/apk needs to be copied before we can call GetAllTests.
-    test.test_package.StripAndCopyExecutable()
-    all_tests = test.test_package.GetAllTests()
-    if not self.rebaseline:
-      disabled_list = test.GetDisabledTests()
-      # Only includes tests that do not have any match in the disabled list.
-      all_tests = filter(lambda t:
-                         not any([fnmatch.fnmatch(t, disabled_pattern)
-                                  for disabled_pattern in disabled_list]),
-                         all_tests)
-    return all_tests
+    raise Exception('No device available to get the list of tests.')
 
   def CreateShardedTestRunner(self, device, index):
     """Creates a suite-specific test runner.
@@ -262,24 +264,29 @@ class TestSharder(BaseTestSharder):
     shard_size = (len(self.tests) + device_num - 1) / device_num
     shard_test_list = self.tests[index * shard_size : (index + 1) * shard_size]
     test_filter = ':'.join(shard_test_list) + self.gtest_filter
-    return SingleTestRunner(device, self.test_suite,
-                            test_filter, self.test_arguments, self.timeout,
-                            self.rebaseline, self.performance_test,
-                            self.cleanup_test_files, self.tool, index,
-                            not not self.log_dump_name, self.fast_and_loose,
-                            self.build_type, self.in_webkit_checkout)
+    return SingleTestRunner(
+        device,
+        self.test_suite,
+        test_filter,
+        self.test_arguments,
+        self.timeout,
+        self.performance_test,
+        self.cleanup_test_files, self.tool, index,
+        not not self.log_dump_name,
+        self.fast_and_loose,
+        self.build_type,
+        self.in_webkit_checkout)
 
   def OnTestsCompleted(self, test_runners, test_results):
     """Notifies that we completed the tests."""
     test_results.LogFull('Unit test', os.path.basename(self.test_suite),
-                         self.build_type, self.tests)
+                         self.build_type, self.all_tests)
     test_results.PrintAnnotation()
-    if test_results.failed and self.rebaseline:
-      test_runners[0].UpdateFilter(test_results.failed)
     if self.log_dump_name:
       # Zip all debug info outputs into a file named by log_dump_name.
       debug_info.GTestDebugInfo.ZipAndCleanResults(
-          os.path.join(cmd_helper.OutDirectory.get(), self.build_type,
+          os.path.join(
+              cmd_helper.OutDirectory.get(), self.build_type,
               'debug_info_dumps'),
           self.log_dump_name)
 
@@ -305,7 +312,7 @@ def _RunATestSuite(options):
   if options.use_emulator:
     for n in range(options.emulator_count):
       t = TimeProfile('Emulator launch %d' % n)
-      avd_name =  None
+      avd_name = None
       if n > 0:
         # Creates a temporary AVD for the extra emulators.
         avd_name = 'run_tests_avd_%d' % n
@@ -333,15 +340,22 @@ def _RunATestSuite(options):
     raise Exception('Failed to reset test server port.')
 
   if options.performance_test or options.gtest_filter:
-    # These configuration can't be split in multiple devices.
+    logging.warning('Sharding is not possible with these configurations.')
     attached_devices = [attached_devices[0]]
-  sharder = TestSharder(attached_devices, options.test_suite,
-                        options.gtest_filter, options.test_arguments,
-                        options.timeout, options.rebaseline,
-                        options.performance_test,
-                        options.cleanup_test_files, options.tool,
-                        options.log_dump, options.fast_and_loose,
-                        options.build_type, options.webkit)
+
+  sharder = TestSharder(
+      attached_devices,
+      options.test_suite,
+      options.gtest_filter,
+      options.test_arguments,
+      options.timeout,
+      options.performance_test,
+      options.cleanup_test_files,
+      options.tool,
+      options.log_dump,
+      options.fast_and_loose,
+      options.build_type,
+      options.webkit)
   test_results = sharder.RunShardedTests()
 
   for buildbot_emulator in buildbot_emulators:
@@ -402,9 +416,6 @@ def main(argv):
                            'the build type. Only for non-Chromium uses.')
   option_parser.add_option('-d', '--device', dest='test_device',
                            help='Target device the test suite to run ')
-  option_parser.add_option('-r', dest='rebaseline',
-                           help='Rebaseline and update *testsuite_disabled',
-                           action='store_true')
   option_parser.add_option('-f', '--gtest_filter', dest='gtest_filter',
                            help='gtest filter')
   option_parser.add_option('-a', '--test_arguments', dest='test_arguments',