Make multiple --isolated-script-test-filter args work for run_web_tests.

Previously run_web_tests implemented an overly simplistic variant
of the test runner api documented in bit.ly/chromium-test-runner-api,
in that it only supported a single occurrence of the flag,
it didn't support negative (skip) filters, and it implemented it
as if you had just listed the globs as positional arguments instead.

This CL implements the full semantics of the flag, so that you can
specify the arg multiple times, and negative (skip) filters work
correctly.

This is a non-backwards-compatible change, because it changes things
such that we first filter by positional arguments and *then* apply
the filters from --isolated-script-test-filter.

However, it seems unlikely that anyone is using the combination of
the two and so hopefully this will not break things. If it does,
we'll need to update callers.

BUG=994943

Change-Id: I7cc10c92364e145e84f3750f0c773b8ab9d371f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1790432Reviewed-by: Rakib Hasan <rmhasan@google.com>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697404}

third_party/blink/tools/blinkpy/web_tests/controllers/manager.py

@@ -260,7 +260,8 @@ class Manager(object):
 
     def _collect_tests(self, args):
         return self._finder.find_tests(args, test_list=self._options.test_list,
-                                       fastest_percentile=self._options.fastest)
+                                       fastest_percentile=self._options.fastest,
+                                       filters=self._options.isolated_script_test_filter)
 
     def _is_http_test(self, test):
         return (

third_party/blink/tools/blinkpy/web_tests/controllers/web_test_finder.py

@@ -27,6 +27,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import errno
+import fnmatch
 import json
 import logging
 import math
@@ -47,7 +48,8 @@ class WebTestFinder(object):
         self._filesystem = self._port.host.filesystem
         self.WEB_TESTS_DIRECTORIES = ('src', 'third_party', 'blink', 'web_tests')
 
-    def find_tests(self, args, test_list=None, fastest_percentile=None):
+    def find_tests(self, args, test_list=None, fastest_percentile=None, filters=None):
+        filters = filters or []
         paths = self._strip_test_dir_prefixes(args)
         if test_list:
             paths += self._strip_test_dir_prefixes(self._read_test_names_from_file(test_list, self._port.TEST_PATH_SEPARATOR))
@@ -77,6 +79,7 @@ class WebTestFinder(object):
             test_files = all_tests
             running_all_tests = True
 
+        test_files = filter_tests(test_files, [f.split('::') for f in filters])
         return (paths, test_files, running_all_tests)
 
     def _times_trie(self):
@@ -213,3 +216,57 @@ class WebTestFinder(object):
                    index, count, len(tests_to_run), len(test_names))
 
         return tests_to_run, other_tests
+
+
+def filter_tests(tests, filters):
+    """Returns a filtered list of tests to run.
+
+    The test-filtering semantics are documented in
+    https://bit.ly/chromium-test-runner-api and
+    https://bit.ly/chromium-test-list-format, but are as follows:
+
+    Each filter is a list of glob expressions, with each expression optionally
+    prefixed by a "-". If the glob starts with a "-", it is a negative glob,
+    otherwise it is a positive glob.
+
+    A test passes the filter if and only if it is explicitly matched by at
+    least one positive glob and no negative globs, or if there are no
+    positive globs and it is not matched by any negative globs.
+
+    Globbing is fairly limited; "?" is not allowed, and "*" must only appear
+    at the end of the glob. If multiple globs match a test, the longest match
+    wins. If both globs are the same length, an error is raised.
+
+    A test will be run only if it passes every filter.
+    """
+
+    def glob_sort_key(k):
+        if k and k[0] == '-':
+            return (len(k[1:]), k[1:])
+        else:
+            return (len(k), k)
+
+    for globs in filters:
+        include_by_default = all(glob.startswith('-') for glob in globs)
+        filtered_tests = []
+        for test in tests:
+            include = include_by_default
+            for glob in sorted(globs, key=glob_sort_key):
+                if not glob[:-1]:
+                    raise ValueError('Empty glob filter "%s"' % (filter,))
+                if '*' in glob[:-1]:
+                    raise ValueError('Bad test filter "%s" specified; '
+                                     'wildcards are only allowed at the end'
+                                     % (glob,))
+                if glob.startswith('-') and glob[1:] in globs:
+                    raise ValueError('Both "%s" and "%s" specified in test '
+                                     'filter' % (glob, glob[1:]))
+                if glob.startswith('-'):
+                    include = include and not fnmatch.fnmatch(test, glob[1:])
+                else:
+                    include = include or fnmatch.fnmatch(test, glob)
+            if include:
+                filtered_tests.append(test)
+        tests = filtered_tests
+
+    return tests

third_party/blink/tools/blinkpy/web_tests/controllers/web_test_finder_unittest.py

@@ -128,3 +128,79 @@ class WebTestFinderTests(unittest.TestCase):
           self.assertEqual(([3, 6], [1, 2, 4, 5]), split(tests, 0, 3))
           self.assertEqual(([1, 4], [2, 3, 5, 6]), split(tests, 1, 3))
           self.assertEqual(([2, 5], [1, 3, 4, 6]), split(tests, 2, 3))
+
+
+class FilterTestsTests(unittest.TestCase):
+    simple_test_list = ['a/a1.html', 'a/a2.html', 'b/b1.html']
+
+    def check(self, tests, filters, expected_tests):
+        self.assertEqual(expected_tests,
+                         web_test_finder.filter_tests(tests, filters))
+
+    def test_no_filters(self):
+        self.check(self.simple_test_list, [],
+                   self.simple_test_list)
+
+    def test_empty_glob_is_rejected(self):
+        self.assertRaises(ValueError, self.check,
+                          self.simple_test_list, [['']], [])
+        self.assertRaises(ValueError, self.check,
+                          self.simple_test_list, [['-']], [])
+
+    def test_one_all_positive_filter(self):
+        self.check(self.simple_test_list, [['a*']],
+                   ['a/a1.html', 'a/a2.html'])
+
+        self.check(self.simple_test_list, [['a*', 'b*']],
+                   self.simple_test_list)
+
+    def test_one_all_negative_filter(self):
+        self.check(self.simple_test_list, [['-c*']],
+                   self.simple_test_list)
+
+    def test_one_mixed_filter(self):
+        self.check(self.simple_test_list, [['a*', '-c*']],
+                   ['a/a1.html', 'a/a2.html'])
+
+    def test_two_all_positive_filters(self):
+        self.check(self.simple_test_list, [['a*'], ['b*']],
+                   [])
+
+    def test_two_all_negative_filters(self):
+        self.check(self.simple_test_list, [['-a*'], ['-b*']],
+                   [])
+
+        self.check(self.simple_test_list, [['-a*'], ['-c*']],
+                   ['b/b1.html'])
+
+    def test_two_mixed_filters(self):
+        self.check(self.simple_test_list, [['a*'], ['-b*']],
+                   ['a/a1.html', 'a/a2.html'])
+
+    def test_longest_glob_wins(self):
+        # These test that if two matching globs are specified as
+        # part of the same filter expression, the longest matching
+        # glob wins (takes precedence). The order of the two globs
+        # must not matter.
+        self.check(self.simple_test_list, [['a/a*', '-a/a2*']],
+                   ['a/a1.html'])
+        self.check(self.simple_test_list, [['-a/a*', 'a/a2*']],
+                   ['a/a2.html'])
+
+        # In this test, the positive and negative globs are in
+        # separate filter expressions, so a2 should be filtered out
+        # and nothing should run (tests should only be run if they
+        # would be run by every filter individually).
+        self.check(self.simple_test_list, [['-a/a*'], ['a/a2*']],
+                   [])
+
+    def test_only_trailing_globs_work(self):
+        self.check(self.simple_test_list, [['a*']],
+                                           ['a/a1.html', 'a/a2.html'])
+
+        # These test that if you have a glob that contains a "*" that isn't
+        # at the end, it is rejected; only globs at the end should work.
+        self.assertRaises(ValueError, self.check,
+                          self.simple_test_list, [['*1.html']], [])
+        self.assertRaises(ValueError, self.check,
+                          self.simple_test_list, [['a*.html']], [])

third_party/blink/tools/blinkpy/web_tests/run_web_tests.py

@@ -438,9 +438,10 @@ def parse_args(args):
                 help='read list of tests to run from file'),
             optparse.make_option(
                 '--isolated-script-test-filter',
+                action='append',
                 type='string',
-                help='A list of tests to run separated by TWO colons, e.g. fast::css/test.html, '
-                     'same as listing them as positional arguments'),
+                help='A list of test globs to run or skip, separated by TWO colons, e.g. fast::css/test.html; '
+                     'prefix the glob with "-" to skip it'),
             # TODO(crbug.com/893235): Remove gtest_filter when FindIt no longer uses it.
             optparse.make_option(
                 '--gtest_filter',
@@ -594,9 +595,6 @@ def _set_up_derived_options(port, options, args):
     if not options.skipped:
         options.skipped = 'default'
 
-    if options.isolated_script_test_filter:
-        args.extend(options.isolated_script_test_filter.split('::'))
-
     if options.gtest_filter:
         args.extend(options.gtest_filter.split(':'))
 

third_party/blink/tools/blinkpy/web_tests/run_web_tests_unittest.py

@@ -519,7 +519,26 @@ class RunTest(unittest.TestCase, StreamTestingMixin):
             ['--isolated-script-test-filter=passes/text.html::passes/image.html', 'passes/error.html'],
             host=host
         )
-        self.assertEqual(sorted(tests_run), ['passes/error.html', 'passes/image.html', 'passes/text.html'])
+        self.assertEqual(sorted(tests_run), [])
+
+        tests_run = get_tests_run(
+            ['--isolated-script-test-filter=passes/error.html::passes/image.html', 'passes/error.html'],
+            host=host
+        )
+        self.assertEqual(sorted(tests_run), ['passes/error.html'])
+
+        tests_run = get_tests_run(
+            ['--isolated-script-test-filter=passes/error.html::passes/image.html'],
+            host=host
+        )
+        self.assertEqual(sorted(tests_run), ['passes/error.html', 'passes/image.html'])
+
+        tests_run = get_tests_run(
+            ['--isolated-script-test-filter=passes/error.html::passes/image.html',
+             '--isolated-script-test-filter=passes/error.html'],
+            host=host
+        )
+        self.assertEqual(sorted(tests_run), ['passes/error.html'])
 
     def test_gtest_filter(self):
         host = MockHost()