From 88750e00484c957bff8030b4aab29cf590bf088a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 30 Aug 2026 15:55:46 +0200 Subject: [PATCH] fix: give the geometry check a browser and stop it skipping in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hero-geometry check has never measured anything in CI. It launches Chromium in a hook, catches the launch failure, and every case then returns early — green. The quality job installs no browsers, so every run of it since the file landed printed [hero geometry] SKIPPED — no Chromium build available (Error: browserType.launch: Executable doesn't exist at /home/runner/.cache/ms-playwright/chromium_headless_shell-1234/ch) and passed. The file's own header warned about exactly this trap and then fell into it. Both ends move. The quality job installs the headless shell before the unit suite — `--only-shell chromium`, because that is the build `chromium.launch()` resolves to and the one the failing runs named, with a cache on the lockfile hash so the download is paid once. And the check now refuses to be optional: with CI set, a launch failure throws out of the hook and fails the suite with the step to restore. Without CI it still skips, through the test context so the skip and its reason land in the report rather than in a console line nobody reads. No panel number changed. The measurements the check reads back are the same ones the reserve was set against: panels of 400 / 428 / 456 / 484 px for three to six rows, 420 px for three rows plus the basis line, against a 448 px reserve. --- .github/workflows/security.yml | 20 ++++++++ .../health-score-card-geometry.test.tsx | 46 ++++++++++++------- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 633758442..f0e42e8e5 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -28,6 +28,26 @@ jobs: - run: pnpm db:generate + # The hero-geometry check in the unit suite renders the real panel in a + # real browser and measures the boxes. Without a browser here it caught + # its own launch failure and passed anyway, so from its first day until + # v1.38 it measured nothing on any run of this job. The browser is part + # of the gate now; the test refuses to skip when CI is set. + # + # Headless shell only: `chromium.launch()` resolves to + # chromium_headless_shell, which is what the failing runs named, and the + # full headed build is several times the download for nothing this job + # uses. The e2e job keeps `--with-deps chromium` — it drives a real app. + - name: Cache Playwright headless shell + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ~/.cache/ms-playwright + key: playwright-shell-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }} + restore-keys: | + playwright-shell-${{ runner.os }}- + + - run: pnpm exec playwright install --only-shell chromium + # Blocking since v1.15.20 — the settings-split refactor that kept # this warn-only is long done and the tree lints clean. - name: ESLint diff --git a/src/components/insights/__tests__/health-score-card-geometry.test.tsx b/src/components/insights/__tests__/health-score-card-geometry.test.tsx index 97547813e..5e2b0711d 100644 --- a/src/components/insights/__tests__/health-score-card-geometry.test.tsx +++ b/src/components/insights/__tests__/health-score-card-geometry.test.tsx @@ -32,11 +32,14 @@ import { HeroStrip } from "../hero-strip"; * that printed them and are reported as such, never baked into a comment as * if they were a property of the design. * - * It needs a Chromium build. `pnpm test` on a machine that has one (any - * machine that has run `pnpm e2e`) measures for real; a machine without one - * skips loudly rather than passing quietly. The unit CI job installs no - * browsers, so treat this as a check for the person editing the panel and for - * the release audit, not as the gate. + * It needs a Chromium build, and under CI it IS the gate: the quality job + * installs the headless shell before the unit suite, and a launch failure + * there fails this suite instead of skipping it. That is the whole point — + * until v1.38 the job installed no browsers, every run caught the launch + * error and passed, and the check had never measured anything in CI. On a + * developer machine without a browser it still skips, loudly, so that + * `pnpm test` stays usable; run `pnpm exec playwright install --only-shell + * chromium` once to measure for real. */ vi.mock("@/hooks/use-auth", () => ({ @@ -219,9 +222,20 @@ beforeAll(async () => { const { chromium } = await import("@playwright/test"); browser = await chromium.launch(); } catch (error) { - skipReason = `no Chromium build available (${String(error).slice(0, 120)})`; - // Loud on purpose: a geometry check that quietly skips is the same thing - // as no geometry check, and this repo has shipped that before. + const reason = `no Chromium build available (${String(error).slice(0, 200)})`; + // In CI the browser is installed by the quality job, so a launch failure + // is the job being wrong, not the machine being bare. Throwing from the + // hook fails every test in this suite: a geometry check that quietly + // skips is the same thing as no geometry check, and this repo shipped + // exactly that for months because the skip only ever reached a console + // line nobody reads. + if (process.env.CI) { + throw new Error( + `[hero geometry] ${reason}. This check is the gate in CI and it has no browser to measure with. ` + + `Restore the "pnpm exec playwright install --only-shell chromium" step in the quality job of .github/workflows/security.yml.`, + ); + } + skipReason = reason; console.warn(`[hero geometry] SKIPPED — ${skipReason}`); return; } @@ -252,8 +266,8 @@ const PANEL = '[data-slot="health-score-card"]'; const RESERVE = '[data-slot="health-score-card-skeleton"]'; describe("hero band geometry", () => { - it("holds a reserve that sits inside the range the real report produces", async () => { - if (skipReason) return expect(skipReason).toBeTruthy(); + it("holds a reserve that sits inside the range the real report produces", async (ctx) => { + if (skipReason) ctx.skip(skipReason); const heights: Record = {}; const shapes: { key: string; report: HealthScoreReport }[] = [ @@ -337,8 +351,8 @@ describe("hero band geometry", () => { ).toBeLessThanOrEqual(heights.rows6); }, 60_000); - it("keeps every row on one line in every locale", async () => { - if (skipReason) return expect(skipReason).toBeTruthy(); + it("keeps every row on one line in every locale", async (ctx) => { + if (skipReason) ctx.skip(skipReason); const widest: Record = {}; @@ -444,13 +458,14 @@ describe("hero band geometry", () => { ); }, 60_000); - it.each([ + it.for([ { width: VIEWPORT_PX, where: "the md column" }, { width: 360, where: "a 360px phone, stacked under the greeting" }, ])( "never scrolls sideways in $where", - async ({ width }) => { - if (skipReason) return expect(skipReason).toBeTruthy(); + { timeout: 60_000 }, + async ({ width }, ctx) => { + if (skipReason) ctx.skip(skipReason); const target = page!; await target.setViewportSize({ width, height: 1200 }); @@ -524,6 +539,5 @@ describe("hero band geometry", () => { await target.setViewportSize({ width: VIEWPORT_PX, height: 1200 }); }, - 60_000, ); });