[Android] Fix instrumentation test filtering for parameterized tests.

build/android/pylib/instrumentation/instrumentation_test_instance.py

 # Copyright 2015 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 collections
 import copy
 import logging
 import os
 import pickle
 import re
@@ skipping 32 matching lines @@
 _EXTRA_DRIVER_TARGET_PACKAGE = (
     'org.chromium.test.driver.OnDeviceInstrumentationDriver.TargetPackage')
 _EXTRA_DRIVER_TARGET_CLASS = (
     'org.chromium.test.driver.OnDeviceInstrumentationDriver.TargetClass')
 _EXTRA_TIMEOUT_SCALE = (
     'org.chromium.test.driver.OnDeviceInstrumentationDriver.TimeoutScale')
 
 _PARAMETERIZED_TEST_ANNOTATION = 'ParameterizedTest'
 _PARAMETERIZED_TEST_SET_ANNOTATION = 'ParameterizedTest$Set'
 _NATIVE_CRASH_RE = re.compile('(process|native) crash', re.IGNORECASE)
+_CMDLINE_NAME_SEGMENT_RE = re.compile(
+    r' with(?:out)? \{[^\}]*\}')

[mikecase (-- gone --), 2016/11/12 01:55:08]

so, I could easily be wrong, but with this work on tests with both positive and
negative flags

mTestClass#testA with {foo} without {bar}

which I think can happen

[jbudorick, 2016/11/14 16:26:32]

Yes, it should: https://docs.python.org/2/library/re.html#re.sub

 _PICKLE_FORMAT_VERSION = 10
 
 
 class MissingSizeAnnotationError(test_exception.TestException):
   def __init__(self, class_name):
     super(MissingSizeAnnotationError, self).__init__(class_name +
         ': Test method is missing required size annotation. Add one of: ' +
         ', '.join('@' + a for a in _VALID_ANNOTATIONS))
 
 
@@ skipping 150 matching lines @@
     tests: a list of tests. e.g. [
            {'annotations": {}, 'class': 'com.example.TestA', 'methods':[]},
            {'annotations": {}, 'class': 'com.example.TestB', 'methods':[]}]
     test_filter: googletest-style filter string.
     annotations: a dict of wanted annotations for test methods.
     exclude_annotations: a dict of annotations to exclude.
 
   Return:
     A list of filtered tests
   """
-  def gtest_filter(c, m):
+  def gtest_filter(t):
     if not test_filter:
       return True
     # Allow fully-qualified name as well as an omitted package.
-    names = ['%s.%s' % (c['class'], m['method']),
-             '%s.%s' % (c['class'].split('.')[-1], m['method'])]
+    unqualified_class_test = {

[mikecase (-- gone --), 2016/11/12 01:55:09]

unqualified_class_test is just like package-name-less test name right?

maybe change name to class_test_name_without_package. just kidding.

[jbudorick, 2016/11/14 16:26:32]

Acknowledged.

+      'class': t['class'].split('.')[-1],
+      'method': t['method']
+    }
+    names = [
+      GetTestName(t, sep='.'),
+      GetTestName(unqualified_class_test, sep='.'),
+      GetUniqueTestName(t, sep='.')
+    ]

[mikecase (-- gone --), 2016/11/12 01:55:09]

I like this solution.  :D

[jbudorick, 2016/11/14 16:26:32]

Yeah, it's a bit more readable this way.

     return unittest_util.FilterTestNames(names, test_filter)
 
   def annotation_filter(all_annotations):
     if not annotations:
       return True
     return any_annotation_matches(annotations, all_annotations)
 
   def excluded_annotation_filter(all_annotations):
     if not excluded_annotations:
       return True
     return not any_annotation_matches(excluded_annotations,
                                       all_annotations)
 
   def any_annotation_matches(filter_annotations, all_annotations):
     return any(
         ak in all_annotations
         and annotation_value_matches(av, all_annotations[ak])
         for ak, av in filter_annotations)
 
   def annotation_value_matches(filter_av, av):
     if filter_av is None:
       return True
     elif isinstance(av, dict):
       return filter_av in av['value']
     elif isinstance(av, list):
       return filter_av in av
     return filter_av == av
 
-  filtered_classes = []
-  for c in tests:
-    filtered_methods = []
-    for m in c['methods']:
-      # Gtest filtering
-      if not gtest_filter(c, m):
-        continue
+  filtered_tests = []
+  for t in tests:
+    # Gtest filtering
+    if not gtest_filter(t):
+      continue
 
-      all_annotations = dict(c['annotations'])
-      all_annotations.update(m['annotations'])
+    # Enforce that all tests declare their size.
+    if not any(a in _VALID_ANNOTATIONS for a in t['annotations']):
+      raise MissingSizeAnnotationError(GetTestName(t))
 
-      # Enforce that all tests declare their size.
-      if not any(a in _VALID_ANNOTATIONS for a in all_annotations):
-        raise MissingSizeAnnotationError('%s.%s' % (c['class'], m['method']))
+    if (not annotation_filter(t['annotations'])
+        or not excluded_annotation_filter(t['annotations'])):
+      continue
 
-      if (not annotation_filter(all_annotations)
-          or not excluded_annotation_filter(all_annotations)):
-        continue
+    filtered_tests.append(t)
 
-      filtered_methods.append(m)
+  return filtered_tests
 
-    if filtered_methods:
-      filtered_class = dict(c)
-      filtered_class['methods'] = filtered_methods
-      filtered_classes.append(filtered_class)
-
-  return filtered_classes
 
 def GetAllTests(test_jar):
   pickle_path = '%s-proguard.pickle' % test_jar
   try:
     tests = _GetTestsFromPickle(pickle_path, test_jar)
   except ProguardPickleException as e:
     logging.info('Could not get tests from pickle: %s', e)
     logging.info('Getting tests from JAR via proguard.')
     tests = _GetTestsFromProguard(test_jar)
     _SaveTestsToPickle(pickle_path, test_jar, tests)
@@ skipping 60 matching lines @@
 
 
 class UnmatchedFilterException(test_exception.TestException):
   """Raised when a user specifies a filter that doesn't match any tests."""
 
   def __init__(self, test_filter):
     super(UnmatchedFilterException, self).__init__(
         'Test filter "%s" matched no tests.' % test_filter)
 
 
+def GetTestName(test, sep='#'):
+  """Gets the name of the given test.
+
+  Note that this may return the same name for more than one test, e.g. if a
+  test is being run multiple times with different parameters.
+
+  Args:
+    test: the instrumentation test dict.
+    sep: the character(s) that should join the class name and the method name.
+  Returns:
+    The test name as a string.
+  """
+  return '%s%s%s' % (test['class'], sep, test['method'])
+
+
+def GetUniqueTestName(test, sep='#'):
+  """Gets the unique name of the given test.
+
+  This will include text to disambiguate between tests for which GetTestName
+  would return the same name.
+
+  Args:
+    test: the instrumentation test dict.
+    sep: the character(s) that should join the class name and the method name.
+  Returns:
+    The unique test name as a string.
+  """
+  display_name = GetTestName(test, sep=sep)
+  if 'flags' in test:
+    flags = test['flags']
+    if flags.add:
+      display_name = '%s with {%s}' % (display_name, ' '.join(flags.add))
+    if flags.remove:
+      display_name = '%s without {%s}' % (display_name, ' '.join(flags.remove))
+  return display_name
+
+
 class InstrumentationTestInstance(test_instance.TestInstance):
 
   def __init__(self, args, isolate_delegate, error_func):
     super(InstrumentationTestInstance, self).__init__()
 
     self._additional_apks = []
     self._apk_under_test = None
     self._apk_under_test_incremental_install_script = None
     self._package_info = None
     self._suite = None
@@ skipping 122 matching lines @@
       self._isolated_abs_path = os.path.join(
           constants.GetOutDirectory(), '%s.isolated' % self._test_package)
     else:
       self._isolate_delegate = None
 
     if not self._isolate_delegate:
       logging.warning('No data dependencies will be pushed.')
 
   def _initializeTestFilterAttributes(self, args):
     if args.test_filter:
-      self._test_filter = args.test_filter.replace('#', '.')
+      self._test_filter = _CMDLINE_NAME_SEGMENT_RE.sub(
+          '', args.test_filter.replace('#', '.'))
 
     def annotation_element(a):
       a = a.split('=', 1)
       return (a[0], a[1] if len(a) == 2 else None)
 
     if args.annotation_str:
       self._annotations = [
           annotation_element(a) for a in args.annotation_str.split(',')]
     elif not self._test_filter:
       self._annotations = [
@@ skipping 137 matching lines @@
       self._isolate_delegate.Remap(
           self._isolate_abs_path, self._isolated_abs_path)
       self._isolate_delegate.MoveOutputDeps()
       self._data_deps.extend([(self._isolate_delegate.isolate_deps_dir, None)])
 
   def GetDataDependencies(self):
     return self._data_deps
 
   def GetTests(self):
     tests = GetAllTests(self.test_jar)
+    inflated_tests = self._ParametrizeTestsWithFlags(self._InflateTests(tests))
     filtered_tests = FilterTests(
-        tests, self._test_filter, self._annotations, self._excluded_annotations)
+        inflated_tests, self._test_filter, self._annotations,
+        self._excluded_annotations)
     if self._test_filter and not filtered_tests:
+      for t in inflated_tests:
+        logging.debug('  %s', GetUniqueTestName(t))
       raise UnmatchedFilterException(self._test_filter)
-    return self._ParametrizeTestsWithFlags(self._InflateTests(filtered_tests))
+    return filtered_tests
 
   # pylint: disable=no-self-use
   def _InflateTests(self, tests):
     inflated_tests = []
     for c in tests:
       for m in c['methods']:
         a = dict(c['annotations'])
         a.update(m['annotations'])
         inflated_tests.append({
             'class': c['class'],
@@ skipping 38 matching lines @@
   @staticmethod
   def GenerateTestResults(
       result_code, result_bundle, statuses, start_ms, duration_ms):
     return GenerateTestResults(result_code, result_bundle, statuses,
                                start_ms, duration_ms)
 
   #override
   def TearDown(self):
     if self._isolate_delegate:
       self._isolate_delegate.Clear()