Bug 1616075 [wpt PR 21838] - Improve python3 support of test_wpt, a=testonly
authorSergio <svillar@igalia.com>
Wed, 26 Feb 2020 10:41:53 +0000
changeset 515810 96817f4e2c2ce8314eb93005cedf6d2ad7e4f564
parent 515809 d1c66f7b8c5d56f5d9b6657f4ed4f2c0eb7f5b0a
child 515811 9c80a03f6b39b99f0643e43b6f2c3e50e5d1ebae
push id37162
push usernbeleuzu@mozilla.com
push dateThu, 27 Feb 2020 09:49:56 +0000
treeherdermozilla-central@9e8d5431c412 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1616075, 21838
milestone75.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 1616075 [wpt PR 21838] - Improve python3 support of test_wpt, a=testonly Automatic update from web-platform-tests Improve python3 support of test_wpt (#21838) Main change is that git commands return Text now instead of bytes. Updated callsites and also mypy comments. -- wpt-commits: b4023970550ce0d454e700fad25e957ad1366ceb wpt-pr: 21838
testing/web-platform/tests/tools/manifest/download.py
testing/web-platform/tests/tools/manifest/sourcefile.py
testing/web-platform/tests/tools/manifest/utils.py
testing/web-platform/tests/tools/manifest/vcs.py
testing/web-platform/tests/tools/wpt/testfiles.py
--- a/testing/web-platform/tests/tools/manifest/download.py
+++ b/testing/web-platform/tests/tools/manifest/download.py
@@ -21,16 +21,17 @@ from . import log
 
 MYPY = False
 if MYPY:
     # MYPY is set to True when run under Mypy.
     from typing import Any
     from typing import Callable
     from typing import List
     from typing import Optional
+    from typing import Text
 
 here = os.path.dirname(__file__)
 
 wpt_root = os.path.abspath(os.path.join(here, os.pardir, os.pardir))
 logger = log.get_logger()
 
 
 def abs_path(path):
@@ -45,19 +46,19 @@ def should_download(manifest_path, rebui
     mtime = datetime.fromtimestamp(os.path.getmtime(manifest_path))
     if mtime < datetime.now() - rebuild_time:
         return True
     logger.info("Skipping manifest download because existing file is recent")
     return False
 
 
 def merge_pr_tags(repo_root, max_count=50):
-    # type: (str, int) -> List[str]
+    # type: (str, int) -> List[Text]
     gitfunc = git(repo_root)
-    tags = []  # type: List[str]
+    tags = []  # type: List[Text]
     if gitfunc is None:
         return tags
     for line in gitfunc("log", "--format=%D", "--max-count=%s" % max_count).split("\n"):
         for ref in line.split(", "):
             if ref.startswith("tag: merge_pr_"):
                 tags.append(ref[5:])
     return tags
 
@@ -74,17 +75,17 @@ def score_name(name):
         if name.endswith(".json.bz2"):
             return 2
         if name.endswith(".json.gz"):
             return 3
     return None
 
 
 def github_url(tags):
-    # type: (List[str]) -> Optional[List[str]]
+    # type: (List[Text]) -> Optional[List[Text]]
     for tag in tags:
         url = "https://api.github.com/repos/web-platform-tests/wpt/releases/tags/%s" % tag
         try:
             resp = urlopen(url)
         except Exception:
             logger.warning("Fetching %s failed" % url)
             continue
 
@@ -106,18 +107,18 @@ def github_url(tags):
 
         return [item[1] for item in sorted(candidates)]
 
     return None
 
 
 def download_manifest(
         manifest_path,  # type: str
-        tags_func,  # type: Callable[[], List[str]]
-        url_func,  # type: Callable[[List[str]], Optional[List[str]]]
+        tags_func,  # type: Callable[[], List[Text]]
+        url_func,  # type: Callable[[List[Text]], Optional[List[Text]]]
         force=False  # type: bool
 ):
     # type: (...) -> bool
     if not force and not should_download(manifest_path):
         return False
 
     tags = tags_func()
 
--- a/testing/web-platform/tests/tools/manifest/sourcefile.py
+++ b/testing/web-platform/tests/tools/manifest/sourcefile.py
@@ -1,13 +1,13 @@
 import hashlib
 import re
 import os
 from collections import deque
-from six import binary_type, PY3
+from six import binary_type, PY3, iteritems
 from six.moves.urllib.parse import urljoin
 from fnmatch import fnmatch
 
 MYPY = False
 if MYPY:
     # MYPY is set to True when run under Mypy.
     from typing import Any
     from typing import AnyStr
@@ -190,17 +190,17 @@ class SourceFile(object):
                     "support",
                     "tools"}
 
     dir_path_non_test = {("css21", "archive"),
                          ("css", "CSS2", "archive"),
                          ("css", "common")}  # type: Set[Tuple[bytes, ...]]
 
     def __init__(self, tests_root, rel_path, url_base, hash=None, contents=None):
-        # type: (AnyStr, AnyStr, Text, Optional[bytes], Optional[bytes]) -> None
+        # type: (AnyStr, AnyStr, Text, Optional[Text], Optional[bytes]) -> None
         """Object representing a file in a source tree.
 
         :param tests_root: Path to the root of the source tree
         :param rel_path: File path relative to tests_root
         :param url_base: Base URL used when converting file paths to urls
         :param contents: Byte array of the contents of the file or ``None``.
         """
 
@@ -237,19 +237,17 @@ class SourceFile(object):
 
     def __getstate__(self):
         # type: () -> Dict[str, Any]
         # Remove computed properties if we pickle this class
         rv = self.__dict__.copy()
 
         if "__cached_properties__" in rv:
             cached_properties = rv["__cached_properties__"]
-            for key in rv.keys():
-                if key in cached_properties:
-                    del rv[key]
+            rv = {key:value for key, value in iteritems(rv) if key not in cached_properties}
             del rv["__cached_properties__"]
         return rv
 
     def name_prefix(self, prefix):
         # type: (bytes) -> bool
         """Check if the filename starts with a given prefix
 
         :param prefix: The prefix to check"""
@@ -299,17 +297,17 @@ class SourceFile(object):
 
     @cached_property
     def url(self):
         # type: () -> Text
         return urljoin(self.url_base, self.rel_url)
 
     @cached_property
     def hash(self):
-        # type: () -> bytes
+        # type: () -> Text
         if not self._hash:
             with self.open() as f:
                 content = f.read()
 
             data = b"".join((b"blob ", b"%d" % len(content), b"\0", content))
             hash_str = hashlib.sha1(data).hexdigest()  # type: str
             if PY3:
                 self._hash = hash_str.encode("ascii")
--- a/testing/web-platform/tests/tools/manifest/utils.py
+++ b/testing/web-platform/tests/tools/manifest/utils.py
@@ -50,26 +50,26 @@ def to_os_path(path):
     if "\\" in path:
         raise ValueError("normalised path contains \\")
     if "/" == os.path.sep:
         return path
     return path.replace("/", os.path.sep)
 
 
 def git(path):
-    # type: (bytes) -> Optional[Callable[..., bytes]]
+    # type: (bytes) -> Optional[Callable[..., Text]]
     def gitfunc(cmd, *args):
-        # type: (bytes, *bytes) -> bytes
+        # type: (bytes, *bytes) -> Text
         full_cmd = ["git", cmd] + list(args)
         try:
-            return subprocess.check_output(full_cmd, cwd=path, stderr=subprocess.STDOUT)
+            return subprocess.check_output(full_cmd, cwd=path, stderr=subprocess.STDOUT).decode('utf8')
         except Exception as e:
             if platform.uname()[0] == "Windows" and isinstance(e, WindowsError):
                 full_cmd[0] = "git.bat"
-                return subprocess.check_output(full_cmd, cwd=path, stderr=subprocess.STDOUT)
+                return subprocess.check_output(full_cmd, cwd=path, stderr=subprocess.STDOUT).decode('utf8')
             else:
                 raise
 
     try:
         # this needs to be a command that fails if we aren't in a git repo
         gitfunc("rev-parse", "--show-toplevel")
     except (subprocess.CalledProcessError, OSError):
         return None
--- a/testing/web-platform/tests/tools/manifest/vcs.py
+++ b/testing/web-platform/tests/tools/manifest/vcs.py
@@ -28,17 +28,17 @@ if MYPY:
         stat_result = os.stat_result
 
 
 def get_tree(tests_root, manifest, manifest_path, cache_root,
              working_copy=True, rebuild=False):
     # type: (bytes, Manifest, Optional[bytes], Optional[bytes], bool, bool) -> FileSystem
     tree = None
     if cache_root is None:
-        cache_root = os.path.join(tests_root, b".wptcache")
+        cache_root = os.path.join(tests_root, ".wptcache")
     if not os.path.exists(cache_root):
         try:
             os.makedirs(cache_root)
         except IOError:
             cache_root = None
 
     if not working_copy:
         raise ValueError("working_copy=False unsupported")
@@ -53,42 +53,42 @@ def get_tree(tests_root, manifest, manif
 
 
 class GitHasher(object):
     def __init__(self, path):
         # type: (bytes) -> None
         self.git = git(path)
 
     def _local_changes(self):
-        # type: () -> Set[bytes]
+        # type: () -> Set[Text]
         """get a set of files which have changed between HEAD and working copy"""
         assert self.git is not None
         # note that git runs the command with tests_root as the cwd, which may
         # not be the root of the git repo (e.g., within a browser repo)
         cmd = [b"diff-index", b"--relative", b"--no-renames", b"--name-only", b"-z", b"HEAD"]
         data = self.git(*cmd)
-        return set(data.split(b"\0"))
+        return set(data.split("\0"))
 
     def hash_cache(self):
-        # type: () -> Dict[bytes, Optional[bytes]]
+        # type: () -> Dict[Text, Optional[Text]]
         """
         A dict of rel_path -> current git object id if the working tree matches HEAD else None
         """
-        hash_cache = {}  # type: Dict[bytes, Optional[bytes]]
+        hash_cache = {}  # type: Dict[Text, Optional[Text]]
 
         if self.git is None:
             return hash_cache
 
         # note that git runs the command with tests_root as the cwd, which may
         # not be the root of the git repo (e.g., within a browser repo)
         cmd = ["ls-tree", "-r", "-z", "HEAD"]
         local_changes = self._local_changes()
-        for result in self.git(*cmd).split(b"\0")[:-1]:  # type: bytes
-            data, rel_path = result.rsplit(b"\t", 1)
-            hash_cache[rel_path] = None if rel_path in local_changes else data.split(b" ", 3)[2]
+        for result in self.git(*cmd).split("\0")[:-1]:  # type: Text
+            data, rel_path = result.rsplit("\t", 1)
+            hash_cache[rel_path] = None if rel_path in local_changes else data.split(" ", 3)[2]
 
         return hash_cache
 
 
 
 class FileSystem(object):
     def __init__(self, root, url_base, cache_path, manifest_path=None, rebuild=False):
         # type: (bytes, Text, Optional[bytes], Optional[bytes], bool) -> None
@@ -169,17 +169,17 @@ class CacheFile(with_metaclass(abc.ABCMe
     def check_valid(self, data):
         # type: (Dict[Any, Any]) -> Dict[Any, Any]
         """Check if the cached data is valid and return an updated copy of the
         cache containing only data that can be used."""
         return data
 
 
 class MtimeCache(CacheFile):
-    file_name = b"mtime.json"
+    file_name = "mtime.json"
 
     def __init__(self, cache_root, tests_root, manifest_path, rebuild=False):
         # type: (bytes, bytes, bytes, bool) -> None
         self.manifest_path = manifest_path
         super(MtimeCache, self).__init__(cache_root, tests_root, rebuild)
 
     def updated(self, rel_path, stat):
         # type: (bytes, stat_result) -> bool
@@ -217,17 +217,17 @@ class MtimeCache(CacheFile):
             return
         mtime = os.path.getmtime(self.manifest_path)
         self.data["/manifest_path"] = [self.manifest_path, mtime]
         self.data["/tests_root"] = self.tests_root
         super(MtimeCache, self).dump()
 
 
 class GitIgnoreCache(CacheFile, MutableMapping):  # type: ignore
-    file_name = b"gitignore.json"
+    file_name = "gitignore.json"
 
     def check_valid(self, data):
         # type: (Dict[Any, Any]) -> Dict[Any, Any]
         ignore_path = os.path.join(self.tests_root, b".gitignore")
         mtime = os.path.getmtime(ignore_path)
         if data.get(b"/gitignore_file") != [ignore_path, mtime]:
             self.modified = True
             data = {}
@@ -281,17 +281,17 @@ def walk(root):
     get_stat = os.stat
     is_dir = stat.S_ISDIR
     is_link = stat.S_ISLNK
     join = os.path.join
     listdir = os.listdir
     relpath = os.path.relpath
 
     root = os.path.abspath(root)
-    stack = deque([(root, b"")])
+    stack = deque([(root, "")])
 
     while stack:
         dir_path, rel_path = stack.popleft()
         try:
             # Note that listdir and error are globals in this module due
             # to earlier import-*.
             names = listdir(dir_path)
         except OSError:
--- a/testing/web-platform/tests/tools/wpt/testfiles.py
+++ b/testing/web-platform/tests/tools/wpt/testfiles.py
@@ -294,17 +294,17 @@ def affected_testfiles(files_changed,  #
                 if not relpath.startswith(os.pardir):
                     # testfile is in subtree of support file
                     affected = True
                     break
         return affected
 
     def affected_by_interfaces(file_contents):
         # type: (Union[bytes, Text]) -> bool
-        if len(interfaces_changed_names) > 0:
+        if len(interfaces_changed) > 0:
             if 'idlharness.js' in file_contents:
                 for interface in interfaces_changed_names:
                     regex = '[\'"]' + interface + '(\\.idl)?[\'"]'
                     if re.search(regex, file_contents):
                         return True
         return False
 
     for root, dirs, fnames in os.walk(wpt_root):
@@ -319,19 +319,19 @@ def affected_testfiles(files_changed,  #
             if test_full_path not in test_files:
                 continue
             if affected_by_wdspec(test_full_path):
                 affected_testfiles.add(test_full_path)
                 continue
 
             with open(test_full_path, "rb") as fh:
                 raw_file_contents = fh.read()  # type: bytes
-                if raw_file_contents.startswith("\xfe\xff"):
+                if raw_file_contents.startswith(b"\xfe\xff"):
                     file_contents = raw_file_contents.decode("utf-16be", "replace")  # type: Text
-                elif raw_file_contents.startswith("\xff\xfe"):
+                elif raw_file_contents.startswith(b"\xff\xfe"):
                     file_contents = raw_file_contents.decode("utf-16le", "replace")
                 else:
                     file_contents = raw_file_contents.decode("utf8", "replace")
                 for full_path, repo_path in nontest_changed_paths:
                     rel_path = os.path.relpath(full_path, root).replace(os.path.sep, "/")
                     if rel_path in file_contents or repo_path in file_contents or affected_by_interfaces(file_contents):
                         affected_testfiles.add(test_full_path)
                         continue