[Android] Clean up gtest filtering logic.

Apply gtest_filter option to tests obtained from the device.

BUG=246871
NOTRY=True

Review URL: https://chromiumcodereview.appspot.com/19479002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@212229 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 110 insertions, 104 deletions

diff --git a/build/android/pylib/browsertests/dispatch.py b/build/android/pylib/browsertests/dispatch.py
index dcd8648..28ee318 100644
--- a/build/android/pylib/browsertests/dispatch.py
+++ b/build/android/pylib/browsertests/dispatch.py
@@ -19,10 +19,6 @@ from pylib.gtest import dispatch as gtest_dispatch
 from pylib.gtest import test_runner
 from pylib.utils import report_results
 
-sys.path.insert(0,
-                os.path.join(constants.DIR_SOURCE_ROOT, 'build', 'util', 'lib'))
-from common import unittest_util
-
 
 def Dispatch(options):
   """Dispatches all content_browsertests.
@@ -75,21 +71,16 @@ def Dispatch(options):
         constants.BROWSERTEST_TEST_ACTIVITY_NAME,
         constants.BROWSERTEST_COMMAND_LINE_FILE)
 
-  # Get tests and split them up based on the number of devices.
-  all_enabled = gtest_dispatch.GetAllEnabledTests(RunnerFactory,
-                                                  attached_devices)
-  if options.test_filter:
-    all_tests = unittest_util.FilterTestNames(all_enabled,
-                                              options.test_filter)
-  else:
-    all_tests = _FilterTests(all_enabled)
+  tests = gtest_dispatch.GetTestsFiltered(
+      constants.BROWSERTEST_SUITE_NAME, options.test_filter, RunnerFactory,
+      attached_devices)
 
   # Run tests.
   # TODO(nileshagrawal): remove this abnormally long setup timeout once fewer
   # files are pushed to the devices for content_browsertests: crbug.com/138275
   setup_timeout = 20 * 60  # 20 minutes
   test_results, exit_code = shard.ShardAndRunTests(
-      RunnerFactory, attached_devices, all_tests, options.build_type,
+      RunnerFactory, attached_devices, tests, options.build_type,
       setup_timeout=setup_timeout, test_timeout=None,
       num_retries=options.num_retries)
   report_results.LogFull(
@@ -103,21 +94,3 @@ def Dispatch(options):
     shutil.rmtree(constants.ISOLATE_DEPS_DIR)
 
   return (test_results, exit_code)
-
-
-def _FilterTests(all_enabled_tests):
-  """Filters out tests and fixtures starting with PRE_ and MANUAL_."""
-  return [t for t in all_enabled_tests if _ShouldRunOnBot(t)]
-
-
-def _ShouldRunOnBot(test):
-  fixture, case = test.split('.', 1)
-  if _StartsWith(fixture, case, 'PRE_'):
-    return False
-  if _StartsWith(fixture, case, 'MANUAL_'):
-    return False
-  return True
-
-
-def _StartsWith(a, b, prefix):
-  return a.startswith(prefix) or b.startswith(prefix)
diff --git a/build/android/pylib/gtest/dispatch.py b/build/android/pylib/gtest/dispatch.py
index bf88bff..eea09a9 100644
--- a/build/android/pylib/gtest/dispatch.py
+++ b/build/android/pylib/gtest/dispatch.py
@@ -10,6 +10,7 @@ import glob
 import logging
 import os
 import shutil
+import sys
 
 from pylib import android_commands
 from pylib import cmd_helper
@@ -24,9 +25,11 @@ from pylib.utils import xvfb
 import gtest_config
 import test_runner
 
+sys.path.insert(0,
+                os.path.join(constants.DIR_SOURCE_ROOT, 'build', 'util', 'lib'))
+from common import unittest_util
+
 
-# TODO(frankf): Add more test targets here after making sure we don't
-# blow up the dependency size (and the world).
 _ISOLATE_FILE_PATHS = {
     'base_unittests': 'base/base_unittests.isolate',
     'breakpad_unittests': 'breakpad/breakpad_unittests.isolate',
@@ -71,7 +74,7 @@ def _GenerateDepsDirUsingIsolate(test_suite, build_type):
   """Generate the dependency dir for the test suite using isolate.
 
   Args:
-    test_suite: The test suite basename (e.g. base_unittests).
+    test_suite: Name of the test suite (e.g. base_unittests).
     build_type: Release/Debug
   """
   product_dir = os.path.join(cmd_helper.OutDirectory.get(), build_type)
@@ -191,51 +194,115 @@ def _FullyQualifiedTestSuites(exe, option_test_suite, build_type):
   return qualified_test_suites
 
 
-def GetTestsFromDevice(runner):
-  """Get a list of tests from a device, excluding disabled tests.
+def _GetDisabledTestsFilterFromFile(test_suite):
+  """Returns a gtest filter based on the *_disabled file.
 
   Args:
-    runner: a TestRunner.
+    test_suite: Name of the test suite (e.g. base_unittests).
+
   Returns:
-    All non-disabled tests on the device.
+    A gtest filter which excludes disabled tests.
+    Example: '*-StackTrace.*:StringPrintfTest.StringPrintfMisc'
   """
-  # The executable/apk needs to be copied before we can call GetAllTests.
-  runner.test_package.Install()
-  all_tests = runner.test_package.GetAllTests()
-  # Only includes tests that do not have any match in the disabled list.
-  disabled_list = runner.GetDisabledTests()
-  return filter(lambda t: not any([fnmatch.fnmatch(t, disabled_pattern)
-                                   for disabled_pattern in disabled_list]),
-                all_tests)
+  filter_file_path = os.path.join(
+      os.path.abspath(os.path.dirname(__file__)),
+      'filter', '%s_disabled' % test_suite)
 
+  if not filter_file_path or not os.path.exists(filter_file_path):
+    logging.info('No filter file found at %s', filter_file_path)
+    return '*'
 
-def GetAllEnabledTests(runner_factory, devices):
-  """Get all enabled tests.
+  filters = [x for x in [x.strip() for x in file(filter_file_path).readlines()]
+             if x and x[0] != '#']
+  disabled_filter = '*-%s' % ':'.join(filters)
+  logging.info('Applying filter "%s" obtained from %s',
+               disabled_filter, filter_file_path)
+  return disabled_filter
 
-  Obtains a list of enabled tests from the test package on the device,
-  then filters it again using the disabled list on the host.
+
+def _GetTestsFromDevice(runner_factory, devices):
+  """Get a list of tests from a device.
 
   Args:
-    runner_factory: callable that takes a devices and returns a TestRunner.
-    devices: list of devices.
+    runner_factory: callable that takes a device and index and returns a
+      TestRunner object.
+    devices: List of devices.
 
   Returns:
-    List of all enabled tests.
-
-  Raises:
-    Exception: If no devices available.
+    All the tests in the test suite.
   """
   for device in devices:
     try:
       logging.info('Obtaining tests from %s', device)
       runner = runner_factory(device, 0)
-      return GetTestsFromDevice(runner)
-    except Exception as e:
+      runner.test_package.Install()
+      return runner.test_package.GetAllTests()
+    except (android_commands.errors.WaitForResponseTimedOutError,
+            android_commands.errors.DeviceUnresponsiveError), e:
       logging.warning('Failed obtaining tests from %s with exception: %s',
                       device, e)
   raise Exception('No device available to get the list of tests.')
 
 
+def _FilterTestsUsingPrefixes(all_tests, pre=False, manual=False):
+  """Removes tests with disabled prefixes.
+
+  Args:
+    all_tests: List of tests to filter.
+    pre: If True, include tests with _PRE prefix.
+    manual: If True, include tests with _MANUAL prefix.
+
+  Returns:
+    List of tests remaining.
+  """
+  filtered_tests = []
+  filter_prefixes = ['DISABLED_', 'FLAKY_', 'FAILS_']
+
+  if not pre:
+    filter_prefixes.append('PRE_')
+
+  if not manual:
+    filter_prefixes.append('MANUAL_')
+
+  for t in all_tests:
+    test_case, test = t.split('.', 1)
+    if not any([test_case.startswith(prefix) or test.startswith(prefix) for
+                prefix in filter_prefixes]):
+      filtered_tests.append(t)
+  return filtered_tests
+
+
+def GetTestsFiltered(test_suite, gtest_filter, runner_factory, devices):
+  """Get all tests in the suite and filter them.
+
+  Obtains a list of tests from the test package on the device, and
+  applies the following filters in order:
+    1. Remove tests with disabled prefixes.
+    2. Remove tests specified in the *_disabled files in the 'filter' dir
+    3. Applies |gtest_filter|.
+
+  Args:
+    test_suite: Name of the test suite (e.g. base_unittests).
+    gtest_filter: A filter including negative and/or positive patterns.
+    runner_factory: callable that takes a device and index and returns a
+      TestRunner object.
+    devices: List of devices.
+
+  Returns:
+    List of tests remaining.
+  """
+  tests = _GetTestsFromDevice(runner_factory, devices)
+  tests = _FilterTestsUsingPrefixes(
+      tests, bool(gtest_filter), bool(gtest_filter))
+  tests = unittest_util.FilterTestNames(
+      tests, _GetDisabledTestsFilterFromFile(test_suite))
+
+  if gtest_filter:
+    tests = unittest_util.FilterTestNames(tests, gtest_filter)
+
+  return tests
+
+
 def _RunATestSuite(options, suite_name):
   """Run a single test suite.
 
@@ -294,12 +361,10 @@ def _RunATestSuite(options, suite_name):
         constants.GTEST_COMMAND_LINE_FILE)
 
   # Get tests and split them up based on the number of devices.
-  if options.test_filter:
-    all_tests = [t for t in options.test_filter.split(':') if t]
-  else:
-    all_tests = GetAllEnabledTests(RunnerFactory, attached_devices)
+  tests = GetTestsFiltered(suite_name, options.test_filter,
+                           RunnerFactory, attached_devices)
   num_devices = len(attached_devices)
-  tests = [':'.join(all_tests[i::num_devices]) for i in xrange(num_devices)]
+  tests = [':'.join(tests[i::num_devices]) for i in xrange(num_devices)]
   tests = [t for t in tests if t]
 
   # Run tests.
diff --git a/build/android/pylib/gtest/test_package.py b/build/android/pylib/gtest/test_package.py
index 4846322..4cccea1 100644
--- a/build/android/pylib/gtest/test_package.py
+++ b/build/android/pylib/gtest/test_package.py
@@ -61,14 +61,11 @@ class TestPackage(object):
     """Install the test package to the device."""
     raise NotImplementedError('Method must be overriden.')
 
-  def GetDisabledPrefixes(self):
-    return ['DISABLED_', 'FLAKY_', 'FAILS_']
-
-  def _ParseGTestListTests(self, all_tests):
-    """Parses and filters the raw test lists.
+  def _ParseGTestListTests(self, raw_list):
+    """Parses a raw test list as provided by --gtest_list_tests.
 
     Args:
-      all_tests: The raw test listing with the following format:
+      raw_list: The raw test listing with the following format:
 
       IPCChannelTest.
         SendMessageInChannelConnected
@@ -77,14 +74,14 @@ class TestPackage(object):
         DISABLED_SendWithTimeoutMixedOKAndTimeout
 
     Returns:
-      A list of non-disabled tests. For the above raw listing:
+      A list of all tests. For the above raw listing:
 
-      [IPCChannelTest.SendMessageInChannelConnected, IPCSyncChannelTest.Simple]
+      [IPCChannelTest.SendMessageInChannelConnected, IPCSyncChannelTest.Simple,
+       IPCSyncChannelTest.DISABLED_SendWithTimeoutMixedOKAndTimeout]
     """
     ret = []
     current = ''
-    disabled_prefixes = self.GetDisabledPrefixes()
-    for test in all_tests:
+    for test in raw_list:
       if not test:
         continue
       if test[0] != ' ' and not test.endswith('.'):
@@ -96,6 +93,5 @@ class TestPackage(object):
       if 'YOU HAVE' in test:
         break
       test_name = test[2:]
-      if not any([test_name.startswith(x) for x in disabled_prefixes]):
-        ret += [current + test_name]
+      ret += [current + test_name]
     return ret
diff --git a/build/android/pylib/gtest/test_runner.py b/build/android/pylib/gtest/test_runner.py
index 159d66a..6bb7179 100644
--- a/build/android/pylib/gtest/test_runner.py
+++ b/build/android/pylib/gtest/test_runner.py
@@ -49,7 +49,6 @@ class TestRunner(base_test_runner.BaseTestRunner):
     """
     super(TestRunner, self).__init__(device, tool_name, build_type, push_deps,
                                      cleanup_test_files)
-    self._running_on_emulator = self.device.startswith('emulator')
     self._test_arguments = test_arguments
     self.in_webkit_checkout = in_webkit_checkout
     if timeout == 0:
@@ -126,25 +125,6 @@ class TestRunner(base_test_runner.BaseTestRunner):
         os.path.join(self.adb.GetExternalStorage(),
                      'third_party/hyphen/hyph_en_US.dic'))
 
-  # TODO(craigdh): There is no reason for this to be part of TestRunner.
-  def GetDisabledTests(self):
-    """Returns a list of disabled tests.
-
-    Returns:
-      A list of disabled tests obtained from 'filter' subdirectory.
-    """
-    gtest_filter_base_path = os.path.join(
-        os.path.abspath(os.path.dirname(__file__)),
-        'filter',
-        self.test_package.test_suite_basename)
-    disabled_tests = run_tests_helper.GetExpectations(
-       gtest_filter_base_path + '_disabled')
-    if self._running_on_emulator:
-      # Append emulator's filter file.
-      disabled_tests.extend(run_tests_helper.GetExpectations(
-          gtest_filter_base_path + '_emulator_additional_disabled'))
-    return disabled_tests
-
   def _ParseTestOutput(self, p):
     """Process the test output.
 
diff --git a/build/android/pylib/utils/run_tests_helper.py b/build/android/pylib/utils/run_tests_helper.py
index eefc03b..60823fc 100644
--- a/build/android/pylib/utils/run_tests_helper.py
+++ b/build/android/pylib/utils/run_tests_helper.py
@@ -31,14 +31,6 @@ class CustomFormatter(logging.Formatter):
     return '%s %ss %s' % (record.levelname[0], timediff.rjust(4), msg)
 
 
-def GetExpectations(file_name):
-  """Returns a list of test names in the |file_name| test expectations file."""
-  if not file_name or not os.path.exists(file_name):
-    return []
-  return [x for x in [x.strip() for x in file(file_name).readlines()]
-          if x and x[0] != '#']
-
-
 def SetLogLevel(verbose_count):
   """Sets log level as |verbose_count|."""
   log_level = logging.WARNING  # Default.
diff --git a/build/android/test_runner.py b/build/android/test_runner.py
index 08bd416..905174d 100755
--- a/build/android/test_runner.py
+++ b/build/android/test_runner.py
@@ -126,7 +126,7 @@ def AddCoreGTestOptions(option_parser, default_timeout=60):
   # TODO(gkanwar): Consolidate and clean up test filtering for gtests and
   # content_browsertests.
   option_parser.add_option('--gtest_filter', dest='test_filter',
-                           help='Filter GTests by name.')
+                           help='googletest-style filter string.')
   option_parser.add_option('-a', '--test_arguments', dest='test_arguments',
                            help='Additional arguments to pass to the test.')
   # TODO(gkanwar): Most likely deprecate/remove this option once we've pinned