From 3d7b2b51a9f09a1c16b6e63471694b6706eab68e Mon Sep 17 00:00:00 2001 From: Robert Ma Date: Wed, 8 Jul 2020 10:05:53 -0400 Subject: [PATCH 1/5] Add nightly channel to Chrome --- tools/wpt/browser.py | 19 ++++++++++++------- tools/wpt/install.py | 6 +++--- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/tools/wpt/browser.py b/tools/wpt/browser.py index cb5974078a8022..6bf123a2c5ecf0 100644 --- a/tools/wpt/browser.py +++ b/tools/wpt/browser.py @@ -536,10 +536,13 @@ class Chrome(Browser): requirements = "requirements_chrome.txt" def download(self, dest=None, channel=None, rename=None): - raise NotImplementedError + if channel != "nightly": + raise NotImplementedError("We can only download Chrome Nightly (Chromium ToT) for you.") + url_base = self.latest_chromium_snapshot_url() def install(self, dest=None, channel=None): - raise NotImplementedError + if channel != "nightly": + raise NotImplementedError("We can only install Chrome Nightly (Chromium ToT) for you.") def platform_string(self): platform = { @@ -575,6 +578,12 @@ def chromium_platform_string(self): return platform + def latest_chromium_snapshot_url(self): + arch = self.chromium_platform_string() + revision_url = "https://storage.googleapis.com/chromium-browser-snapshots/%s/LAST_CHANGE" % arch + revision = get(revision_url).text.strip() + return "https://storage.googleapis.com/chromium-browser-snapshots/%s/%s/" % arch, revision + def find_binary(self, venv_path=None, channel=None): if uname[0] == "Linux": name = "google-chrome" @@ -626,11 +635,7 @@ def _chromium_chromedriver_url(self, chrome_version): get(url) except requests.RequestException: # Fall back to the tip-of-tree Chromium build. - revision_url = "https://storage.googleapis.com/chromium-browser-snapshots/%s/LAST_CHANGE" % ( - self.chromium_platform_string()) - revision = get(revision_url).text.strip() - url = "https://storage.googleapis.com/chromium-browser-snapshots/%s/%s/chromedriver_%s.zip" % ( - self.chromium_platform_string(), revision, self.platform_string()) + url = "%s/chromedriver_%s.zip" % self.latest_chromium_snapshot_url(), self.platform_string() return url def _latest_chromedriver_url(self, chrome_version): diff --git a/tools/wpt/install.py b/tools/wpt/install.py index 1fe091879b5ca0..81770225428d46 100644 --- a/tools/wpt/install.py +++ b/tools/wpt/install.py @@ -3,7 +3,7 @@ latest_channels = { 'firefox': 'nightly', - 'chrome': 'dev', + 'chrome': 'nightly', 'chrome_android': 'dev', 'edgechromium': 'dev', 'safari': 'preview', @@ -14,11 +14,11 @@ 'stable': 'stable', 'release': 'stable', 'beta': 'beta', + 'dev': 'dev', + 'canary': 'canary', 'nightly': latest_channels, - 'dev': latest_channels, 'preview': latest_channels, 'experimental': latest_channels, - 'canary': 'canary', } channel_args = argparse.ArgumentParser(add_help=False) From 87aa845d9c0dbc6c7fe231096644d137eb2733ac Mon Sep 17 00:00:00 2001 From: Robert Ma Date: Wed, 22 Jul 2020 18:44:25 -0400 Subject: [PATCH 2/5] Implement install{,_webdriver} for Chrome nightly --- tools/wpt/browser.py | 130 ++++++++++++++++++++++++++++--------------- 1 file changed, 84 insertions(+), 46 deletions(-) diff --git a/tools/wpt/browser.py b/tools/wpt/browser.py index 6bf123a2c5ecf0..b083e0bef65d0a 100644 --- a/tools/wpt/browser.py +++ b/tools/wpt/browser.py @@ -63,6 +63,18 @@ class Browser(object): def __init__(self, logger): self.logger = logger + def _get_dest(self, dest, channel): + if dest is None: + # os.getcwd() doesn't include the venv path + dest = os.path.join(os.getcwd(), "_venv") + + dest = os.path.join(dest, "browsers", channel) + + if not os.path.exists(dest): + os.makedirs(dest) + + return dest + @abstractmethod def download(self, dest=None, channel=None, rename=None): """Download a package or installer for the browser @@ -156,18 +168,6 @@ def platform_string_geckodriver(self): return "%s%s" % (self.platform, bits) - def _get_dest(self, dest, channel): - if dest is None: - # os.getcwd() doesn't include the venv path - dest = os.path.join(os.getcwd(), "_venv") - - dest = os.path.join(dest, "browsers", channel) - - if not os.path.exists(dest): - os.makedirs(dest) - - return dest - def download(self, dest=None, channel="nightly", rename=None): product = { "nightly": "firefox-nightly-latest-ssl", @@ -534,25 +534,48 @@ class Chrome(Browser): product = "chrome" requirements = "requirements_chrome.txt" + platforms = { + "Linux": "Linux", + "Windows": "Win", + "Darwin": "Mac", + } + + def __init__(self, logger): + super(Chrome, self).__init__(logger) + self._last_change = None def download(self, dest=None, channel=None, rename=None): if channel != "nightly": raise NotImplementedError("We can only download Chrome Nightly (Chromium ToT) for you.") - url_base = self.latest_chromium_snapshot_url() + if dest is None: + dest = self._get_dest(None, channel) + + filename = self._chromium_package_name() + ".zip" + url = self._latest_chromium_snapshot_url() + filename + self.logger.info("Downloading Chrome from %s" % url) + resp = get(url) + installer_path = os.path.join(dest, filename) + with open(installer_path, "wb") as f: + f.write(resp.content) + return installer_path def install(self, dest=None, channel=None): if channel != "nightly": raise NotImplementedError("We can only install Chrome Nightly (Chromium ToT) for you.") + dest = self._get_dest(dest, channel) - def platform_string(self): - platform = { - "Linux": "linux", - "Windows": "win", - "Darwin": "mac" - }.get(uname[0]) + installer_path = self.download(dest, channel) + with open(installer_path, "rb") as f: + unzip(f, dest) + os.remove(installer_path) + return self.find_nightly_binary(dest, channel) + + def _chromedriver_platform_string(self): + platform = self.platforms.get(uname[0]) if platform is None: raise ValueError("Unable to construct a valid Chrome package name for current platform") + platform = platform.lower() if platform == "linux": bits = "64" if uname[4] == "x86_64" else "32" @@ -563,12 +586,8 @@ def platform_string(self): return "%s%s" % (platform, bits) - def chromium_platform_string(self): - platform = { - "Linux": "Linux", - "Windows": "Win", - "Darwin": "Mac" - }.get(uname[0]) + def _chromium_platform_string(self): + platform = self.platforms.get(uname[0]) if platform is None: raise ValueError("Unable to construct a valid Chromium package name for current platform") @@ -578,13 +597,26 @@ def chromium_platform_string(self): return platform - def latest_chromium_snapshot_url(self): - arch = self.chromium_platform_string() - revision_url = "https://storage.googleapis.com/chromium-browser-snapshots/%s/LAST_CHANGE" % arch - revision = get(revision_url).text.strip() - return "https://storage.googleapis.com/chromium-browser-snapshots/%s/%s/" % arch, revision + def _chromium_package_name(self): + return "chrome-%s" % self.platforms.get(uname[0]).lower() + + def _latest_chromium_snapshot_url(self): + # Make sure we use the same revision in an invocation. + if self._last_change is None: + arch = self._chromium_platform_string() + revision_url = "https://storage.googleapis.com/chromium-browser-snapshots/%s/LAST_CHANGE" % arch + self._last_change = get(revision_url).text.strip() + return "https://storage.googleapis.com/chromium-browser-snapshots/%s/%s/" % (arch, self._last_change) + + def find_nightly_binary(self, dest, channel): + binary = "Chromium" if uname[0] == "Darwin" else "chrome" + # find_executable will add .exe on Windows automatically. + return find_executable(binary, os.path.join(dest, self._chromium_package_name())) def find_binary(self, venv_path=None, channel=None): + if channel == "nightly": + return self.find_nightly_binary(self._get_dest(venv_path, channel)) + if uname[0] == "Linux": name = "google-chrome" if channel == "stable": @@ -622,21 +654,23 @@ def _official_chromedriver_url(self, chrome_version): except requests.RequestException: return None return "https://chromedriver.storage.googleapis.com/%s/chromedriver_%s.zip" % ( - latest, self.platform_string()) + latest, self._chromedriver_platform_string()) def _chromium_chromedriver_url(self, chrome_version): - try: - # Try to find the Chromium build with the same revision. - omaha = get("https://omahaproxy.appspot.com/deps.json?version=" + chrome_version).json() - revision = omaha['chromium_base_position'] - url = "https://storage.googleapis.com/chromium-browser-snapshots/%s/%s/chromedriver_%s.zip" % ( - self.chromium_platform_string(), revision, self.platform_string()) - # Check the status without downloading the content (this is a streaming request). - get(url) - except requests.RequestException: - # Fall back to the tip-of-tree Chromium build. - url = "%s/chromedriver_%s.zip" % self.latest_chromium_snapshot_url(), self.platform_string() - return url + if chrome_version: + try: + # Try to find the Chromium build with the same revision. + omaha = get("https://omahaproxy.appspot.com/deps.json?version=" + chrome_version).json() + revision = omaha['chromium_base_position'] + url = "https://storage.googleapis.com/chromium-browser-snapshots/%s/%s/chromedriver_%s.zip" % ( + self._chromium_platform_string(), revision, self._chromedriver_platform_string()) + # Check the status without downloading the content (this is a streaming request). + get(url) + return url + except requests.RequestException: + pass + # Fall back to the tip-of-tree Chromium build. + return "%schromedriver_%s.zip" % (self._latest_chromium_snapshot_url(), self._chromedriver_platform_string()) def _latest_chromedriver_url(self, chrome_version): # Remove channel suffixes (e.g. " dev"). @@ -645,20 +679,24 @@ def _latest_chromedriver_url(self, chrome_version): self._chromium_chromedriver_url(chrome_version)) def install_webdriver_by_version(self, version, dest=None): - assert version, "Cannot install ChromeDriver without Chrome version" if dest is None: dest = os.pwd - url = self._latest_chromedriver_url(version) + url = self._latest_chromedriver_url(version) if version \ + else self._chromium_chromedriver_url(None) self.logger.info("Downloading ChromeDriver from %s" % url) unzip(get(url).raw, dest) chromedriver_dir = os.path.join( - dest, 'chromedriver_%s' % self.platform_string()) + dest, 'chromedriver_%s' % self._chromedriver_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 install_webdriver(self, dest=None, channel=None, browser_binary=None): + if channel == "nightly": + # The "nightly" channel is not an official channel, so we simply download ToT. + return self.install_webdriver_by_version(None, dest) + if browser_binary is None: browser_binary = self.find_binary(channel) return self.install_webdriver_by_version( From 17b4e6665e86170ab255b9524b41ca2143a3691c Mon Sep 17 00:00:00 2001 From: Robert Ma Date: Thu, 23 Jul 2020 16:04:08 -0400 Subject: [PATCH 3/5] Support Chrome nightly in "wpt run" --- tools/wpt/run.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/wpt/run.py b/tools/wpt/run.py index 32f903ce9bd644..79329a5dab4472 100644 --- a/tools/wpt/run.py +++ b/tools/wpt/run.py @@ -343,8 +343,8 @@ def setup_kwargs(self, kwargs): kwargs["webdriver_binary"] = webdriver_binary else: raise WptrunError("Unable to locate or install chromedriver binary") - if browser_channel in ("dev", "canary"): - logger.info("Automatically turning on experimental features for Chrome Dev/Canary") + if browser_channel in ("dev", "canary", "nightly"): + logger.info("Automatically turning on experimental features for Chrome Dev/Canary or Chromium trunk") kwargs["binary_args"].append("--enable-experimental-web-platform-features") # HACK(Hexcles): work around https://github.com/web-platform-tests/wpt/issues/16448 kwargs["webdriver_args"].append("--disable-build-check") From f6dda5c74486f926e74da559d568abc44155708f Mon Sep 17 00:00:00 2001 From: Robert Ma Date: Wed, 22 Jul 2020 22:01:51 -0400 Subject: [PATCH 4/5] Add and fix some tests Add a test for installing Chrome nightly. In addition, create a safer version of shutil.rmtree based on existing implementation in browser.py and reuse it in more places in place of shutil.rmtree. --- tools/wpt/browser.py | 27 +++++++++----------------- tools/wpt/tests/test_wpt.py | 38 +++++++++++++++++++++++++++++++------ tools/wpt/utils.py | 19 +++++++++++++++++++ 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/tools/wpt/browser.py b/tools/wpt/browser.py index b083e0bef65d0a..c13a47b072865e 100644 --- a/tools/wpt/browser.py +++ b/tools/wpt/browser.py @@ -3,7 +3,6 @@ import re import shutil import stat -import errno import subprocess import tempfile from abc import ABCMeta, abstractmethod @@ -13,7 +12,7 @@ from six.moves.urllib.parse import urlsplit import requests -from .utils import call, get, untar, unzip +from .utils import call, get, untar, unzip, rmtree uname = platform.uname() @@ -31,15 +30,6 @@ def _get_fileversion(binary, logger=None): return None -def handle_remove_readonly(func, path, exc): - excvalue = exc[1] - if func in (os.rmdir, os.remove) and excvalue.errno == errno.EACCES: - os.chmod(path, stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO) # 0777 - func(path) - else: - raise - - def get_ext(filename): """Get the extension from a filename with special handling for .tar.foo""" name, ext = os.path.splitext(filename) @@ -374,7 +364,7 @@ def install_prefs(self, binary, dest=None, channel=None): # If we don't have a recent download, grab and extract the latest one if not have_cache: if os.path.exists(dest): - shutil.rmtree(dest) + rmtree(dest) os.makedirs(dest) url = self.get_profile_bundle_url(version, channel) @@ -389,7 +379,7 @@ def install_prefs(self, binary, dest=None, channel=None): path = os.path.join(profiles, name) shutil.move(path, dest) finally: - shutil.rmtree(extract_dir) + rmtree(extract_dir) else: self.logger.info("Using cached test prefs from %s" % dest) @@ -687,9 +677,10 @@ def install_webdriver_by_version(self, version, dest=None): unzip(get(url).raw, dest) chromedriver_dir = os.path.join( dest, 'chromedriver_%s' % self._chromedriver_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) + unzipped_path = find_executable("chromedriver", chromedriver_dir) + assert unzipped_path is not None + shutil.move(unzipped_path, dest) + rmtree(chromedriver_dir) return find_executable("chromedriver", dest) def install_webdriver(self, dest=None, channel=None, browser_binary=None): @@ -921,7 +912,7 @@ def install_webdriver(self, dest=None, channel=None, browser_binary=None): operadriver_dir = os.path.join(dest, "operadriver_%s" % self.platform_string()) shutil.move(os.path.join(operadriver_dir, "operadriver"), dest) - shutil.rmtree(operadriver_dir) + rmtree(operadriver_dir) path = find_executable("operadriver") st = os.stat(path) @@ -1018,7 +1009,7 @@ def install_webdriver(self, dest=None, channel=None, browser_binary=None): driver_notes_path = os.path.join(dest, "Driver_notes") if os.path.isdir(driver_notes_path): print("Delete %s folder" % driver_notes_path) - shutil.rmtree(driver_notes_path, ignore_errors=False, onerror=handle_remove_readonly) + rmtree(driver_notes_path) self.logger.info("Downloading MSEdgeDriver from %s" % url) unzip(get(url).raw, dest) diff --git a/tools/wpt/tests/test_wpt.py b/tools/wpt/tests/test_wpt.py index f897b688ad6f68..71ffc0c51f9dbc 100644 --- a/tools/wpt/tests/test_wpt.py +++ b/tools/wpt/tests/test_wpt.py @@ -16,7 +16,7 @@ import pytest -from tools.wpt import wpt +from tools.wpt import wpt, utils here = os.path.abspath(os.path.dirname(__file__)) @@ -61,7 +61,7 @@ def manifest_dir(): os.path.join(path, "MANIFEST.json")) yield path finally: - shutil.rmtree(path) + utils.rmtree(path) @pytest.fixture @@ -86,7 +86,7 @@ def make_test(body): yield make_test - shutil.rmtree("../../.tools-tests") + utils.rmtree("../../.tools-tests") def test_missing(): @@ -185,6 +185,7 @@ def test_run_zero_tests(): "chrome", "/non-existent-dir/non-existent-file.html"]) assert excinfo.value.code != 0 + @pytest.mark.slow @pytest.mark.remote_network def test_run_failing_test(): @@ -238,6 +239,25 @@ def test_run_verify_unstable(temp_test): assert excinfo.value.code == 0 +@pytest.mark.slow +@pytest.mark.remote_network +def test_install_chromium(): + if sys.platform == "win32": + chromium_path = os.path.join(wpt.localpaths.repo_root, wpt.venv_dir(), "browsers", "nightly", "chrome-win") + elif sys.platform == "darwin": + chromium_path = os.path.join(wpt.localpaths.repo_root, wpt.venv_dir(), "browsers", "nightly", "chrome-mac") + else: + chromium_path = os.path.join(wpt.localpaths.repo_root, wpt.venv_dir(), "browsers", "nightly", "chrome-linux") + + if os.path.exists(chromium_path): + utils.rmtree(chromium_path) + with pytest.raises(SystemExit) as excinfo: + wpt.main(argv=["install", "chrome", "browser", "--channel=nightly"]) + assert excinfo.value.code == 0 + assert os.path.exists(chromium_path) + utils.rmtree(chromium_path) + + @pytest.mark.slow @pytest.mark.remote_network def test_install_chromedriver(): @@ -251,7 +271,13 @@ def test_install_chromedriver(): wpt.main(argv=["install", "chrome", "webdriver"]) assert excinfo.value.code == 0 assert os.path.exists(chromedriver_path) - os.unlink(chromedriver_path) + # FIXME: On Windows, this may sometimes fail (access denied), possibly + # because the file handler is not released immediately. + try: + os.unlink(chromedriver_path) + except OSError: + if sys.platform != "win32": + raise @pytest.mark.slow @@ -264,12 +290,12 @@ def test_install_firefox(): else: fx_path = os.path.join(wpt.localpaths.repo_root, wpt.venv_dir(), "browsers", "nightly", "firefox") if os.path.exists(fx_path): - shutil.rmtree(fx_path) + utils.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) + utils.rmtree(fx_path) def test_files_changed(capsys): diff --git a/tools/wpt/utils.py b/tools/wpt/utils.py index 18551481ab8cae..6608af754b2ada 100644 --- a/tools/wpt/utils.py +++ b/tools/wpt/utils.py @@ -1,5 +1,8 @@ +import errno import logging import os +import shutil +import stat import subprocess import tarfile import zipfile @@ -95,3 +98,19 @@ def get(url): resp = requests.get(url, stream=True) resp.raise_for_status() return resp + + +def rmtree(path): + # This works around two issues: + # 1. Cannot delete read-only files owned by us (e.g. files extracted from tarballs) + # 2. On Windows, we sometimes just need to retry in case the file handler + # hasn't been fully released (a common issue). + def handle_remove_readonly(func, path, exc): + excvalue = exc[1] + if func in (os.rmdir, os.remove) and excvalue.errno == errno.EACCES: + os.chmod(path, stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO) # 0777 + func(path) + else: + raise + + return shutil.rmtree(path, onerror=handle_remove_readonly) From 06dd93de5c0a6c11f6320329550219b6faedeb5f Mon Sep 17 00:00:00 2001 From: Robert Ma Date: Fri, 24 Jul 2020 10:50:29 -0400 Subject: [PATCH 5/5] Apply suggestions from code review --- tools/wpt/browser.py | 8 ++++---- tools/wpt/tests/test_wpt.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/wpt/browser.py b/tools/wpt/browser.py index c13a47b072865e..85b7b63c219726 100644 --- a/tools/wpt/browser.py +++ b/tools/wpt/browser.py @@ -12,7 +12,7 @@ from six.moves.urllib.parse import urlsplit import requests -from .utils import call, get, untar, unzip, rmtree +from .utils import call, get, rmtree, untar, unzip uname = platform.uname() @@ -592,11 +592,11 @@ def _chromium_package_name(self): def _latest_chromium_snapshot_url(self): # Make sure we use the same revision in an invocation. + architecture = self._chromium_platform_string() if self._last_change is None: - arch = self._chromium_platform_string() - revision_url = "https://storage.googleapis.com/chromium-browser-snapshots/%s/LAST_CHANGE" % arch + revision_url = "https://storage.googleapis.com/chromium-browser-snapshots/%s/LAST_CHANGE" % architecture self._last_change = get(revision_url).text.strip() - return "https://storage.googleapis.com/chromium-browser-snapshots/%s/%s/" % (arch, self._last_change) + return "https://storage.googleapis.com/chromium-browser-snapshots/%s/%s/" % (architecture, self._last_change) def find_nightly_binary(self, dest, channel): binary = "Chromium" if uname[0] == "Darwin" else "chrome" diff --git a/tools/wpt/tests/test_wpt.py b/tools/wpt/tests/test_wpt.py index 71ffc0c51f9dbc..d2d0b541a37965 100644 --- a/tools/wpt/tests/test_wpt.py +++ b/tools/wpt/tests/test_wpt.py @@ -16,7 +16,7 @@ import pytest -from tools.wpt import wpt, utils +from tools.wpt import utils, wpt here = os.path.abspath(os.path.dirname(__file__))