Skip to content

Commit 872bed3

Browse files
committed
fix(chrome): sort Playwright Chromium by revision, not lexically
_candidate_chrome_paths sorted Playwright's chromium-* matches as strings (reverse=True), so once a revision crosses the 3->4 digit boundary "chromium-999" ranks above "chromium-1187" and find_chrome() returns an older browser than intended. Sort by the integer revision instead.
1 parent 096adf5 commit 872bed3

2 files changed

Lines changed: 44 additions & 2 deletions

File tree

render/src/pixelrag_render/chrome.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import json
1717
import os
1818
import platform
19+
import re
1920
import subprocess
2021
import sys
2122
import tarfile
@@ -34,6 +35,17 @@
3435
)
3536

3637

38+
def _playwright_revision(path: str) -> int:
39+
"""Integer Playwright Chromium revision embedded in ``path``.
40+
41+
Playwright caches browsers under ``chromium-<revision>`` with a plain
42+
monotonically-increasing integer revision. Paths without a recognizable
43+
revision sort last (``-1``) rather than raising.
44+
"""
45+
m = re.search(r"chromium-(\d+)", path)
46+
return int(m.group(1)) if m else -1
47+
48+
3749
def _candidate_chrome_paths(system: str | None = None) -> list[str]:
3850
"""Ordered Chrome binary candidates for the given OS (default: this OS).
3951
@@ -55,8 +67,11 @@ def _candidate_chrome_paths(system: str | None = None) -> list[str]:
5567
paths.append(str(INSTALL_DIR / "headless_shell"))
5668

5769
def add_playwright(cache_dir: Path, rel_glob: str) -> None:
58-
# Newest chromium-NNNN first.
59-
paths.extend(sorted(glob.glob(str(cache_dir / rel_glob)), reverse=True))
70+
# Newest chromium-NNNN first. Sort by integer revision, not lexically:
71+
# a string sort ranks "chromium-999" above "chromium-1187" once the
72+
# revision crosses the 3→4 digit boundary, picking an older browser.
73+
matches = glob.glob(str(cache_dir / rel_glob))
74+
paths.extend(sorted(matches, key=_playwright_revision, reverse=True))
6075

6176
if system == "Darwin":
6277
add_playwright(

tests/test_chrome_paths.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,30 @@ def test_find_chrome_returns_executable():
3131
# On any supported dev box this resolves to an installed/usable binary.
3232
path = find_chrome()
3333
assert os.path.isfile(path) and os.access(path, os.X_OK)
34+
35+
36+
def test_playwright_chromium_sorted_by_revision_not_lexically(monkeypatch):
37+
"""Newer Playwright Chromium revisions must rank ahead of older ones.
38+
39+
Playwright caches browsers under ``chromium-<revision>`` with a plain
40+
integer revision. A lexicographic sort ranks ``chromium-999`` above
41+
``chromium-1187`` once revisions cross the 3→4 digit boundary, so the
42+
resolver would pick an *older* browser. Sorting by integer revision fixes
43+
that — assert the candidate list is newest-first regardless of digit count.
44+
"""
45+
import glob
46+
47+
found = [
48+
"/home/u/.cache/ms-playwright/chromium-999/chrome-linux/chrome",
49+
"/home/u/.cache/ms-playwright/chromium-1208/chrome-linux/chrome",
50+
"/home/u/.cache/ms-playwright/chromium-1187/chrome-linux/chrome",
51+
]
52+
monkeypatch.setattr(glob, "glob", lambda pattern: list(found))
53+
54+
paths = _candidate_chrome_paths("Linux")
55+
revisions = [p for p in paths if "ms-playwright" in p]
56+
assert revisions == [
57+
"/home/u/.cache/ms-playwright/chromium-1208/chrome-linux/chrome",
58+
"/home/u/.cache/ms-playwright/chromium-1187/chrome-linux/chrome",
59+
"/home/u/.cache/ms-playwright/chromium-999/chrome-linux/chrome",
60+
]

0 commit comments

Comments
 (0)