Skip idlharness tests on MSAN/ASAN

The idlharness tests routinely timeout on MSAN/ASAN due to
how slow those bots are. Given that IDL tests are just examining
the existence of web APIs, they are unlikely to turn up crashes
in the browser (they don't even call the APIs under test), so
we have decided to unilaterally disable them on MSAN/ASAN to reduce
the pain for Chromium sheriffs.

Bug: 856601
Change-Id: I96c36a4e3dcda9437ac776620bfc5c9cc48f52a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151087
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762125}

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

@@ -311,8 +311,7 @@ class Manager(object):
 
     def _prepare_lists(self, paths, test_names):
         tests_to_skip = self._finder.skip_tests(paths, test_names,
-                                                self._expectations,
-                                                self._http_tests(test_names))
+                                                self._expectations)
         tests_to_run = [
             test for test in test_names if test not in tests_to_skip
         ]

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

@@ -36,6 +36,7 @@ import re
 from blinkpy.web_tests.layout_package.json_results_generator import convert_times_trie_to_flat_paths
 from blinkpy.web_tests.models import test_expectations
 from blinkpy.web_tests.models.typ_types import ResultType
+from blinkpy.web_tests.port.base import Port
 
 _log = logging.getLogger(__name__)
 
@@ -174,14 +175,37 @@ class WebTestFinder(object):
         else:
             return line
 
-    def skip_tests(self, paths, all_tests_list, expectations, http_tests):
-        if self._options.no_expectations:
-            # do not skip anything.
-            return []
+    def skip_tests(self, paths, all_tests_list, expectations):
+        """Given a list of tests, returns the ones that should be skipped.
 
+        A test may be skipped for many reasons, depending on the expectation
+        files and options selected. The most obvious is SKIP entries in
+        TestExpectations, but we also e.g. skip idlharness tests on MSAN/ASAN
+        due to https://crbug.com/856601.
+
+        Args:
+            paths: the paths passed on the command-line to run_web_tests.py
+            all_tests_list: all tests that we are considering running
+            expectations: parsed TestExpectations data
+
+        Returns: a set of tests that should be skipped (not run).
+        """
         all_tests = set(all_tests_list)
         tests_to_skip = set()
         for test in all_tests:
+            # We always skip idlharness tests for MSAN/ASAN, even when running
+            # with --no-expectations (https://crbug.com/856601). Note we will
+            # run the test anyway if it is explicitly specified on the command
+            # line; paths are removed from the skip list after this loop.
+            if self._options.enable_sanitizer and Port.is_wpt_idlharness_test(
+                    test):
+                tests_to_skip.update({test})
+                continue
+
+            if self._options.no_expectations:
+                # do not skip anything from TestExpectations
+                continue
+
             expected_results = expectations.get_expectations(test).results
             if ResultType.Skip in expected_results:
                 tests_to_skip.update({test})
@@ -189,6 +213,7 @@ class WebTestFinder(object):
                 tests_to_skip.update({test})
             if self._options.skip_failing_tests and ResultType.Failure in expected_results:
                 tests_to_skip.update({test})
+
         if self._options.skipped == 'only':
             tests_to_skip = all_tests - tests_to_skip
         elif self._options.skipped == 'ignore':

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

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import optparse
 import os
 import sys
 import unittest
@@ -9,6 +10,7 @@ import unittest
 from blinkpy.common import path_finder
 from blinkpy.common.host_mock import MockHost
 from blinkpy.web_tests.controllers import web_test_finder
+from blinkpy.web_tests.models import test_expectations
 
 _MOCK_ROOT = os.path.join(path_finder.get_chromium_src_dir(), 'third_party',
                           'pymock')
@@ -17,6 +19,136 @@ import mock
 
 
 class WebTestFinderTests(unittest.TestCase):
+
+    def test_skip_tests_expectations(self):
+        """Tests that tests are skipped based on to expectations and options."""
+        host = MockHost()
+        port = host.port_factory.get('test-win-win7', None)
+
+        all_tests = [
+            'fast/css/passes.html',
+            'fast/css/fails.html',
+            'fast/css/times_out.html',
+            'fast/css/skip.html',
+        ]
+
+        # Patch port.tests() to return our tests
+        port.tests = lambda paths: paths or all_tests
+
+        options = optparse.Values({
+            'no_expectations': False,
+            'enable_sanitizer': False,
+            'skipped': 'default',
+            'skip_timeouts': False,
+            'skip_failing_tests': False,
+        })
+        finder = web_test_finder.WebTestFinder(port, options)
+
+        expectations = test_expectations.TestExpectations(port)
+        expectations.merge_raw_expectations(
+            ('# results: [ Failure Timeout Skip ]'
+             '\nfast/css/fails.html [ Failure ]'
+             '\nfast/css/times_out.html [ Timeout ]'
+             '\nfast/css/skip.html [ Skip ]'))
+
+        # When run with default settings, we only skip the tests marked Skip.
+        tests = finder.skip_tests([], all_tests, expectations)
+        self.assertEqual(tests, set(['fast/css/skip.html']))
+
+        # Specify test on the command line; by default should not skip.
+        tests = finder.skip_tests(['fast/css/skip.html'], all_tests,
+                                  expectations)
+        self.assertEqual(tests, set())
+
+        # Specify test on the command line, but always skip.
+        finder._options.skipped = 'always'
+        tests = finder.skip_tests(['fast/css/skip.html'], all_tests,
+                                  expectations)
+        self.assertEqual(tests, set(['fast/css/skip.html']))
+        finder._options.skipped = 'default'
+
+        # Only run skip tests, aka skip all non-skipped tests.
+        finder._options.skipped = 'only'
+        tests = finder.skip_tests([], all_tests, expectations)
+        self.assertEqual(
+            tests,
+            set([
+                'fast/css/passes.html', 'fast/css/fails.html',
+                'fast/css/times_out.html'
+            ]))
+        finder._options.skipped = 'default'
+
+        # Ignore any skip entries, aka never skip anything.
+        finder._options.skipped = 'ignore'
+        tests = finder.skip_tests([], all_tests, expectations)
+        self.assertEqual(tests, set())
+        finder._options.skipped = 'default'
+
+        # Skip tests that are marked TIMEOUT.
+        finder._options.skip_timeouts = True
+        tests = finder.skip_tests([], all_tests, expectations)
+        self.assertEqual(
+            tests, set(['fast/css/times_out.html', 'fast/css/skip.html']))
+        finder._options.skip_timeouts = False
+
+        # Skip tests that are marked FAILURE
+        finder._options.skip_failing_tests = True
+        tests = finder.skip_tests([], all_tests, expectations)
+        self.assertEqual(tests,
+                         set(['fast/css/fails.html', 'fast/css/skip.html']))
+        finder._options.skip_failing_tests = False
+
+        # Disable expectations entirely; nothing should be skipped by default.
+        finder._options.no_expectations = True
+        tests = finder.skip_tests([], all_tests, expectations)
+        self.assertEqual(tests, set())
+
+    def test_skip_tests_idlharness(self):
+        """Tests that idlharness tests are skipped on MSAN/ASAN runs.
+
+        See https://crbug.com/856601
+        """
+        host = MockHost()
+        port = host.port_factory.get('test-win-win7', None)
+
+        all_tests = [
+            'external/wpt/dir1/dir2/foo.html',
+            'external/wpt/dir1/dir2/idlharness.any.html',
+            'external/wpt/dir1/dir2/idlharness.any.worker.html',
+        ]
+
+        # Patch port.tests() to return our tests
+        port.tests = lambda paths: paths or all_tests
+
+        options = optparse.Values({
+            'no_expectations': True,
+            'enable_sanitizer': False,
+            'skipped': 'default',
+        })
+        finder = web_test_finder.WebTestFinder(port, options)
+
+        # Default case; not MSAN/ASAN so should not skip anything.
+        tests = finder.skip_tests([], all_tests, None)
+        self.assertEqual(tests, set())
+
+        # MSAN/ASAN, with no paths specified explicitly, so should skip both
+        # idlharness tests.
+        finder._options.enable_sanitizer = True
+        tests = finder.skip_tests([], all_tests, None)
+        self.assertEqual(
+            tests,
+            set([
+                'external/wpt/dir1/dir2/idlharness.any.html',
+                'external/wpt/dir1/dir2/idlharness.any.worker.html'
+            ]))
+
+        # MSAN/ASAN, with one of the tests specified explicitly (and
+        # --skipped=default), so should skip only the unspecified test.
+        tests = finder.skip_tests(
+            ['external/wpt/dir1/dir2/idlharness.any.html'], all_tests, None)
+        self.assertEqual(
+            tests, set(['external/wpt/dir1/dir2/idlharness.any.worker.html']))
+
     def test_find_fastest_tests(self):
         host = MockHost()
         port = host.port_factory.get('test-win-win7', None)

third_party/blink/tools/blinkpy/web_tests/port/base.py

@@ -1377,6 +1377,21 @@ class Port(object):
         """Whether a test is considered a web-platform-tests test."""
         return Port.WPT_REGEX.match(test)
 
+    @staticmethod
+    def is_wpt_idlharness_test(test_file):
+        """Returns whether a WPT test is (probably) an idlharness test.
+
+        There are no rules in WPT that can be used to identify idlharness tests
+        without examining the file contents (which would be expensive). This
+        method utilizes a filename heuristic, based on the convention of
+        including 'idlharness' in the appropriate test names.
+        """
+        match = Port.WPT_REGEX.match(test_file)
+        if not match:
+            return False
+        filename = match.group(2).split('/')[-1]
+        return 'idlharness' in filename
+
     @staticmethod
     def should_use_wptserve(test):
         return Port.is_wpt_test(test)

third_party/blink/tools/blinkpy/web_tests/port/base_unittest.py

@@ -1053,6 +1053,24 @@ class PortTest(LoggingTestCase):
         self.assertFalse(
             Port.is_wpt_test('not-virtual/a-name/external/wpt/baz/qux.htm'))
 
+    def test_is_wpt_idlharness_test(self):
+        self.assertTrue(
+            Port.is_wpt_idlharness_test(
+                'external/wpt/css/css-pseudo/idlharness.html'))
+        self.assertTrue(
+            Port.is_wpt_idlharness_test(
+                'external/wpt/payment-handler/idlharness.https.any.html'))
+        self.assertTrue(
+            Port.is_wpt_idlharness_test(
+                'external/wpt/payment-handler/idlharness.https.any.serviceworker.html'
+            ))
+        self.assertFalse(
+            Port.is_wpt_idlharness_test(
+                'external/wpt/css/foo/interfaces.html'))
+        self.assertFalse(
+            Port.is_wpt_idlharness_test(
+                'external/wpt/css/idlharness/bar.html'))
+
     def test_should_use_wptserve(self):
         self.assertTrue(
             Port.should_use_wptserve('external/wpt/dom/interfaces.html'))