From 6ddad08d4a729d7d8fc1997ee2aab5f6be729c27 Mon Sep 17 00:00:00 2001 From: James Graham Date: Fri, 15 May 2020 20:18:39 +0100 Subject: [PATCH] Convert all script metadata to text 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. --- tools/lint/lint.py | 2 +- tools/manifest/item.py | 2 +- tools/manifest/sourcefile.py | 68 ++++++++++++------------- tools/manifest/tests/test_sourcefile.py | 20 ++++---- tools/serve/serve.py | 32 ++++++------ tools/wptrunner/wptrunner/wpttest.py | 2 +- 6 files changed, 63 insertions(+), 63 deletions(-) diff --git a/tools/lint/lint.py b/tools/lint/lint.py index 6a5359bfb8a877..c56720d9a3f712 100644 --- a/tools/lint/lint.py +++ b/tools/lint/lint.py @@ -652,7 +652,7 @@ def check_python_ast(repo_root, path, f): 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: diff --git a/tools/manifest/item.py b/tools/manifest/item.py index 6601f9c719e040..ee07f0d1d79717 100644 --- a/tools/manifest/item.py +++ b/tools/manifest/item.py @@ -212,7 +212,7 @@ def to_json(self): if self.quic is not None: rv[-1]["quic"] = self.quic 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 diff --git a/tools/manifest/sourcefile.py b/tools/manifest/sourcefile.py index f36ce1ab3304d7..5d8b73481e6e19 100644 --- a/tools/manifest/sourcefile.py +++ b/tools/manifest/sourcefile.py @@ -56,9 +56,9 @@ def replace_end(s, 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 @@ -70,25 +70,25 @@ def read_script_metadata(f, regexp): 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: @@ -98,46 +98,46 @@ def get_any_variants(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 @@ -462,7 +462,7 @@ def timeout_nodes(self): @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: @@ -479,7 +479,7 @@ def timeout(self): """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: @@ -641,8 +641,8 @@ def test_variants(self): 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: @@ -691,7 +691,7 @@ def quic(self): (`script_metadata()`). """ if self.script_metadata: - if any(m == (b"quic", b"true") for m in self.script_metadata): + if any(m == ("quic", "true") for m in self.script_metadata): return True if self.root is None: @@ -864,11 +864,11 @@ def manifest_items(self): )] 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 @@ -993,8 +993,8 @@ def manifest_items(self): 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 diff --git a/tools/manifest/tests/test_sourcefile.py b/tools/manifest/tests/test_sourcefile.py index a8fbc27071408d..4d2080a70283be 100644 --- a/tools/manifest/tests/test_sourcefile.py +++ b/tools/manifest/tests/test_sourcefile.py @@ -180,7 +180,7 @@ def test_worker_long_timeout(): 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 @@ -197,7 +197,7 @@ def test_window_long_timeout(): 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 @@ -278,7 +278,7 @@ def test_python_long_timeout(): 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 @@ -322,7 +322,7 @@ def test_multi_global_long_timeout(): 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 @@ -447,14 +447,14 @@ def test_multi_global_with_variants(): @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""", []), ]) diff --git a/tools/serve/serve.py b/tools/serve/serve.py index 45bfd4766ebcd2..0e5090c2eef80a 100644 --- a/tools/serve/serve.py +++ b/tools/serve/serve.py @@ -178,9 +178,9 @@ class HtmlWrapperHandler(WrapperHandler): 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 @@ -189,23 +189,23 @@ def check_exposure(self, request): self.global_type) def _meta_replacement(self, key, value): - if key == b"timeout": - if value == b"long": + if key == "timeout": + if value == "long": return '' - if key == b"title": - value = value.decode('utf-8').replace("&", "&").replace("<", "<") + if key == "title": + value = value.replace("&", "&").replace("<", "<") return '%s' % 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 '' % 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 = """ @@ -234,7 +234,7 @@ class WindowHandler(HtmlWrapperHandler): class AnyHtmlHandler(HtmlWrapperHandler): - global_type = b"window" + global_type = "window" path_replace = [(".any.html", ".any.js")] wrapper = """ @@ -254,7 +254,7 @@ class AnyHtmlHandler(HtmlWrapperHandler): class SharedWorkersHandler(HtmlWrapperHandler): - global_type = b"sharedworker" + global_type = "sharedworker" path_replace = [(".any.sharedworker.html", ".any.js", ".any.worker.js")] wrapper = """ @@ -269,7 +269,7 @@ class SharedWorkersHandler(HtmlWrapperHandler): class ServiceWorkersHandler(HtmlWrapperHandler): - global_type = b"serviceworker" + global_type = "serviceworker" path_replace = [(".any.serviceworker.html", ".any.js", ".any.worker.js")] wrapper = """ @@ -307,11 +307,11 @@ 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 diff --git a/tools/wptrunner/wptrunner/wpttest.py b/tools/wptrunner/wptrunner/wpttest.py index 4b43b73610a591..6b0fab6eebaf50 100644 --- a/tools/wptrunner/wptrunner/wpttest.py +++ b/tools/wptrunner/wptrunner/wpttest.py @@ -411,7 +411,7 @@ def from_manifest(cls, manifest_file, manifest_item, inherit_metadata, test_meta quic = manifest_item.quic if hasattr(manifest_item, "quic") 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,