android: generalize support for --test-launcher-filter-file.

This adds filter file support to all test types that previously
supported gtest-style test filtering, notably including instrumentation
and junit tests.

This was requested in crrev.com/c/1208442

Change-Id: I552843898735b52b773bdc397c39ecf64e4df275
Reviewed-on: https://chromium-review.googlesource.com/1214083
Commit-Queue: John Budorick <jbudorick@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589682}

build/android/PRESUBMIT.py

@@ -79,6 +79,7 @@ def CommonChecks(input_api, output_api):
           J('pylib', 'utils', 'device_dependencies_test.py'),
           J('pylib', 'utils', 'dexdump_test.py'),
           J('pylib', 'utils', 'proguard_test.py'),
+          J('pylib', 'utils', 'test_filter_test.py'),
       ],
       env=pylib_test_env))
 

build/android/pylib/gtest/gtest_test_instance.py

@@ -16,6 +16,7 @@ from pylib.constants import host_paths
 from pylib.base import base_test_result
 from pylib.base import test_instance
 from pylib.symbols import stack_symbolizer
+from pylib.utils import test_filter
 
 with host_paths.SysPath(host_paths.BUILD_COMMON_PATH):
   import unittest_util # pylint: disable=import-error
@@ -234,35 +235,6 @@ def ParseGTestXML(xml_content):
   return results
 
 
-def ConvertTestFilterFileIntoGTestFilterArgument(input_lines):
-  """Converts test filter file contents into --gtest_filter argument.
-
-  See //testing/buildbot/filters/README.md for description of the
-  syntax that |input_lines| are expected to follow.
-
-  See
-  https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#running-a-subset-of-the-tests
-  for description of the syntax that --gtest_filter argument should follow.
-
-  Args:
-    input_lines: An iterable (e.g. a list or a file) containing input lines.
-  Returns:
-    a string suitable for feeding as an argument of --gtest_filter parameter.
-  """
-  # Strip comments and whitespace from each line and filter non-empty lines.
-  stripped_lines = (l.split('#', 1)[0].strip() for l in input_lines)
-  filter_lines = list(l for l in stripped_lines if l)
-
-  # Split the tests into positive and negative patterns (gtest treats
-  # every pattern after the first '-' sign as an exclusion).
-  positive_patterns = ':'.join(l for l in filter_lines if l[0] != '-')
-  negative_patterns = ':'.join(l[1:] for l in filter_lines if l[0] == '-')
-  if negative_patterns:
-    negative_patterns = '-' + negative_patterns
-
-  # Join the filter lines into one, big --gtest_filter argument.
-  return positive_patterns + negative_patterns
-
 def TestNameWithoutDisabledPrefix(test_name):
   """Modify the test name without disabled prefix if prefix 'DISABLED_' or
   'FLAKY_' presents.
@@ -338,14 +310,7 @@ class GtestTestInstance(test_instance.TestInstance):
       error_func('Could not find apk or executable for %s' % self._suite)
 
     self._data_deps = []
-    if args.test_filter:
-      self._gtest_filter = args.test_filter
-    elif args.test_filter_file:
-      with open(args.test_filter_file, 'r') as f:
-        self._gtest_filter = ConvertTestFilterFileIntoGTestFilterArgument(f)
-    else:
-      self._gtest_filter = None
-
+    self._gtest_filter = test_filter.InitializeFilterFromArgs(args)
     self._run_disabled = args.run_disabled
 
     self._data_deps_delegate = data_deps_delegate

build/android/pylib/gtest/gtest_test_instance_test.py

@@ -181,51 +181,6 @@ class GtestTestInstanceTests(unittest.TestCase):
     actual = gtest_test_instance.ParseGTestXML(None)
     self.assertEquals([], actual)
 
-  def testConvertTestFilterFile_commentsAndBlankLines(self):
-    input_lines = [
-      'positive1',
-      '# comment',
-      'positive2  # Another comment',
-      ''
-      'positive3'
-    ]
-    actual = gtest_test_instance \
-        .ConvertTestFilterFileIntoGTestFilterArgument(input_lines)
-    expected = 'positive1:positive2:positive3'
-    self.assertEquals(expected, actual)
-
-  def testConvertTestFilterFile_onlyPositive(self):
-    input_lines = [
-      'positive1',
-      'positive2'
-    ]
-    actual = gtest_test_instance \
-        .ConvertTestFilterFileIntoGTestFilterArgument(input_lines)
-    expected = 'positive1:positive2'
-    self.assertEquals(expected, actual)
-
-  def testConvertTestFilterFile_onlyNegative(self):
-    input_lines = [
-      '-negative1',
-      '-negative2'
-    ]
-    actual = gtest_test_instance \
-        .ConvertTestFilterFileIntoGTestFilterArgument(input_lines)
-    expected = '-negative1:negative2'
-    self.assertEquals(expected, actual)
-
-  def testConvertTestFilterFile_positiveAndNegative(self):
-    input_lines = [
-      'positive1',
-      'positive2',
-      '-negative1',
-      '-negative2'
-    ]
-    actual = gtest_test_instance \
-        .ConvertTestFilterFileIntoGTestFilterArgument(input_lines)
-    expected = 'positive1:positive2-negative1:negative2'
-    self.assertEquals(expected, actual)
-
   def testTestNameWithoutDisabledPrefix_disabled(self):
     test_name_list = [
       'A.DISABLED_B',

build/android/pylib/instrumentation/instrumentation_test_instance.py

@@ -22,6 +22,7 @@ from pylib.utils import dexdump
 from pylib.utils import instrumentation_tracing
 from pylib.utils import proguard
 from pylib.utils import shared_preference_utils
+from pylib.utils import test_filter
 
 
 with host_paths.SysPath(host_paths.BUILD_COMMON_PATH):
@@ -62,8 +63,6 @@ _TEST_LIST_JUNIT4_RUNNERS = [
 _SKIP_PARAMETERIZATION = 'SkipCommandLineParameterization'
 _COMMANDLINE_PARAMETERIZATION = 'CommandLineParameter'
 _NATIVE_CRASH_RE = re.compile('(process|native) crash', re.IGNORECASE)
-_CMDLINE_NAME_SEGMENT_RE = re.compile(
-    r' with(?:out)? \{[^\}]*\}')
 _PICKLE_FORMAT_VERSION = 12
 
 
@@ -181,7 +180,7 @@ def GenerateTestResults(
   return results
 
 
-def FilterTests(tests, test_filter=None, annotations=None,
+def FilterTests(tests, filter_str=None, annotations=None,
                 excluded_annotations=None):
   """Filter a list of tests
 
@@ -189,7 +188,7 @@ def FilterTests(tests, test_filter=None, annotations=None,
     tests: a list of tests. e.g. [
            {'annotations": {}, 'class': 'com.example.TestA', 'method':'test1'},
            {'annotations": {}, 'class': 'com.example.TestB', 'method':'test2'}]
-    test_filter: googletest-style filter string.
+    filter_str: googletest-style filter string.
     annotations: a dict of wanted annotations for test methods.
     exclude_annotations: a dict of annotations to exclude.
 
@@ -197,7 +196,7 @@ def FilterTests(tests, test_filter=None, annotations=None,
     A list of filtered tests
   """
   def gtest_filter(t):
-    if not test_filter:
+    if not filter_str:
       return True
     # Allow fully-qualified name as well as an omitted package.
     unqualified_class_test = {
@@ -216,7 +215,7 @@ def FilterTests(tests, test_filter=None, annotations=None,
           GetTestNameWithoutParameterPostfix(unqualified_class_test, sep='.')
       ]
 
-    pattern_groups = test_filter.split('-')
+    pattern_groups = filter_str.split('-')
     if len(pattern_groups) > 1:
       negative_filter = pattern_groups[1]
       if unittest_util.FilterTestNames(names, negative_filter):
@@ -385,9 +384,9 @@ class MissingJUnit4RunnerException(test_exception.TestException):
 class UnmatchedFilterException(test_exception.TestException):
   """Raised when a user specifies a filter that doesn't match any tests."""
 
-  def __init__(self, test_filter):
+  def __init__(self, filter_str):
     super(UnmatchedFilterException, self).__init__(
-        'Test filter "%s" matched no tests.' % test_filter)
+        'Test filter "%s" matched no tests.' % filter_str)
 
 
 def GetTestName(test, sep='#'):
@@ -622,9 +621,7 @@ class InstrumentationTestInstance(test_instance.TestInstance):
       logging.warning('No data dependencies will be pushed.')
 
   def _initializeTestFilterAttributes(self, args):
-    if args.test_filter:
-      self._test_filter = _CMDLINE_NAME_SEGMENT_RE.sub(
-          '', args.test_filter.replace('#', '.'))
+    self._test_filter = test_filter.InitializeFilterFromArgs(args)
 
     def annotation_element(a):
       a = a.split('=', 1)

build/android/pylib/junit/junit_test_instance.py

@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 from pylib.base import test_instance
+from pylib.utils import test_filter
 
 
 class JunitTestInstance(test_instance.TestInstance):
@@ -18,7 +19,7 @@ class JunitTestInstance(test_instance.TestInstance):
     self._resource_zips = args.resource_zips
     self._robolectric_runtime_deps_dir = args.robolectric_runtime_deps_dir
     self._runner_filter = args.runner_filter
-    self._test_filter = args.test_filter
+    self._test_filter = test_filter.InitializeFilterFromArgs(args)
     self._test_suite = args.test_suite
 
   #override

build/android/pylib/linker/linker_test_instance.py

@@ -5,6 +5,7 @@
 from pylib.base import test_instance
 from pylib.constants import host_paths
 from pylib.linker import test_case
+from pylib.utils import test_filter
 
 with host_paths.SysPath(host_paths.BUILD_COMMON_PATH):
   import unittest_util
@@ -15,7 +16,7 @@ class LinkerTestInstance(test_instance.TestInstance):
   def __init__(self, args):
     super(LinkerTestInstance, self).__init__()
     self._test_apk = args.test_apk
-    self._test_filter = args.test_filter
+    self._test_filter = test_filter.InitializeFilterFromArgs(args)
 
   @property
   def test_apk(self):

build/android/pylib/perf/perf_test_instance.py

@@ -14,6 +14,7 @@ from pylib import constants
 from pylib.base import base_test_result
 from pylib.base import test_instance
 from pylib.constants import host_paths
+from pylib.utils import test_filter
 
 
 _GIT_CR_POS_RE = re.compile(r'^Cr-Commit-Position: refs/heads/master@{#(\d+)}$')
@@ -77,7 +78,7 @@ class PerfTestInstance(test_instance.TestInstance):
     self._single_step = (
         ' '.join(args.single_step_command) if args.single_step else None)
     self._steps = args.steps
-    self._test_filter = args.test_filter
+    self._test_filter = test_filter.InitializeFilterFromArgs(args)
     self._write_buildbot_json = args.write_buildbot_json
 
   #override

build/android/pylib/utils/test_filter.py

+# Copyright 2018 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 os
+import re
+
+
+_CMDLINE_NAME_SEGMENT_RE = re.compile(
+    r' with(?:out)? \{[^\}]*\}')
+
+
+def ParseFilterFile(input_lines):
+  """Converts test filter file contents into --gtest_filter argument.
+
+  See //testing/buildbot/filters/README.md for description of the
+  syntax that |input_lines| are expected to follow.
+
+  See
+  https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#running-a-subset-of-the-tests
+  for description of the syntax that --gtest_filter argument should follow.
+
+  Args:
+    input_lines: An iterable (e.g. a list or a file) containing input lines.
+  Returns:
+    a string suitable for feeding as an argument of --gtest_filter parameter.
+  """
+  # Strip comments and whitespace from each line and filter non-empty lines.
+  stripped_lines = (l.split('#', 1)[0].strip() for l in input_lines)
+  filter_lines = [l for l in stripped_lines if l]
+
+  # Split the tests into positive and negative patterns (gtest treats
+  # every pattern after the first '-' sign as an exclusion).
+  positive_patterns = ':'.join(l for l in filter_lines if l[0] != '-')
+  negative_patterns = ':'.join(l[1:] for l in filter_lines if l[0] == '-')
+  if negative_patterns:
+    negative_patterns = '-' + negative_patterns
+
+  # Join the filter lines into one, big --gtest_filter argument.
+  return positive_patterns + negative_patterns
+
+
+def AddFilterOptions(parser):
+  """Adds filter command-line options to the provided parser.
+
+  Args:
+    parser: an argparse.ArgumentParser instance.
+  """
+  filter_group = parser.add_mutually_exclusive_group()
+  filter_group.add_argument(
+      '-f', '--test-filter', '--gtest_filter', '--gtest-filter',
+      dest='test_filter',
+      help='googletest-style filter string.',
+      default=os.environ.get('GTEST_FILTER'))
+  filter_group.add_argument(
+      # Deprecated argument.
+      '--gtest-filter-file',
+      # New argument.
+      '--test-launcher-filter-file',
+      dest='test_filter_file', type=os.path.realpath,
+      help='Path to file that contains googletest-style filter strings. '
+           'See also //testing/buildbot/filters/README.md.')
+
+
+def InitializeFilterFromArgs(args):
+  """Returns a filter string from the command-line option values.
+
+  Args:
+    args: an argparse.Namespace instance resulting from a using parser
+      to which the filter options above were added.
+  """
+  parsed_filter = None
+  if args.test_filter:
+    parsed_filter = _CMDLINE_NAME_SEGMENT_RE.sub(
+        '', args.test_filter.replace('#', '.'))
+  elif args.test_filter_file:
+    with open(args.test_filter_file, 'r') as f:
+      parsed_filter = ParseFilterFile(f)
+
+  return parsed_filter

build/android/pylib/utils/test_filter_test.py

+#!/usr/bin/env vpython
+# Copyright 2018 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 sys
+import unittest
+
+from pylib.utils import test_filter
+
+class ParseFilterFileTest(unittest.TestCase):
+
+  def testParseFilterFile_commentsAndBlankLines(self):
+    input_lines = [
+      'positive1',
+      '# comment',
+      'positive2  # Another comment',
+      ''
+      'positive3'
+    ]
+    actual = test_filter.ParseFilterFile(input_lines)
+    expected = 'positive1:positive2:positive3'
+    self.assertEquals(expected, actual)
+
+  def testParseFilterFile_onlyPositive(self):
+    input_lines = [
+      'positive1',
+      'positive2'
+    ]
+    actual = test_filter.ParseFilterFile(input_lines)
+    expected = 'positive1:positive2'
+    self.assertEquals(expected, actual)
+
+  def testParseFilterFile_onlyNegative(self):
+    input_lines = [
+      '-negative1',
+      '-negative2'
+    ]
+    actual = test_filter.ParseFilterFile(input_lines)
+    expected = '-negative1:negative2'
+    self.assertEquals(expected, actual)
+
+  def testParseFilterFile_positiveAndNegative(self):
+    input_lines = [
+      'positive1',
+      'positive2',
+      '-negative1',
+      '-negative2'
+    ]
+    actual = test_filter.ParseFilterFile(input_lines)
+    expected = 'positive1:positive2-negative1:negative2'
+    self.assertEquals(expected, actual)
+
+
+if __name__ == '__main__':
+  sys.exit(unittest.main())

build/android/test_runner.py

@@ -47,6 +47,7 @@ from pylib.results import report_results
 from pylib.results.presentation import test_results_presentation
 from pylib.utils import logdog_helper
 from pylib.utils import logging_utils
+from pylib.utils import test_filter
 
 from py_utils import contextlib_ext
 
@@ -92,6 +93,8 @@ def AddTestLauncherOptions(parser):
       type=int, default=os.environ.get('GTEST_TOTAL_SHARDS', 1),
       help='Total number of external shards.')
 
+  test_filter.AddFilterOptions(parser)
+
   return parser
 
 
@@ -357,21 +360,6 @@ def AddGTestOptions(parser):
       help='Wait for java debugger to attach before running any application '
            'code. Also disables test timeouts and sets retries=0.')
 
-  filter_group = parser.add_mutually_exclusive_group()
-  filter_group.add_argument(
-      '-f', '--gtest_filter', '--gtest-filter',
-      dest='test_filter',
-      help='googletest-style filter string.',
-      default=os.environ.get('GTEST_FILTER'))
-  filter_group.add_argument(
-      # Deprecated argument.
-      '--gtest-filter-file',
-      # New argument.
-      '--test-launcher-filter-file',
-      dest='test_filter_file', type=os.path.realpath,
-      help='Path to file that contains googletest-style filter strings. '
-           'See also //testing/buildbot/filters/README.md.')
-
 
 def AddInstrumentationTestOptions(parser):
   """Adds Instrumentation test options to |parser|."""
@@ -418,11 +406,6 @@ def AddInstrumentationTestOptions(parser):
       dest='exclude_annotation_str',
       help='Comma-separated list of annotations. Exclude tests with these '
            'annotations.')
-  parser.add_argument(
-      '-f', '--test-filter', '--gtest_filter', '--gtest-filter',
-      dest='test_filter',
-      help='Test filter (if not fully qualified, will run all matches).',
-      default=os.environ.get('GTEST_FILTER'))
   parser.add_argument(
       '--gtest_also_run_disabled_tests', '--gtest-also-run-disabled-tests',
       dest='run_disabled', action='store_true',
@@ -521,9 +504,6 @@ def AddJUnitTestOptions(parser):
   parser.add_argument(
       '--runner-filter',
       help='Filters tests by runner class. Must be fully qualified.')
-  parser.add_argument(
-      '-f', '--test-filter',
-      help='Filters tests googletest-style.')
   parser.add_argument(
       '-s', '--test-suite', required=True,
       help='JUnit test suite to run.')
@@ -557,11 +537,6 @@ def AddLinkerTestOptions(parser):
 
   parser.add_argument_group('linker arguments')
 
-  parser.add_argument(
-      '-f', '--gtest-filter',
-      dest='test_filter',
-      help='googletest-style filter string.',
-      default=os.environ.get('GTEST_FILTER'))
   parser.add_argument(
       '--test-apk',
       type=os.path.realpath,
@@ -676,9 +651,6 @@ def AddPerfTestOptions(parser):
       help='Writes a JSON list of information for each --steps into the given '
            'file. Information includes runtime and device affinity for each '
            '--steps.')
-  parser.add_argument(
-      '-f', '--test-filter',
-      help='Test filter (will match against the names listed in --steps).')
   parser.add_argument(
       '--write-buildbot-json',
       action='store_true',

build/android/test_runner.pydeps

@@ -195,6 +195,7 @@ pylib/utils/logging_utils.py
 pylib/utils/proguard.py
 pylib/utils/repo_utils.py
 pylib/utils/shared_preference_utils.py
+pylib/utils/test_filter.py
 pylib/utils/time_profile.py
 pylib/valgrind_tools.py
 test_runner.py