Apply suggestions from @foolip code review

.github/workflows/update_chromium_revision.yml

@@ -1,6 +1,6 @@
 name: update_chromium_revision
 on:
-  # Trigger everyday at 10 UTC.
+  # Trigger every day at 10 UTC, or manually.
   schedule:
    - cron: 0 10 * * *
   workflow_dispatch:
@@ -10,14 +10,15 @@ jobs:
     steps:
     - name: Checkout
       uses: actions/checkout@v3
-    - name: setup python
+    - name: Set up Python
       uses: actions/setup-python@v3
       with:
         python-version: '3.10'
         cache: 'pip'
-    - run:  pip install requests
+    - name: Install dependencies
+      run: pip install requests
     - name: execute update_chromium_revision.py
-      run: python -m tools.wpt.update_chromium_revision
+      run: python tools/wpt/update_chromium_revision.py
     - name: Create pull request
       # Use a conditional step instead of a conditional job to work around #20700.
       if: github.repository == 'web-platform-tests/wpt'
@@ -26,7 +27,7 @@ jobs:
         token: ${{ secrets.GITHUB_TOKEN }}
         author: wpt-pr-bot <[email protected]>
         title: "Update latest available Chromium revisions"
-        commit-message: "Latest Chromium revision for Linux/Mac/Win"
+        commit-message: "Latest Chromium revision for Linux/macOS/Windows"
         body: |
           This automated pull request updates `tools/wpt/latest_chromium_revisions.json` with the current latest available Chromium revisions for major platforms.
 

tools/ci/run_tc.py

@@ -46,7 +46,7 @@
 import zipfile
 
 sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), '..')))
-from wpt.utils import get, get_download_to_descriptor, unzip  # type: ignore
+from wpt.utils import get_download_to_descriptor  # type: ignore
 
 root = os.path.abspath(
     os.path.join(os.path.dirname(__file__),
@@ -257,33 +257,13 @@ def setup_environment(args):
         assert args.channel is not None
         install_chrome(args.channel)
 
-    if "chromium" in args.browser:
-        install_chromium()
-
     if args.xvfb:
         start_xvfb()
 
     if args.oom_killer:
         start_userspace_oom_killer()
 
 
-def install_chromium():
-    revisions_path = os.path.join(os.getcwd(), "tools", "wpt", "latest_chromium_revision.txt")
-    with open(revisions_path) as f:
-        revision = f.read().strip()
-        dest = os.path.join("/tmp")
-        installer_path = os.path.join("/tmp", "chrome-linux.zip")
-
-        url = ("https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/"
-               f"{revision}/chrome-linux.zip")
-        resp = get(url)
-        with open(installer_path, "wb") as f:
-            f.write(resp.content)
-        with open(installer_path, "rb") as f:
-            unzip(f, dest)
-        os.remove(installer_path)
-
-
 def setup_repository(args):
     is_pr = os.environ.get("GITHUB_PULL_REQUEST", "false") != "false"
 

tools/wpt/browser.py

@@ -529,8 +529,8 @@ class ChromeChromiumBase(Browser):
         "Darwin": "Mac",
     }.get(uname[0])
 
-    def _get_latest_chromium_revision(self):
-        """Returns the latest Chromium revision number for the current platform."""
+    def _get_pinned_chromium_revision(self) -> str:
+        """Returns the pinned Chromium revision number."""
         path = os.path.join(os.path.dirname(os.path.abspath(__file__)),
                             "latest_chromium_revision.txt")
         with open(path) as f:
@@ -556,7 +556,7 @@ def _get_chromium_download_url(self, version=None):
                                         f"based on version. {url}")
         # If no URL was used in a previous install
         # and no version was passed, use the latest Chromium revision.
-        revision = self._get_latest_chromium_revision()
+        revision = self._get_pinned_chromium_revision()
 
         # If the url is successfully used to download/install, it will be used again
         # if another component is also installed during this run (browser/webdriver).

tools/wpt/latest_chromium_revision.txt

@@ -1 +1 @@
-996554
+998197

tools/wpt/update_chromium_revision.py

@@ -1,8 +1,6 @@
 import requests
 from time import time
 
-from .utils import get
-
 
 PLATFORM_INFO = [
     ("Win_x64", "chrome-win.zip"),
@@ -13,76 +11,74 @@
 SNAPSHOTS_PATH = "https://storage.googleapis.com/chromium-browser-snapshots/"
 
 
-def main(timeout=600.0):
+def main(timeout: float = 600.0) -> None:
     start = time()
 
     # Load existing pinned revision.
-    existing_revision = None
+    start_revision = None
     with open("tools/wpt/latest_chromium_revision.txt", "r") as f:
         existing_revision = int(f.read())
 
-    # Find the lowest new revision number among latest revisions by platform.
-    # We need to find a revision number that is available for download for all platforms,
-    # so we start looking from the smallest of these latest revisions.
-    smallest_revision = existing_revision
-    for platform, filename in PLATFORM_INFO:
-        try:
-            url = f"{SNAPSHOTS_PATH}{platform}/LAST_CHANGE"
-            revision = get(url).text.strip()
-            smallest_revision = max(smallest_revision, int(revision))
-        except requests.RequestException as e:
-            print(f"failed LAST_CHANGE lookup for {platform}: {e}")
-            continue
+    # Get the latest revision for Linux as a starting point to check for
+    # a valid revision for all platforms.
+    try:
+        url = f"{SNAPSHOTS_PATH}Linux_x64/LAST_CHANGE"
+        start_revision = int(requests.get(url).text.strip())
+    except requests.RequestException as e:
+        raise requests.RequestException(f"Failed LAST_CHANGE lookup: {e}")
 
-    if smallest_revision == existing_revision:
+    if start_revision == existing_revision:
         return
 
     # Step backwards through revision numbers until we find one
     # that is available for all platforms.
-    largest_mutually_available_revision = smallest_revision
-    available_for_all = False
+    candidate_revision = start_revision
+    new_revision = -1
     timed_out = False
-    while not available_for_all and not timed_out:
+    while new_revision == -1 and candidate_revision > existing_revision:
         available_for_all = True
         # For each platform, check if Chromium is available for download from snapshots.
         for platform, filename in PLATFORM_INFO:
             try:
                 url = (f"{SNAPSHOTS_PATH}{platform}/"
-                       f"{largest_mutually_available_revision}/{filename}")
+                       f"{candidate_revision}/{filename}")
                 # Check the headers of each possible download URL.
                 r = requests.head(url)
-                # If the "Accept-Ranges" header is not present, we know the file is not
-                # available for download. Decrement the revision number and try again.
-                if "Accept-Ranges" not in r.headers:
-                    largest_mutually_available_revision -= 1
+                # If the file is not available for download, decrement the revision and try again.
+                if r.status_code != 200:
+                    candidate_revision -= 1
                     available_for_all = False
                     break
-            except requests.RequestException as e:
-                print(e)
-                largest_mutually_available_revision -= 1
+            except requests.RequestException:
+                print(f"Failed to fetch headers for revision {candidate_revision}. Skipping it.")
+                candidate_revision -= 1
                 available_for_all = False
                 break
+
+        if available_for_all:
+            new_revision = candidate_revision
         if time() - start > timeout:
             timed_out = True
+            break
 
     if timed_out:
         raise TimeoutError(f"Reached timeout {timeout}s while checking revision "
-                           f"{largest_mutually_available_revision}")
+                           f"{candidate_revision}")
 
     end = time()
-    if largest_mutually_available_revision == existing_revision:
-        print(f"No new mutually available revision numbers found after "
-              f"{'{:.2f}'.format(end - start)} seconds.")
+    if new_revision <= existing_revision:
+        print(f"No new mutually available revision found after "
+              f"{'{:.2f}'.format(end - start)} seconds. Keeping revision {existing_revision}.")
         return
 
-    print(f"Found mutually available revision at {largest_mutually_available_revision}.")
-    print(f"This process started at {smallest_revision} and checked "
-          f"{smallest_revision - largest_mutually_available_revision} revisions.")
+    print(f"Found mutually available revision at {new_revision}.")
+    print(f"This process started at {start_revision} and checked "
+          f"{start_revision - new_revision} revisions.")
     print(f"The whole process took {'{:.2f}'.format(end - start)} seconds.")
 
     with open("tools/wpt/latest_chromium_revision.txt", "w") as f:
-        f.write(f"{largest_mutually_available_revision}\n")
+        f.write(f"{new_revision}\n")
 
 
 if __name__ == "__main__":
-    main()  # type: ignore
+    main()