[blinkpy] Find manifest items for tests by URL

When running WPT in web_tests, test names are more like URLs instead of
file paths (to be precise, the part of a test name after external/wpt is
a WPT URL), because WPT can generate multiple tests (variations) for a
single test file (e.g. .any.js, ?run_type).

Therefore, when asking questions like "is a test slow?", we need to find
the manifest item for that test by URL instead of by file path; whereas
if we want to know if a file in external/wpt is a test file, we find
the manifest item(s) for that file by file path.

This CL separates the two types of manifest queries, and chooses the
correct one for each call site. Besides, unit tests are improved to
cover the WPT variations (.any.js & ?run_type).

Bug: 831975, 800570
Change-Id: I230d5ec7df06b7df1387da1b93788d6cae55d153
Reviewed-on: https://chromium-review.googlesource.com/1043160
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556043}

third_party/blink/tools/blinkpy/w3c/wpt_manifest.py

@@ -18,23 +18,50 @@ from blinkpy.common.path_finder import PathFinder
 _log = logging.getLogger(__file__)
 
 
+# TODO(robertma): Use the official wpt.manifest module.
+
 class WPTManifest(object):
+    """A simple abstraction of WPT MANIFEST.json.
+
+    The high-level structure of the manifest is as follows:
+        {
+            "items": {
+                "manual": {
+                    "file/path": [manifest items],
+                    ...
+                },
+                "reftest": {...},
+                "testharness": {...}
+            },
+            // other info...
+        }
+
+    The format of a manifest item depends on:
+        https://github.com/w3c/web-platform-tests/blob/master/tools/manifest/item.py
+    which can be roughly summarized as follows:
+        * testharness test: [url, extras]
+        * reftest: [url, references, extras]
+    where `extras` is a dict with the following optional items:
+        * testharness test: {"timeout": "long", "testdriver": True}
+        * reftest: {"timeout": "long", "viewport_size": ..., "dpi": ...}
+    and `references` is a list that looks like:
+        [[reference_url1, "=="], [reference_url2, "!="], ...]
+    """
 
     def __init__(self, json_content):
-        # TODO(tkent): Create a Manifest object by Manifest.from_json().
-        # See ../third_party/wpt/wpt/tools/manifest/manifest.py.
         self.raw_dict = json.loads(json_content)
         self.test_types = ('manual', 'reftest', 'testharness')
 
-    def _items_for_path(self, path_in_wpt):
-        """Returns manifest items for the given WPT path, or None if not found.
+    def _items_for_file_path(self, path_in_wpt):
+        """Finds manifest items for the given WPT path.
 
-        The format of a manifest item depends on:
-          https://github.com/w3c/web-platform-tests/blob/master/tools/manifest/item.py
+        Args:
+            path_in_wpt: A file path relative to the root of WPT. Note that this
+                is different from a WPT URL; a file path does not have a leading
+                slash or a query string.
 
-        For most testharness tests, the returned items is expected
-        to look like this: [["/some/test/path.html", {}]]. For reference tests,
-        it will be a list with three items ([url, references, extras]).
+        Returns:
+            A list of manifest items, or None if not found.
         """
         items = self.raw_dict['items']
         for test_type in self.test_types:
@@ -42,20 +69,44 @@ class WPTManifest(object):
                 return items[test_type][path_in_wpt]
         return None
 
+    def _item_for_url(self, url):
+        """Finds the manifest item for the given WPT URL.
+
+        Args:
+            url: A WPT URL (with the leading slash).
+
+        Returns:
+            A manifest item, or None if not found.
+        """
+        return self.all_url_items().get(url)
+
+    @staticmethod
+    def _get_url_from_item(item):
+        return item[0]
+
+    @staticmethod
+    def _get_extras_from_item(item):
+        return item[-1]
+
+    @memoized
+    def all_url_items(self):
+        """Returns a dict mapping every URL in the manifest to its item."""
+        url_items = {}
+        if 'items' not in self.raw_dict:
+            return url_items
+        for test_type in self.test_types:
+            for records in self.raw_dict['items'][test_type].itervalues():
+                for item in records:
+                    url_items[self._get_url_from_item(item)] = item
+        return url_items
+
     @memoized
     def all_urls(self):
-        """Returns a set of the urls for all items in the manifest."""
-        urls = set()
-        if 'items' in self.raw_dict:
-            items = self.raw_dict['items']
-            for category in self.test_types:
-                if category in items:
-                    for records in items[category].values():
-                        urls.update([item[0] for item in records])
-        return urls
+        """Returns a set of the URLs for all items in the manifest."""
+        return frozenset(self.all_url_items().keys())
 
     def is_test_file(self, path_in_wpt):
-        return self._items_for_path(path_in_wpt) is not None
+        return self._items_for_file_path(path_in_wpt) is not None
 
     def is_test_url(self, url):
         """Checks if url is a valid test in the manifest.
@@ -67,20 +118,28 @@ class WPTManifest(object):
         return url in self.all_urls()
 
     def file_path_to_url_paths(self, path_in_wpt):
-        manifest_items = self._items_for_path(path_in_wpt)
+        manifest_items = self._items_for_file_path(path_in_wpt)
         assert manifest_items is not None
-        return [item[0][1:] for item in manifest_items]
+        # Remove the leading slashes when returning.
+        return [self._get_url_from_item(item)[1:] for item in manifest_items]
 
-    @staticmethod
-    def _get_extras_from_item(item):
-        return item[-1]
+    def is_slow_test(self, url):
+        """Checks if a WPT is slow (long timeout) according to the manifest.
+
+        Args:
+            url: A WPT URL (with the leading slash).
+
+        Returns:
+            True if the test is found and is slow, False otherwise.
+        """
+        if not self.is_test_url(url):
+            return False
 
-    def is_slow_test(self, test_name):
-        items = self._items_for_path(test_name)
-        if not items:
+        item = self._item_for_url(url)
+        if not item:
             return False
-        extras = WPTManifest._get_extras_from_item(items[0])
-        return 'timeout' in extras and extras['timeout'] == 'long'
+        extras = self._get_extras_from_item(item)
+        return extras.get('timeout') == 'long'
 
     def extract_reference_list(self, path_in_wpt):
         """Extracts reference information of the specified reference test.

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

@@ -876,7 +876,8 @@ class Port(object):
         match = re.match(r'virtual/[^/]+/', test_file)
         if match:
             test_file = test_file[match.end(0):]
-        match = re.match(r'external/wpt/(.*)', test_file)
+        # WPTManifest.is_slow_test() takes a WPT URL with the leading slash.
+        match = re.match(r'external/wpt(.*)', test_file)
         if not match:
             return False
         return self._wpt_manifest().is_slow_test(match.group(1))

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

@@ -428,11 +428,15 @@ class PortTest(unittest.TestCase):
                         ['/dom/ranges/Range-attributes.html', {}]
                     ],
                     'dom/ranges/Range-attributes-slow.html': [
-                        ['/dom/ranges/Range-attributes.html', {'timeout': 'long'}]
+                        ['/dom/ranges/Range-attributes-slow.html', {'timeout': 'long'}]
                     ],
                     'console/console-is-a-namespace.any.js': [
                         ['/console/console-is-a-namespace.any.html', {}],
-                        ['/console/console-is-a-namespace.any.worker.html', {}],
+                        ['/console/console-is-a-namespace.any.worker.html', {'timeout': 'long'}],
+                    ],
+                    'html/parse.html': [
+                        ['/html/parse.html?run_type=uri', {}],
+                        ['/html/parse.html?run_type=write', {'timeout': 'long'}],
                     ],
                 },
                 'manual': {},
@@ -459,37 +463,34 @@ class PortTest(unittest.TestCase):
         port = self.make_port(with_tests=True)
         PortTest._add_manifest_to_mock_file_system(port.host.filesystem)
         self.assertNotIn('external/wpt/common/blank.html', port.tests([]))
+        self.assertNotIn('external/wpt/console/console-is-a-namespace.any.js', port.tests([]))
 
     def test_find_one_if_in_manifest(self):
         port = self.make_port(with_tests=True)
         PortTest._add_manifest_to_mock_file_system(port.host.filesystem)
         self.assertIn('external/wpt/dom/ranges/Range-attributes.html', port.tests([]))
-        self.assertNotIn('external/wpt/console/console-is-a-namespace.any.js', port.tests([]))
+        self.assertIn('external/wpt/console/console-is-a-namespace.any.html', port.tests([]))
 
+    def test_wpt_tests_paths(self):
+        port = self.make_port(with_tests=True)
+        PortTest._add_manifest_to_mock_file_system(port.host.filesystem)
+        all_wpt = [
+            'external/wpt/console/console-is-a-namespace.any.html',
+            'external/wpt/console/console-is-a-namespace.any.worker.html',
+            'external/wpt/dom/ranges/Range-attributes-slow.html',
+            'external/wpt/dom/ranges/Range-attributes.html',
+            'external/wpt/html/dom/elements/global-attributes/dir_auto-EN-L.html',
+            'external/wpt/html/parse.html?run_type=uri',
+            'external/wpt/html/parse.html?run_type=write',
+        ]
         # test.any.js shows up on the filesystem as one file but it effectively becomes two test files:
         # test.any.html and test.any.worker.html. We should support running test.any.js by name and
         # indirectly by specifying a parent directory.
-        self.assertEqual(port.tests(['external']),
-                         ['external/wpt/html/dom/elements/global-attributes/dir_auto-EN-L.html',
-                          'external/wpt/dom/ranges/Range-attributes.html',
-                          'external/wpt/console/console-is-a-namespace.any.worker.html',
-                          'external/wpt/console/console-is-a-namespace.any.html'])
-        self.assertEqual(port.tests(['external/']),
-                         ['external/wpt/html/dom/elements/global-attributes/dir_auto-EN-L.html',
-                          'external/wpt/dom/ranges/Range-attributes.html',
-                          'external/wpt/console/console-is-a-namespace.any.worker.html',
-                          'external/wpt/console/console-is-a-namespace.any.html'])
+        self.assertEqual(sorted(port.tests(['external'])), all_wpt)
+        self.assertEqual(sorted(port.tests(['external/'])), all_wpt)
         self.assertEqual(port.tests(['external/csswg-test']), [])
-        self.assertEqual(port.tests(['external/wpt']),
-                         ['external/wpt/html/dom/elements/global-attributes/dir_auto-EN-L.html',
-                          'external/wpt/dom/ranges/Range-attributes.html',
-                          'external/wpt/console/console-is-a-namespace.any.worker.html',
-                          'external/wpt/console/console-is-a-namespace.any.html'])
-        self.assertEqual(port.tests(['external/wpt/']),
-                         ['external/wpt/html/dom/elements/global-attributes/dir_auto-EN-L.html',
-                          'external/wpt/dom/ranges/Range-attributes.html',
-                          'external/wpt/console/console-is-a-namespace.any.worker.html',
-                          'external/wpt/console/console-is-a-namespace.any.html'])
+        self.assertEqual(sorted(port.tests(['external/wpt'])), all_wpt)
+        self.assertEqual(sorted(port.tests(['external/wpt/'])), all_wpt)
         self.assertEqual(port.tests(['external/wpt/console']),
                          ['external/wpt/console/console-is-a-namespace.any.worker.html',
                           'external/wpt/console/console-is-a-namespace.any.html'])
@@ -502,9 +503,11 @@ class PortTest(unittest.TestCase):
         self.assertEqual(port.tests(['external/wpt/console/console-is-a-namespace.any.html']),
                          ['external/wpt/console/console-is-a-namespace.any.html'])
         self.assertEqual(port.tests(['external/wpt/dom']),
-                         ['external/wpt/dom/ranges/Range-attributes.html'])
+                         ['external/wpt/dom/ranges/Range-attributes-slow.html',
+                          'external/wpt/dom/ranges/Range-attributes.html'])
         self.assertEqual(port.tests(['external/wpt/dom/']),
-                         ['external/wpt/dom/ranges/Range-attributes.html'])
+                         ['external/wpt/dom/ranges/Range-attributes-slow.html',
+                          'external/wpt/dom/ranges/Range-attributes.html'])
         self.assertEqual(port.tests(['external/wpt/dom/ranges/Range-attributes.html']),
                          ['external/wpt/dom/ranges/Range-attributes.html'])
 
@@ -513,23 +516,28 @@ class PortTest(unittest.TestCase):
         PortTest._add_manifest_to_mock_file_system(port.host.filesystem)
         all_wpt = [
             'virtual/virtual_wpt/external/wpt/console/console-is-a-namespace.any.html',
-            'virtual/virtual_wpt/external/wpt/html/dom/elements/global-attributes/dir_auto-EN-L.html',
             'virtual/virtual_wpt/external/wpt/console/console-is-a-namespace.any.worker.html',
+            'virtual/virtual_wpt/external/wpt/dom/ranges/Range-attributes-slow.html',
             'virtual/virtual_wpt/external/wpt/dom/ranges/Range-attributes.html',
+            'virtual/virtual_wpt/external/wpt/html/dom/elements/global-attributes/dir_auto-EN-L.html',
+            'virtual/virtual_wpt/external/wpt/html/parse.html?run_type=uri',
+            'virtual/virtual_wpt/external/wpt/html/parse.html?run_type=write',
         ]
         dom_wpt = [
+            'virtual/virtual_wpt_dom/external/wpt/dom/ranges/Range-attributes-slow.html',
             'virtual/virtual_wpt_dom/external/wpt/dom/ranges/Range-attributes.html',
         ]
 
-        self.assertEqual(port.tests(['virtual/virtual_wpt/external/']), all_wpt)
-        self.assertEqual(port.tests(['virtual/virtual_wpt/external/wpt/']), all_wpt)
+        self.assertEqual(sorted(port.tests(['virtual/virtual_wpt/external/'])), all_wpt)
+        self.assertEqual(sorted(port.tests(['virtual/virtual_wpt/external/wpt/'])), all_wpt)
         self.assertEqual(port.tests(['virtual/virtual_wpt/external/wpt/console']),
-                         ['virtual/virtual_wpt/external/wpt/console/console-is-a-namespace.any.html',
-                          'virtual/virtual_wpt/external/wpt/console/console-is-a-namespace.any.worker.html'])
+                         ['virtual/virtual_wpt/external/wpt/console/console-is-a-namespace.any.worker.html',
+                          'virtual/virtual_wpt/external/wpt/console/console-is-a-namespace.any.html'])
 
         self.assertEqual(port.tests(['virtual/virtual_wpt_dom/external/wpt/dom/']), dom_wpt)
         self.assertEqual(port.tests(['virtual/virtual_wpt_dom/external/wpt/dom/ranges/']), dom_wpt)
-        self.assertEqual(port.tests(['virtual/virtual_wpt_dom/external/wpt/dom/ranges/Range-attributes.html']), dom_wpt)
+        self.assertEqual(port.tests(['virtual/virtual_wpt_dom/external/wpt/dom/ranges/Range-attributes.html']),
+                         ['virtual/virtual_wpt_dom/external/wpt/dom/ranges/Range-attributes.html'])
 
     def test_is_test_file(self):
         port = self.make_port(with_tests=True)
@@ -609,16 +617,37 @@ class PortTest(unittest.TestCase):
 
     def test_is_slow_wpt_test(self):
         port = self.make_port(with_tests=True)
-        filesystem = port.host.filesystem
-        PortTest._add_manifest_to_mock_file_system(filesystem)
+        PortTest._add_manifest_to_mock_file_system(port.host.filesystem)
 
         self.assertFalse(port.is_slow_wpt_test('external/wpt/dom/ranges/Range-attributes.html'))
-        self.assertFalse(port.is_slow_wpt_test('dom/ranges/Range-attributes.html'))
         self.assertTrue(port.is_slow_wpt_test('external/wpt/dom/ranges/Range-attributes-slow.html'))
         self.assertTrue(port.is_slow_wpt_test('external/wpt/html/dom/elements/global-attributes/dir_auto-EN-L.html'))
+
+    def test_is_slow_wpt_test_with_variations(self):
+        port = self.make_port(with_tests=True)
+        PortTest._add_manifest_to_mock_file_system(port.host.filesystem)
+
+        self.assertFalse(port.is_slow_wpt_test('external/wpt/console/console-is-a-namespace.any.html'))
+        self.assertTrue(port.is_slow_wpt_test('external/wpt/console/console-is-a-namespace.any.worker.html'))
+        self.assertFalse(port.is_slow_wpt_test('external/wpt/html/parse.html?run_type=uri'))
+        self.assertTrue(port.is_slow_wpt_test('external/wpt/html/parse.html?run_type=write'))
+
+    def test_is_slow_wpt_test_takes_virtual_tests(self):
+        port = self.make_port(with_tests=True)
+        PortTest._add_manifest_to_mock_file_system(port.host.filesystem)
+
         self.assertFalse(port.is_slow_wpt_test('virtual/virtual_wpt/external/wpt/dom/ranges/Range-attributes.html'))
         self.assertTrue(port.is_slow_wpt_test('virtual/virtual_wpt/external/wpt/dom/ranges/Range-attributes-slow.html'))
 
+    def test_is_slow_wpt_test_returns_false_for_illegal_paths(self):
+        port = self.make_port(with_tests=True)
+        PortTest._add_manifest_to_mock_file_system(port.host.filesystem)
+
+        self.assertFalse(port.is_slow_wpt_test('dom/ranges/Range-attributes.html'))
+        self.assertFalse(port.is_slow_wpt_test('dom/ranges/Range-attributes-slow.html'))
+        self.assertFalse(port.is_slow_wpt_test('/dom/ranges/Range-attributes.html'))
+        self.assertFalse(port.is_slow_wpt_test('/dom/ranges/Range-attributes-slow.html'))
+
     def test_parse_reftest_list(self):
         port = self.make_port(with_tests=True)
         port.host.filesystem.files['bar/reftest.list'] = '\n'.join(['== test.html test-ref.html',