Bug 1638435 [wpt PR 23644] - Convert all script metadata to text, a=testonly
authorJames Graham <james@hoppipolla.co.uk>
Mon, 18 May 2020 14:21:48 +0000
changeset 531199 e752bc588a218457572454e2da885a59ff4e3e4f
parent 531198 742dba3065f5d30fe17728d6c64e8fc758552229
child 531200 b7bfbeddfe2a01669f5a316b9867618e51b3a633
push id37435
push userapavel@mozilla.com
push dateWed, 20 May 2020 15:28:23 +0000
treeherdermozilla-central@5415da14ec9a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1638435, 23644
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1638435 [wpt PR 23644] - Convert all script metadata to text, a=testonly SKIP_BMO_CHECK Otherwise we were getting a different type when loading the manifest compared to when reading data from the initial file. This caused problems in Python 3. wpt-commit: 6ddad08d4a729d7d8fc1997ee2aab5f6be729c27 wpt-pr: 23644
testing/web-platform/tests/tools/lint/lint.py
testing/web-platform/tests/tools/manifest/item.py
testing/web-platform/tests/tools/manifest/sourcefile.py
testing/web-platform/tests/tools/manifest/tests/test_sourcefile.py
testing/web-platform/tests/tools/serve/serve.py
testing/web-platform/tests/tools/wptrunner/wptrunner/wpttest.py
--- a/testing/web-platform/tests/tools/lint/lint.py
+++ b/testing/web-platform/tests/tools/lint/lint.py
@@ -612,17 +612,17 @@ def check_python_ast(repo_root, path, f)
 
 
 broken_js_metadata = re.compile(br"//\s*META:")
 broken_python_metadata = re.compile(br"#\s*META:")
 
 
 def check_global_metadata(value):
     # type: (str) -> Iterable[Tuple[Type[rules.Rule], Tuple[Any, ...]]]
-    global_values = {item.strip() for item in value.split(b",") if item.strip()}
+    global_values = {item.strip().decode("utf8") for item in value.split(b",") if item.strip()}
 
     # TODO: this could check for duplicates and such
     for global_value in global_values:
         if not get_any_variants(global_value):
             yield (rules.UnknownGlobalMetadata, ())
 
 
 def check_script_metadata(repo_root, path, f):
--- a/testing/web-platform/tests/tools/manifest/item.py
+++ b/testing/web-platform/tests/tools/manifest/item.py
@@ -200,17 +200,17 @@ class TestharnessTest(URLManifestItem):
         rv = super(TestharnessTest, self).to_json()
         if self.timeout is not None:
             rv[-1]["timeout"] = self.timeout
         if self.testdriver:
             rv[-1]["testdriver"] = self.testdriver
         if self.jsshell:
             rv[-1]["jsshell"] = True
         if self.script_metadata:
-            rv[-1]["script_metadata"] = [(k.decode('utf8'), v.decode('utf8')) for (k,v) in self.script_metadata]
+            rv[-1]["script_metadata"] = [(k, v) for (k,v) in self.script_metadata]
         return rv
 
 
 class RefTest(URLManifestItem):
     __slots__ = ("references",)
 
     item_type = "reftest"
 
--- a/testing/web-platform/tests/tools/manifest/sourcefile.py
+++ b/testing/web-platform/tests/tools/manifest/sourcefile.py
@@ -51,98 +51,98 @@ def replace_end(s, old, new):
     Given a string `s` that ends with `old`, replace that occurrence of `old`
     with `new`.
     """
     assert s.endswith(old)
     return s[:-len(old)] + new
 
 
 def read_script_metadata(f, regexp):
-    # type: (BinaryIO, Pattern[bytes]) -> Iterable[Tuple[bytes, bytes]]
+    # type: (BinaryIO, Pattern[bytes]) -> Iterable[Tuple[Text, Text]]
     """
-    Yields any metadata (pairs of bytestrings) from the file-like object `f`,
+    Yields any metadata (pairs of strings) from the file-like object `f`,
     as specified according to a supplied regexp.
 
     `regexp` - Regexp containing two groups containing the metadata name and
                value.
     """
     for line in f:
         assert isinstance(line, binary_type), line
         m = regexp.match(line)
         if not m:
             break
 
-        yield (m.groups()[0], m.groups()[1])
+        yield (m.groups()[0].decode("utf8"), m.groups()[1].decode("utf8"))
 
 
 _any_variants = {
-    b"window": {"suffix": ".any.html"},
-    b"serviceworker": {"force_https": True},
-    b"sharedworker": {},
-    b"dedicatedworker": {"suffix": ".any.worker.html"},
-    b"worker": {"longhand": {b"dedicatedworker", b"sharedworker", b"serviceworker"}},
-    b"jsshell": {"suffix": ".any.js"},
-}  # type: Dict[bytes, Dict[str, Any]]
+    "window": {"suffix": ".any.html"},
+    "serviceworker": {"force_https": True},
+    "sharedworker": {},
+    "dedicatedworker": {"suffix": ".any.worker.html"},
+    "worker": {"longhand": {"dedicatedworker", "sharedworker", "serviceworker"}},
+    "jsshell": {"suffix": ".any.js"},
+}  # type: Dict[Text, Dict[Text, Any]]
 
 
 def get_any_variants(item):
-    # type: (bytes) -> Set[bytes]
+    # type: (Text) -> Set[Text]
     """
-    Returns a set of variants (bytestrings) defined by the given keyword.
+    Returns a set of variants (strings) defined by the given keyword.
     """
-    assert isinstance(item, binary_type), item
+    assert isinstance(item, text_type), item
 
     variant = _any_variants.get(item, None)
     if variant is None:
         return set()
 
     return variant.get("longhand", {item})
 
 
 def get_default_any_variants():
-    # type: () -> Set[bytes]
+    # type: () -> Set[Text]
     """
-    Returns a set of variants (bytestrings) that will be used by default.
+    Returns a set of variants (strings) that will be used by default.
     """
-    return set({b"window", b"dedicatedworker"})
+    return set({"window", "dedicatedworker"})
 
 
 def parse_variants(value):
-    # type: (bytes) -> Set[bytes]
+    # type: (Text) -> Set[Text]
     """
-    Returns a set of variants (bytestrings) defined by a comma-separated value.
+    Returns a set of variants (strings) defined by a comma-separated value.
     """
-    assert isinstance(value, binary_type), value
+    assert isinstance(value, text_type), value
 
-    if value == b"":
+    if value == "":
         return get_default_any_variants()
 
     globals = set()
-    for item in value.split(b","):
+    for item in value.split(","):
         item = item.strip()
         globals |= get_any_variants(item)
     return globals
 
 
 def global_suffixes(value):
-    # type: (bytes) -> Set[Tuple[bytes, bool]]
+    # type: (Text) -> Set[Tuple[Text, bool]]
     """
     Yields tuples of the relevant filename suffix (a string) and whether the
     variant is intended to run in a JS shell, for the variants defined by the
     given comma-separated value.
     """
-    assert isinstance(value, binary_type), value
+    assert isinstance(value, text_type), value
 
     rv = set()
 
     global_types = parse_variants(value)
     for global_type in global_types:
         variant = _any_variants[global_type]
-        suffix = variant.get("suffix", ".any.%s.html" % global_type.decode("utf-8"))
-        rv.add((suffix, global_type == b"jsshell"))
+        suffix = variant.get("suffix", ".any.%s.html" % global_type)
+        rv.add((suffix, global_type == "jsshell"))
 
     return rv
 
 
 def global_variant_url(url, suffix):
     # type: (Text, Text) -> Text
     """
     Returns a url created from the given url and suffix (all strings).
@@ -457,34 +457,34 @@ class SourceFile(object):
         # type: () -> List[ElementTree.Element]
         """List of ElementTree Elements corresponding to nodes in a test that
         specify timeouts"""
         assert self.root is not None
         return self.root.findall(".//{http://www.w3.org/1999/xhtml}meta[@name='timeout']")
 
     @cached_property
     def script_metadata(self):
-        # type: () -> Optional[List[Tuple[bytes, bytes]]]
+        # type: () -> Optional[List[Tuple[Text, Text]]]
         if self.name_is_worker or self.name_is_multi_global or self.name_is_window:
             regexp = js_meta_re
         elif self.name_is_webdriver:
             regexp = python_meta_re
         else:
             return None
 
         with self.open() as f:
             return list(read_script_metadata(f, regexp))
 
     @cached_property
     def timeout(self):
         # type: () -> Optional[Text]
         """The timeout of a test or reference file. "long" if the file has an extended timeout
         or None otherwise"""
         if self.script_metadata:
-            if any(m == (b"timeout", b"long") for m in self.script_metadata):
+            if any(m == ("timeout", "long") for m in self.script_metadata):
                 return "long"
 
         if self.root is None:
             return None
 
         if self.timeout_nodes:
             timeout_str = self.timeout_nodes[0].attrib.get("content", None)  # type: Optional[Text]
             if timeout_str and timeout_str.lower() == "long":
@@ -636,18 +636,18 @@ class SourceFile(object):
     @cached_property
     def test_variants(self):
         # type: () -> List[Text]
         rv = []  # type: List[Text]
         if self.ext == ".js":
             script_metadata = self.script_metadata
             assert script_metadata is not None
             for (key, value) in script_metadata:
-                if key == b"variant":
-                    rv.append(value.decode("utf-8"))
+                if key == "variant":
+                    rv.append(value)
         else:
             for element in self.variant_nodes:
                 if "content" in element.attrib:
                     variant = element.attrib["content"]  # type: Text
                     rv.append(variant)
 
         for variant in rv:
             assert variant == "" or variant[0] in ["#", "?"], variant
@@ -829,21 +829,21 @@ class SourceFile(object):
                 CrashTest(
                     self.tests_root,
                     self.rel_path,
                     self.url_base,
                     self.rel_url
                 )]
 
         elif self.name_is_multi_global:
-            globals = b""
+            globals = u""
             script_metadata = self.script_metadata
             assert script_metadata is not None
             for (key, value) in script_metadata:
-                if key == b"global":
+                if key == "global":
                     globals = value
                     break
 
             tests = [
                 TestharnessTest(
                     self.tests_root,
                     self.rel_path,
                     self.url_base,
@@ -953,13 +953,13 @@ class SourceFile(object):
 
         assert len(rv[1]) == len(set(rv[1]))
 
         self.items_cache = rv
 
         if drop_cached and "__cached_properties__" in self.__dict__:
             cached_properties = self.__dict__["__cached_properties__"]
             for key in cached_properties:
-                if key in self.__dict__:
-                    del self.__dict__[key]
+                if str(key) in self.__dict__:
+                    del self.__dict__[str(key)]
             del self.__dict__["__cached_properties__"]
 
         return rv
--- a/testing/web-platform/tests/tools/manifest/tests/test_sourcefile.py
+++ b/testing/web-platform/tests/tools/manifest/tests/test_sourcefile.py
@@ -171,34 +171,34 @@ def test_window():
 
 
 def test_worker_long_timeout():
     contents = b"""// META: timeout=long
 importScripts('/resources/testharness.js')
 test()"""
 
     metadata = list(read_script_metadata(BytesIO(contents), js_meta_re))
-    assert metadata == [(b"timeout", b"long")]
+    assert metadata == [("timeout", "long")]
 
     s = create("html/test.worker.js", contents=contents)
     assert s.name_is_worker
 
     item_type, items = s.manifest_items()
     assert item_type == "testharness"
 
     for item in items:
         assert item.timeout == "long"
 
 
 def test_window_long_timeout():
     contents = b"""// META: timeout=long
 test()"""
 
     metadata = list(read_script_metadata(BytesIO(contents), js_meta_re))
-    assert metadata == [(b"timeout", b"long")]
+    assert metadata == [("timeout", "long")]
 
     s = create("html/test.window.js", contents=contents)
     assert s.name_is_window
 
     item_type, items = s.manifest_items()
     assert item_type == "testharness"
 
     for item in items:
@@ -267,17 +267,17 @@ test()"""
 
 def test_python_long_timeout():
     contents = b"""# META: timeout=long
 
 """
 
     metadata = list(read_script_metadata(BytesIO(contents),
                                          python_meta_re))
-    assert metadata == [(b"timeout", b"long")]
+    assert metadata == [("timeout", "long")]
 
     s = create("webdriver/test.py", contents=contents)
     assert s.name_is_webdriver
 
     item_type, items = s.manifest_items()
     assert item_type == "wdspec"
 
     for item in items:
@@ -310,17 +310,17 @@ def test_multi_global():
 
 
 def test_multi_global_long_timeout():
     contents = b"""// META: timeout=long
 importScripts('/resources/testharness.js')
 test()"""
 
     metadata = list(read_script_metadata(BytesIO(contents), js_meta_re))
-    assert metadata == [(b"timeout", b"long")]
+    assert metadata == [("timeout", "long")]
 
     s = create("html/test.any.js", contents=contents)
     assert s.name_is_multi_global
 
     item_type, items = s.manifest_items()
     assert item_type == "testharness"
 
     for item in items:
@@ -432,24 +432,24 @@ test()"""
     assert len(items) == len(expected_urls)
 
     for item, url in zip(items, expected_urls):
         assert item.url == url
         assert item.timeout is None
 
 
 @pytest.mark.parametrize("input,expected", [
-    (b"""//META: foo=bar\n""", [(b"foo", b"bar")]),
-    (b"""// META: foo=bar\n""", [(b"foo", b"bar")]),
-    (b"""//  META: foo=bar\n""", [(b"foo", b"bar")]),
+    (b"""//META: foo=bar\n""", [("foo", "bar")]),
+    (b"""// META: foo=bar\n""", [("foo", "bar")]),
+    (b"""//  META: foo=bar\n""", [("foo", "bar")]),
     (b"""\n// META: foo=bar\n""", []),
     (b""" // META: foo=bar\n""", []),
-    (b"""// META: foo=bar\n// META: baz=quux\n""", [(b"foo", b"bar"), (b"baz", b"quux")]),
-    (b"""// META: foo=bar\n\n// META: baz=quux\n""", [(b"foo", b"bar")]),
-    (b"""// META: foo=bar\n// Start of the test\n// META: baz=quux\n""", [(b"foo", b"bar")]),
+    (b"""// META: foo=bar\n// META: baz=quux\n""", [("foo", "bar"), ("baz", "quux")]),
+    (b"""// META: foo=bar\n\n// META: baz=quux\n""", [("foo", "bar")]),
+    (b"""// META: foo=bar\n// Start of the test\n// META: baz=quux\n""", [("foo", "bar")]),
     (b"""// META:\n""", []),
     (b"""// META: foobar\n""", []),
 ])
 def test_script_metadata(input, expected):
     metadata = read_script_metadata(BytesIO(input), js_meta_re)
     assert list(metadata) == expected
 
 
--- a/testing/web-platform/tests/tools/serve/serve.py
+++ b/testing/web-platform/tests/tools/serve/serve.py
@@ -173,44 +173,44 @@ class WrapperHandler(object):
 
 
 class HtmlWrapperHandler(WrapperHandler):
     global_type = None
     headers = [('Content-Type', 'text/html')]
 
     def check_exposure(self, request):
         if self.global_type:
-            globals = b""
+            globals = u""
             for (key, value) in self._get_metadata(request):
-                if key == b"global":
+                if key == "global":
                     globals = value
                     break
 
             if self.global_type not in parse_variants(globals):
                 raise HTTPException(404, "This test cannot be loaded in %s mode" %
                                     self.global_type)
 
     def _meta_replacement(self, key, value):
-        if key == b"timeout":
-            if value == b"long":
+        if key == "timeout":
+            if value == "long":
                 return '<meta name="timeout" content="long">'
-        if key == b"title":
-            value = value.decode('utf-8').replace("&", "&amp;").replace("<", "&lt;")
+        if key == "title":
+            value = value.replace("&", "&amp;").replace("<", "&lt;")
             return '<title>%s</title>' % value
         return None
 
     def _script_replacement(self, key, value):
-        if key == b"script":
-            attribute = value.decode('utf-8').replace("&", "&amp;").replace('"', "&quot;")
+        if key == "script":
+            attribute = value.replace("&", "&amp;").replace('"', "&quot;")
             return '<script src="%s"></script>' % attribute
         return None
 
 
 class WorkersHandler(HtmlWrapperHandler):
-    global_type = b"dedicatedworker"
+    global_type = "dedicatedworker"
     path_replace = [(".any.worker.html", ".any.js", ".any.worker.js"),
                     (".worker.html", ".worker.js")]
     wrapper = """<!doctype html>
 <meta charset=utf-8>
 %(meta)s
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <div id=log></div>
@@ -229,17 +229,17 @@ class WindowHandler(HtmlWrapperHandler):
 <script src="/resources/testharnessreport.js"></script>
 %(script)s
 <div id=log></div>
 <script src="%(path)s"></script>
 """
 
 
 class AnyHtmlHandler(HtmlWrapperHandler):
-    global_type = b"window"
+    global_type = "window"
     path_replace = [(".any.html", ".any.js")]
     wrapper = """<!doctype html>
 <meta charset=utf-8>
 %(meta)s
 <script>
 self.GLOBAL = {
   isWindow: function() { return true; },
   isWorker: function() { return false; },
@@ -249,32 +249,32 @@ self.GLOBAL = {
 <script src="/resources/testharnessreport.js"></script>
 %(script)s
 <div id=log></div>
 <script src="%(path)s"></script>
 """
 
 
 class SharedWorkersHandler(HtmlWrapperHandler):
-    global_type = b"sharedworker"
+    global_type = "sharedworker"
     path_replace = [(".any.sharedworker.html", ".any.js", ".any.worker.js")]
     wrapper = """<!doctype html>
 <meta charset=utf-8>
 %(meta)s
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <div id=log></div>
 <script>
 fetch_tests_from_worker(new SharedWorker("%(path)s%(query)s"));
 </script>
 """
 
 
 class ServiceWorkersHandler(HtmlWrapperHandler):
-    global_type = b"serviceworker"
+    global_type = "serviceworker"
     path_replace = [(".any.serviceworker.html", ".any.js", ".any.worker.js")]
     wrapper = """<!doctype html>
 <meta charset=utf-8>
 %(meta)s
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <div id=log></div>
 <script>
@@ -302,21 +302,21 @@ importScripts("/resources/testharness.js
 importScripts("%(path)s");
 done();
 """
 
     def _meta_replacement(self, key, value):
         return None
 
     def _script_replacement(self, key, value):
-        if key == b"script":
-            attribute = value.decode('utf-8').replace("\\", "\\\\").replace('"', '\\"')
+        if key == "script":
+            attribute = value.replace("\\", "\\\\").replace('"', '\\"')
             return 'importScripts("%s")' % attribute
-        if key == b"title":
-            value = value.decode('utf-8').replace("\\", "\\\\").replace('"', '\\"')
+        if key == "title":
+            value = value.replace("\\", "\\\\").replace('"', '\\"')
             return 'self.META_TITLE = "%s";' % value
         return None
 
 
 rewrites = [("GET", "/resources/WebIDLParser.js", "/resources/webidl2/lib/webidl2.js")]
 
 
 class RoutesBuilder(object):
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/wpttest.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/wpttest.py
@@ -406,17 +406,17 @@ class TestharnessTest(Test):
 
     @classmethod
     def from_manifest(cls, manifest_file, manifest_item, inherit_metadata, test_metadata):
         timeout = cls.long_timeout if manifest_item.timeout == "long" else cls.default_timeout
         testdriver = manifest_item.testdriver if hasattr(manifest_item, "testdriver") else False
         jsshell = manifest_item.jsshell if hasattr(manifest_item, "jsshell") else False
         script_metadata = manifest_item.script_metadata or []
         scripts = [v for (k, v) in script_metadata
-                   if k in (b"script", "script")]
+                   if k == "script"]
         return cls(manifest_file.tests_root,
                    manifest_item.url,
                    inherit_metadata,
                    test_metadata,
                    timeout=timeout,
                    path=os.path.join(manifest_file.tests_root, manifest_item.path),
                    protocol=server_protocol(manifest_item),
                    testdriver=testdriver,