From a5c92910d02645089ab46a0abacae67a8fd526a0 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 20 Aug 2026 02:10:09 +0200 Subject: [PATCH] ci(frontend-e2e): fail the job when Chromium install wedges, and cache it `npx playwright install --with-deps chromium` wedged five times on 2026-08-19: three attempts on #1864 and two on #1868. Each ran until the job's 10-minute cap killed it, so the run ended `cancelled` with every spec skipped. Cancelled is not failed, so on the two PRs whose entire subject was layout that jest structurally cannot see, the e2e signal was absent while nothing showed as red. Measured over the last 40 runs, walking every attempt rather than only the latest (`gh run list` reports the latest attempt, which is what hides a re-run to green): 39 successful installs, 36 at or under 39s, outliers at 101s, 140s and 381s; 5 cancelled, none under 585s. Two changes: - `timeout-minutes: 8` on the install step, converting the silent skip into a red X naming the step. The distinction it relies on is in the runner itself: `StepsRunner.cs` sets `TaskResult.Failed` when the step's own token fired and the job's did not, and `TaskResult.Canceled` otherwise. 8 minutes sits in the gap the data leaves: above the 381s slowest healthy install, below the 585s floor of every hang. - The job cap goes 10 -> 12 minutes so that step cap is always what fires. Every other step at its worst observed duration sums to 78s, plus about 30s for the cache restore and save, so a full 480s install would land at ~600s, exactly the old job cap. At 10 the job would time out first and cancel the run, reproducing the bug. A cap is not a cost: the median successful job takes 69s (n=39) and only pathological runs approach either limit. `cancel-in-progress` is now scoped to pull requests. Left unconditional it would apply to the new push trigger too, superseding exactly the main run that populates the cache and carries the post-merge signal. - Restore and save `~/.cache/ms-playwright`, keyed on the Playwright version resolved from `package-lock.json`, so the common path skips the download and the 22s it costs. The workflow now also runs on push to main, because a cache written by a `pull_request` run lands in that PR's own scope: with no run on the default branch there is no base-branch scope to inherit, and the first run of every PR would miss. That also gives main a post-merge Playwright signal it did not have. No retry. A retry has to be capped above the healthy maximum or it kills good runs, and two 381s-capped attempts do not fit in a 10-minute job alongside npm ci, the build and the specs. An earlier draft of this change used three 90s attempts, which the distribution above shows would have turned all three healthy outliers red. Restore and save are separate steps rather than the combined `actions/cache`, whose save is a post step gated on `post-if: success()` (verified in the action's own action.yml at the pinned SHA). That never stores the browser on a run whose specs failed, which is exactly the run being iterated on. Saving straight after a successful install decouples the cache from the verdict. Verified locally: actionlint 1.7.12 and zizmor 1.29.0 at `--persona=pedantic --min-severity=medium` both exit 0 with the finding count unchanged from origin/main at every severity; structural assertions on the parsed workflow cover the timeout ordering, that `Run Playwright tests` is unconditional and reachable on both the hit and miss paths, and that the save is the only step gated on the cache; and the version-resolution block was run against the real lockfile to confirm a Playwright bump changes the key, an unrelated lockfile edit does not, and a missing entry fails the step instead of keying on an empty string. The wedge is not reproducible on demand, so none of this shows the download no longer hangs. It shows that a hang now costs a named red step instead of a silently skipped suite. Refs #1869 --- .github/workflows/frontend-e2e.yml | 74 ++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 4 deletions(-) diff --git a/.github/workflows/frontend-e2e.yml b/.github/workflows/frontend-e2e.yml index 902f7d9c1..50000e07e 100644 --- a/.github/workflows/frontend-e2e.yml +++ b/.github/workflows/frontend-e2e.yml @@ -1,4 +1,4 @@ -name: Frontend E2E (PR) +name: Frontend E2E on: pull_request: @@ -7,19 +7,40 @@ on: paths: - "frontend/**" - ".github/workflows/frontend-e2e.yml" + # On main only to populate the browser cache in the default-branch scope: a + # cache written by a pull_request run is readable only within that PR, so + # without this the first run of every PR misses. + push: + branches: + - main + paths: + - "frontend/**" + - ".github/workflows/frontend-e2e.yml" permissions: contents: read concurrency: - group: frontend-e2e-pr-${{ github.ref }} - cancel-in-progress: true + group: frontend-e2e-${{ github.ref }} + # PRs only. Superseding a push on main would cancel the run that populates + # the cache and the one carrying the post-merge signal, which are the two + # reasons the push trigger exists. + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: playwright: name: Playwright Chromium runs-on: ubuntu-latest - timeout-minutes: 10 + # Raised from 10 so the install step's own 8-minute cap is always what + # fires. Every other step at its worst across all attempts sums to 78s + # (75s if you look only at green runs, which understates it, and no sample + # exercised the spec-failure path at all), plus roughly 30s for the cache + # restore and save. So a full 480s install lands at ~600s: exactly the old + # cap, leaving nothing for the job timeout to be a backstop with. And a + # job timeout cancels, which is the conclusion #1869 is about. A cap is + # not a cost: the median successful job takes 69s, and only pathological + # runs ever approach either limit. + timeout-minutes: 12 defaults: run: working-directory: frontend @@ -45,9 +66,54 @@ jobs: - name: Build run: npm run build + # The resolved version, not the `^1.60.0` range in package.json, so a + # floating range cannot silently reuse the previous browser build. + - name: Resolve Playwright version + id: playwright + run: | + set -euo pipefail + version="$(jq -re '.packages["node_modules/@playwright/test"].version' package-lock.json)" + echo "version=${version}" >> "$GITHUB_OUTPUT" + echo "Playwright ${version}" + + # Split restore/save rather than the combined actions/cache, whose save is + # a post step gated on `post-if: success()` and so skips the run being + # iterated on: the one whose specs are still failing. + - name: Restore Chromium + id: chromium-cache + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: ~/.cache/ms-playwright + key: ${{ runner.os }}-${{ runner.arch }}-playwright-${{ steps.playwright.outputs.version }}-chromium + - name: Install Chromium + # This step timeout is the fix for #1869, where this install wedged + # five times on 2026-08-19 (#1864, #1868) and ran until the job cap, + # ending each run `cancelled` with every spec skipped. Cancelled is not + # failed, so the missing e2e signal showed up as nothing at all. A step + # that busts its own timeout FAILS, so a future wedge is a red X naming + # this step. + # + # 8 minutes sits in the gap measured over the last 40 runs and all + # their attempts: 39 successful installs, 36 at or under 39s with + # outliers at 101s, 140s and 381s, against 5 wedges none under 585s. + # Above the healthy maximum, below every wedge, under the job cap. + # That gap is also why there is no retry: an attempt capped under 381s + # fails healthy runs, and two capped above it do not fit in the job. + timeout-minutes: 8 + # Idempotent on a cache hit: skips the download when the restored + # browser is complete and re-fetches when it is not. run: npx playwright install --with-deps chromium + - name: Save Chromium + # Skipped on a hit, and on an install failure (an untaken `if:` still + # implies success()), so only a complete browser reaches the key. + if: steps.chromium-cache.outputs.cache-hit != 'true' + uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: ~/.cache/ms-playwright + key: ${{ steps.chromium-cache.outputs.cache-primary-key }} + - name: Run Playwright tests run: npm run test:e2e