Bug 1555432 [wpt PR 17079] - Get more of tools/wpt/tests passing on Windows, a=testonly
authorSam Sneddon <me@gsnedders.com>
Thu, 13 Jun 2019 15:31:15 +0000
changeset 479605 7e1ad3f296d5f549102ad9634464291bfd4cd8cd
parent 479604 f17b9d4d942b82c0aae232e008895b9bdc458a86
child 479606 ea29e77dd7357d8fa1f8b317acca8153c3455489
push id36179
push usercsabou@mozilla.com
push dateThu, 20 Jun 2019 22:06:31 +0000
treeherdermozilla-central@4cde299454c9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1555432, 17079, 17075, 17077
milestone69.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 1555432 [wpt PR 17079] - Get more of tools/wpt/tests passing on Windows, a=testonly Automatic update from web-platform-tests Windows calls the arch AMD64, not x86_64 -- files-changed outputs OS paths -- Binary data needs to be written as binary data on Windows -- Make test_serve skipped on Windows with a clear reason -- Provide a more specific link for this xfail -- Fix #17075: canonicalize case when checking for certutil.exe -- Fix #17077: Teach wpt.browser about Chrome on Windows -- Teach tools.wpt.browser how to find Firefox on Windows -- wp5At-commits: de25e04947dab014dfa623948e055594e203131f, 4f975b7207653319b363e549feb87715a7a29609, ffaaaca11bb8d297feba557692f00771618fe7e9, f00a22e88fe42bcc06e57768f8da9eecaa643556, f2eddb9c548d43c58fcb3ca5b6afd1b680a17077, 820c319869533873d119b929b5692bb172a2c33a, 36a63ef48af0e9a99e790727ff1143aa11e30dad, 744c614ea83973a5316fdec3b0f99ecef7820a28 wpt-pr: 17079
testing/web-platform/tests/tools/wpt/browser.py
testing/web-platform/tests/tools/wpt/tests/test_wpt.py
--- a/testing/web-platform/tests/tools/wpt/browser.py
+++ b/testing/web-platform/tests/tools/wpt/browser.py
@@ -12,16 +12,26 @@ from distutils.spawn import find_executa
 
 import requests
 
 from utils import call, get, untar, unzip
 
 uname = platform.uname()
 
 
+def _get_fileversion(binary, logger=None):
+    command = "(Get-Item '%s').VersionInfo.FileVersion" % binary.replace("'", "''")
+    try:
+        return call("powershell.exe", command).strip()
+    except (subprocess.CalledProcessError, OSError):
+        if logger is not None:
+            logger.warning("Failed to call %s in PowerShell" % command)
+        return None
+
+
 class Browser(object):
     __metaclass__ = ABCMeta
 
     def __init__(self, logger):
         self.logger = logger
 
     @abstractmethod
     def install(self, dest=None):
@@ -103,17 +113,17 @@ class Firefox(Browser):
             "beta": "firefox-beta-latest-ssl",
             "stable": "firefox-latest-ssl"
         }
 
         os_builds = {
             ("linux", "x86"): "linux",
             ("linux", "x86_64"): "linux64",
             ("win", "x86"): "win",
-            ("win", "x86_64"): "win64",
+            ("win", "AMD64"): "win64",
             ("macos", "x86_64"): "osx",
         }
         os_key = (self.platform, uname[4])
 
         if channel not in product:
             raise ValueError("Unrecognised release channel: %s" % channel)
 
         if os_key not in os_builds:
@@ -144,17 +154,17 @@ class Firefox(Browser):
         if not filename:
             filename = urlparse.urlsplit(resp.url).path.rsplit("/", 1)[1]
 
         if not filename:
             filename = "firefox.tar.bz2"
 
         installer_path = os.path.join(dest, filename)
 
-        with open(installer_path, "w") as f:
+        with open(installer_path, "wb") as f:
             f.write(resp.content)
 
         try:
             mozinstall.install(installer_path, dest)
         except mozinstall.mozinstall.InstallError:
             if self.platform == "macos" and os.path.exists(os.path.join(dest, self.application_name.get(channel, "Firefox Nightly.app"))):
                 # mozinstall will fail if nightly is already installed in the venv because
                 # mac installation uses shutil.copy_tree
@@ -192,16 +202,24 @@ class Firefox(Browser):
 
     def find_binary(self, venv_path=None, channel="nightly"):
         if venv_path is None:
             venv_path = os.path.join(os.getcwd(), "_venv")
 
         path = os.path.join(venv_path, "browsers", channel)
         binary = self.find_binary_path(path, channel)
 
+        if not binary and self.platform == "win":
+            winpaths = [os.path.expandvars("$SYSTEMDRIVE\\Program Files\\Mozilla Firefox"),
+                        os.path.expandvars("$SYSTEMDRIVE\\Program Files (x86)\\Mozilla Firefox")]
+            for winpath in winpaths:
+                binary = self.find_binary_path(winpath, channel)
+                if binary is not None:
+                    break
+
         if not binary and self.platform == "macos":
             macpaths = ["/Applications/Firefox Nightly.app/Contents/MacOS",
                         os.path.expanduser("~/Applications/Firefox Nightly.app/Contents/MacOS"),
                         "/Applications/Firefox Developer Edition.app/Contents/MacOS",
                         os.path.expanduser("~/Applications/Firefox Developer Edition.app/Contents/MacOS"),
                         "/Applications/Firefox.app/Contents/MacOS",
                         os.path.expanduser("~/Applications/Firefox.app/Contents/MacOS")]
             return find_executable("firefox", os.pathsep.join(macpaths))
@@ -210,17 +228,17 @@ class Firefox(Browser):
             return find_executable("firefox")
 
         return binary
 
     def find_certutil(self):
         path = find_executable("certutil")
         if path is None:
             return None
-        if os.path.splitdrive(path)[1].split(os.path.sep) == ["", "Windows", "system32", "certutil.exe"]:
+        if os.path.splitdrive(os.path.normcase(path))[1].split(os.path.sep) == ["", "windows", "system32", "certutil.exe"]:
             return None
         return path
 
     def find_webdriver(self, channel=None):
         return find_executable("geckodriver")
 
     def get_version_and_channel(self, binary):
         version_string = call(binary, "--version").strip()
@@ -413,17 +431,18 @@ class Chrome(Browser):
     requirements = "requirements_chrome.txt"
 
     @property
     def binary(self):
         if uname[0] == "Linux":
             return "/usr/bin/google-chrome"
         if uname[0] == "Darwin":
             return "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome"
-        # TODO Windows?
+        if uname[0] == "Windows":
+            return os.path.expandvars("$SYSTEMDRIVE\\Program Files (x86)\\Google\\Chrome\\Application\\chrome.exe")
         self.logger.warning("Unable to find the browser binary.")
         return None
 
     def install(self, dest=None, channel=None):
         raise NotImplementedError
 
     def platform_string(self):
         platform = {
@@ -516,29 +535,29 @@ class Chrome(Browser):
         chromedriver_dir = os.path.join(dest, 'chromedriver_%s' % self.platform_string())
         if os.path.isfile(os.path.join(chromedriver_dir, "chromedriver")):
             shutil.move(os.path.join(chromedriver_dir, "chromedriver"), dest)
             shutil.rmtree(chromedriver_dir)
         return find_executable("chromedriver", dest)
 
     def version(self, binary=None, webdriver_binary=None):
         binary = binary or self.binary
-        if uname[0] != "Windows":
-            try:
-                version_string = call(binary, "--version").strip()
-            except subprocess.CalledProcessError:
-                self.logger.warning("Failed to call %s" % binary)
-                return None
-            m = re.match(r"(?:Google Chrome|Chromium) (.*)", version_string)
-            if not m:
-                self.logger.warning("Failed to extract version from: %s" % version_string)
-                return None
-            return m.group(1)
-        self.logger.warning("Unable to extract version from binary on Windows.")
-        return None
+        if uname[0] == "Windows":
+            return _get_fileversion(binary, self.logger)
+
+        try:
+            version_string = call(binary, "--version").strip()
+        except subprocess.CalledProcessError:
+            self.logger.warning("Failed to call %s" % binary)
+            return None
+        m = re.match(r"(?:Google Chrome|Chromium) (.*)", version_string)
+        if not m:
+            self.logger.warning("Failed to extract version from: %s" % version_string)
+            return None
+        return m.group(1)
 
 
 class ChromeAndroid(Browser):
     """Chrome-specific interface for Android.
 
     Includes webdriver installation.
     """
 
@@ -706,22 +725,17 @@ class EdgeChromium(Browser):
                 return None
             m = re.match(r"Microsoft Edge (.*) ", version_string)
             if not m:
                 self.logger.warning("Failed to extract version from: %s" % version_string)
                 return None
             return m.group(1)
         else:
             if binary is not None:
-                command = "(Get-Item '%s').VersionInfo.FileVersion" % binary
-                try:
-                    return call("powershell.exe", command).strip()
-                except (subprocess.CalledProcessError, OSError):
-                    self.logger.warning("Failed to call %s in PowerShell" % command)
-                    return None
+                return _get_fileversion(binary, self.logger)
             self.logger.warning("Failed to find Edge binary.")
             return None
 
 class Edge(Browser):
     """Edge-specific interface."""
 
     product = "edge"
     requirements = "requirements_edge.txt"
--- a/testing/web-platform/tests/tools/wpt/tests/test_wpt.py
+++ b/testing/web-platform/tests/tools/wpt/tests/test_wpt.py
@@ -89,32 +89,28 @@ def test_help():
     # TODO: It seems like there's a bug in argparse that makes this argument order required
     # should try to work around that
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["--help"])
     assert excinfo.value.code == 0
 
 
 @pytest.mark.slow
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="https://github.com/web-platform-tests/wpt/issues/12935")
 def test_list_tests(manifest_dir):
     """The `--list-tests` option should not produce an error under normal
     conditions."""
 
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["run", "--metadata", manifest_dir, "--list-tests",
                        "--channel", "dev", "--yes", "chrome",
                        "/dom/nodes/Element-tagName.html"])
     assert excinfo.value.code == 0
 
 
 @pytest.mark.slow
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="https://github.com/web-platform-tests/wpt/issues/12935")
 def test_list_tests_missing_manifest(manifest_dir):
     """The `--list-tests` option should not produce an error in the absence of
     a test manifest file."""
 
     os.remove(os.path.join(manifest_dir, "MANIFEST.json"))
 
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["run",
@@ -128,18 +124,16 @@ def test_list_tests_missing_manifest(man
                        "--list-tests",
                        "--yes",
                        "firefox", "/dom/nodes/Element-tagName.html"])
 
     assert excinfo.value.code == 0
 
 
 @pytest.mark.slow
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="https://github.com/web-platform-tests/wpt/issues/12935")
 def test_list_tests_invalid_manifest(manifest_dir):
     """The `--list-tests` option should not produce an error in the presence of
     a malformed test manifest file."""
 
     manifest_filename = os.path.join(manifest_dir, "MANIFEST.json")
 
     assert os.path.isfile(manifest_filename)
 
@@ -159,18 +153,16 @@ def test_list_tests_invalid_manifest(man
                        "--yes",
                        "firefox", "/dom/nodes/Element-tagName.html"])
 
     assert excinfo.value.code == 0
 
 
 @pytest.mark.slow
 @pytest.mark.remote_network
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="Tests currently don't work on Windows for path reasons")
 def test_run_zero_tests():
     """A test execution describing zero tests should be reported as an error
     even in the presence of the `--no-fail-on-unexpected` option."""
     if is_port_8000_in_use():
         pytest.skip("port 8000 already in use")
 
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["run", "--yes", "--no-pause", "--binary-arg", "headless",
@@ -181,18 +173,16 @@ def test_run_zero_tests():
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["run", "--yes", "--no-pause", "--binary-arg", "headless",
                        "--no-fail-on-unexpected", "--channel", "dev",
                        "chrome", "/non-existent-dir/non-existent-file.html"])
     assert excinfo.value.code != 0
 
 @pytest.mark.slow
 @pytest.mark.remote_network
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="Tests currently don't work on Windows for path reasons")
 def test_run_failing_test():
     """Failing tests should be reported with a non-zero exit status unless the
     `--no-fail-on-unexpected` option has been specified."""
     if is_port_8000_in_use():
         pytest.skip("port 8000 already in use")
     failing_test = "/infrastructure/expected-fail/failing-test.html"
 
     assert os.path.isfile("../../%s" % failing_test)
@@ -206,18 +196,16 @@ def test_run_failing_test():
         wpt.main(argv=["run", "--yes", "--no-pause", "--binary-arg", "headless",
                        "--no-fail-on-unexpected", "--channel", "dev",
                        "chrome", failing_test])
     assert excinfo.value.code == 0
 
 
 @pytest.mark.slow
 @pytest.mark.remote_network
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="Tests currently don't work on Windows for path reasons")
 def test_run_verify_unstable(temp_test):
     """Unstable tests should be reported with a non-zero exit status. Stable
     tests should be reported with a zero exit status."""
     if is_port_8000_in_use():
         pytest.skip("port 8000 already in use")
     unstable_test = temp_test("""
         test(function() {
             if (localStorage.getItem('wpt-unstable-test-flag')) {
@@ -238,82 +226,81 @@ def test_run_verify_unstable(temp_test):
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["run", "--yes", "--verify", "--binary-arg", "headless",
                        "--channel", "dev", "chrome", stable_test])
     assert excinfo.value.code == 0
 
 
 @pytest.mark.slow
 @pytest.mark.remote_network
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="Tests currently don't work on Windows for path reasons")
 def test_install_chromedriver():
-    chromedriver_path = os.path.join(wpt.localpaths.repo_root, "_venv", "bin", "chromedriver")
+    if sys.platform == "win32":
+        chromedriver_path = os.path.join(wpt.localpaths.repo_root, "_venv", "Scripts", "chromedriver.exe")
+    else:
+        chromedriver_path = os.path.join(wpt.localpaths.repo_root, "_venv", "bin", "chromedriver")
     if os.path.exists(chromedriver_path):
         os.unlink(chromedriver_path)
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["install", "chrome", "webdriver"])
     assert excinfo.value.code == 0
     assert os.path.exists(chromedriver_path)
     os.unlink(chromedriver_path)
 
 
 @pytest.mark.slow
 @pytest.mark.remote_network
 @pytest.mark.xfail(sys.platform == "win32",
-                   reason="Tests currently don't work on Windows for path reasons")
+                   reason="https://github.com/web-platform-tests/wpt/issues/17074")
 def test_install_firefox():
     if sys.platform == "darwin":
         fx_path = os.path.join(wpt.localpaths.repo_root, "_venv", "browsers", "nightly", "Firefox Nightly.app")
     else:
         fx_path = os.path.join(wpt.localpaths.repo_root, "_venv", "browsers", "nightly", "firefox")
     if os.path.exists(fx_path):
         shutil.rmtree(fx_path)
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["install", "firefox", "browser", "--channel=nightly"])
     assert excinfo.value.code == 0
     assert os.path.exists(fx_path)
     shutil.rmtree(fx_path)
 
 
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="Tests currently don't work on Windows for path reasons")
 def test_files_changed(capsys):
     commit = "9047ac1d9f51b1e9faa4f9fad9c47d109609ab09"
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["files-changed", "%s~..%s" % (commit, commit)])
     assert excinfo.value.code == 0
     out, err = capsys.readouterr()
-    assert out == """html/browsers/offline/appcache/workers/appcache-worker.html
+    expected = """html/browsers/offline/appcache/workers/appcache-worker.html
 html/browsers/offline/appcache/workers/resources/appcache-dedicated-worker-not-in-cache.js
 html/browsers/offline/appcache/workers/resources/appcache-shared-worker-not-in-cache.js
 html/browsers/offline/appcache/workers/resources/appcache-worker-data.py
 html/browsers/offline/appcache/workers/resources/appcache-worker-import.py
 html/browsers/offline/appcache/workers/resources/appcache-worker.manifest
 html/browsers/offline/appcache/workers/resources/appcache-worker.py
-"""
+""".replace("/", os.path.sep)
+    assert out == expected
     assert err == ""
 
 
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="Tests currently don't work on Windows for path reasons")
 def test_files_changed_null(capsys):
     commit = "9047ac1d9f51b1e9faa4f9fad9c47d109609ab09"
     with pytest.raises(SystemExit) as excinfo:
         wpt.main(argv=["files-changed", "--null", "%s~..%s" % (commit, commit)])
     assert excinfo.value.code == 0
     out, err = capsys.readouterr()
-    assert out == "\0".join(["html/browsers/offline/appcache/workers/appcache-worker.html",
+    expected = "\0".join(["html/browsers/offline/appcache/workers/appcache-worker.html",
         "html/browsers/offline/appcache/workers/resources/appcache-dedicated-worker-not-in-cache.js",
         "html/browsers/offline/appcache/workers/resources/appcache-shared-worker-not-in-cache.js",
         "html/browsers/offline/appcache/workers/resources/appcache-worker-data.py",
         "html/browsers/offline/appcache/workers/resources/appcache-worker-import.py",
         "html/browsers/offline/appcache/workers/resources/appcache-worker.manifest",
         "html/browsers/offline/appcache/workers/resources/appcache-worker.py",
-        ""])
+        ""]).replace("/", os.path.sep)
+    assert out == expected
     assert err == ""
 
 
 def test_files_changed_ignore():
     from tools.wpt.testfiles import exclude_ignored
     files = ["resources/testharness.js", "resources/webidl2/index.js", "test/test.js"]
     changed, ignored = exclude_ignored(files, ignore_rules=["resources/testharness*"])
     assert changed == [os.path.join(wpt.wpt_root, item) for item in
@@ -379,18 +366,18 @@ def test_tests_affected_null(capsys, man
     out, err = capsys.readouterr()
 
     tests = out.split("\0")
     assert "dom/interfaces.html" in tests
     assert "html/dom/interfaces.https.html" in tests
 
 
 @pytest.mark.slow
-@pytest.mark.xfail(sys.platform == "win32",
-                   reason="Tests currently don't work on Windows for path reasons")
+@pytest.mark.skipif(sys.platform == "win32",
+                    reason="no os.setsid/killpg to easily cleanup the process tree")
 def test_serve():
     if is_port_8000_in_use():
         pytest.skip("port 8000 already in use")
 
     p = subprocess.Popen([os.path.join(wpt.localpaths.repo_root, "wpt"), "serve"],
                          preexec_fn=os.setsid)
 
     start = time.time()