Key browser downloads by the installed Playwright version - #75
Merged
Merged
Conversation
comp615
marked this pull request as ready for review
September 15, 2026 14:45
Co-authored-by: Amp <amp@ampcode.com> Signed-off-by: Charlie Croom <ccroom@squareup.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a0a149-b278-7550-acc4-356168b0c4b5
comp615
force-pushed
the
playwright-browser-cache
branch
from
September 15, 2026 15:12
c34f868 to
0f91afa
Compare
wesbillman
reviewed
Sep 16, 2026
wesbillman
left a comment
Collaborator
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blocking findings
Reviewed 0f91afa71bff50d6a1bd9fdb2f1f096fc3428e01 against d21f43364fe9b7bdf2643779dab819fd9e8c16bb.
- The installed-version output is resolved after the frozen dependency install, and the browser cache key includes OS, architecture, Playwright version and the actual Chromium/WebKit engine set. The exact 1.60.0 dependency chain is unchanged. Missing package resolution fails setup; browser/system-library installation still runs after either a hit or a miss. All current action callers and the unchanged browser selection/required gate were traced.
- Final hosted run 34986925873 succeeded, including all four browser shards, five measurements and the full 39-test Node integration suite. Its checkout merge commit has the identical Git tree to the reviewed head. The actual measurement job resolved and restored
playwright-Linux-X64-1.60.0-chromium-webkit, then successfully ranpnpm exec playwright install --with-deps chromium webkit. Version resolution took 135 ms, cache restoration 4.922 s and the installer 40.437 s. This verifies warm reuse and retained system-dependency installation, not an isolated speedup. - The new regression exercises the action’s real shell with two installed versions, unrelated lockfile edits and a missing installation. Independent source review found no blocking defect. No local suite, install, browser launch or native acceptance run was repeated. The PR body’s “still running” and “warm-cache reuse is not yet measured” notes are stale relative to this successful run; that is non-blocking documentation cleanup. The earlier cancelled duplicate is not the final CI result.
wesbillman
approved these changes
Sep 16, 2026
delkc
added a commit
that referenced
this pull request
Sep 16, 2026
* origin/main: Reduce navigation fixture cost and stabilize typing thread opening (#68) Key browser downloads by the installed Playwright version (#75) Expose CI test counts and execution costs in job summaries (#66) Right-size notification fixtures and stabilize thread-navigation coverage (#69) Apply shared design foundations throughout the Buzz app (#59) Generate only the history needed by navigation and plugin journeys (#65) Move stale completion permutations into mounted React tests (#64) Signed-off-by: Clay Delk <clay.delk@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
September 15 — independent PR after #63 merge
🤖 Rebased onto main after #63 merged. No sibling branch is required. Verified the exact resulting Git tree against the original changes combined with the merged parent; no new behavior changes, missing assertions, or unrelated files. Every remaining commit retains its DCO sign-off, and hosted DCO passed at the rewritten head.
All eight targeted cache/topology/fixture Node checks passed on the rebased tree. The unchanged push hook deferred infrastructure inputs to CI. The previous run failed only the navigation-thread-history Beta selector in WebKit; that test fix remains in #69, not this cache-only PR. Fresh CI will establish the rebased result; no new speedup claim.
Fresh CI run is still running. Superseded duplicate runs were cancelled; their required-gate failures are not the final result. Timings and older validation below describe their linked historical snapshots, not new post-rebase measurements. Current review status is ready for review, not a claim that current CI is green.
Behavior
Keep the existing exact Playwright 1.60.0 dependency pin. Resolve the installed version after the frozen pnpm install, then key Chromium/WebKit downloads by OS, architecture, installed Playwright version and engine selection. Unrelated lockfile changes no longer invalidate identical browser binaries. Missing Playwright fails setup rather than emitting an empty cache version.
Continue running
playwright install --with-deps chromium webkiton hits and misses. No dependency upgrade, restore-key fallback, test selection, worker count, retry, budget or product changes. "Independent of the remaining PRs; #63 is now in main."Verification
"- Hosted run verified version resolution, the exact version-based cache key, cold installation and successful cache publication. Warm-cache reuse is not yet measured. This is a setup-cache correction, not a demonstrated browser-test runtime improvement."
Timing and limitations
The earlier paired experiment showed an exact browser cache hit on the baseline and a miss on the RTL branch because the full lockfile hash changed: baseline, combined. Those were different test snapshots, not an isolated cache benchmark. No wall-time savings are claimed yet; this new key will initially be cold.
Playwright guidance recommends version-based keys if caching, but notes restoration may cost as much as downloading and Linux system dependencies still require installation. Retaining versus removing caching is a separate measurement decision.
Local main verification reproduced the existing WebKit emoji bounds failure unchanged (last image bottom 1103.453125 versus editor bottom 1103.21875). This PR does not alter or suppress that assertion.
🤖 Prepared with Amp for Charlie Croom.
Hosted verification — September 15
🤖 Run 34983590669 used
playwright-Linux-X64-1.60.0-chromium-webkit, installed the browsers on the initial miss and saved the cache. The measurement job setup took 53 seconds cold; all five measurement cases passed. No warm-cache speedup is claimed.All CI lanes except WebKit shard 2 and its required gate passed. The sole browser failure was the navigation-thread-history exact-name
Betaselector: the rendered channel included its unread badge. That selector defect is addressed by still-unmerged #69; this cache-only PR does not duplicate its changes or relax the assertion. DCO passed.