From ab13cfb689f1d0608487fbd13d20de5fd7f0b90e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 2 Aug 2026 05:37:03 +0200 Subject: [PATCH 1/2] Scan the painted page, not the skeleton standing in front of it The `/insights/workouts` case gated its axe scan on `#main-content [role="status"]`. That role is not a content marker: the busy skeletons carry it too, and the one the streamed route shell paints first is a `chart-skeleton` with `aria-busy="true"`. The sentinel therefore went green on a document the page client had not touched yet. A frame trace under a 20x CPU throttle puts that skeleton on screen at 408 ms and the page's own `

` at 4119 ms, so for most of four seconds the scan could land on a tree with no level-one heading and report `page-has-heading-one` against `html`. That is the failure a loaded runner has been showing, and the page it accused has the heading. Gate on the two states the page itself renders instead, the list or the empty state. The overview case above already excludes `aria-busy` skeletons the same way. `/coach` had a smaller version of the same gap: `coach-page` is the sizing wrapper around a ``, on screen 241 ms before the conversation under it exists. Gate on the conversation. Neither sentinel waits for a heading. Removing the `

` from `SubPageShell`, `CoachHero` and the conversation's screen-reader title still turns the matrix red with four `page-has-heading-one` violations per theme. --- e2e/a11y.spec.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/e2e/a11y.spec.ts b/e2e/a11y.spec.ts index 03b200203..3cd202203 100644 --- a/e2e/a11y.spec.ts +++ b/e2e/a11y.spec.ts @@ -664,12 +664,21 @@ const INSIGHTS_ROUTES: readonly RouteCase[] = [ painted: (page) => page.locator(".recharts-wrapper").first(), }, { + // The two states this page's OWN render produces: the list, or the + // no-workouts empty state. Never `[role="status"]`, because the busy + // skeletons carry it too (`chart-skeleton` in the streamed route shell is + // one), so that selector matched the loading tree the server sends before + // the page client has mounted anything. On a loaded runner the scan then + // landed on a document whose only heading had not rendered yet and blamed + // the app for a missing `

` it does have. Same rule as the overview + // case above: a sentinel must prove the content, not the placeholder + // standing in for it. name: "/insights/workouts list", path: "/insights/workouts", painted: (page) => page .locator( - '#main-content [data-slot="workout-list"], #main-content [role="status"]', + '#main-content [data-slot="workout-list"], #main-content [data-slot="empty-state"]', ) .first(), }, @@ -679,9 +688,13 @@ const INSIGHTS_ROUTES: readonly RouteCase[] = [ painted: (page) => page.locator('[data-slot="workout-detail-header"]'), }, { + // `coach-page` is the sizing wrapper around a ``, + // so it is on screen while the conversation below it is still nothing at + // all. Gate on the conversation surface itself. name: "/coach", path: "/coach", - painted: (page) => page.locator('[data-slot="coach-page"]').first(), + painted: (page) => + page.locator('[data-slot="coach-conversation"][data-variant="page"]'), }, ]; From ece9ba90b11b2b67a66d2ddc42dbcdb59f272954 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 2 Aug 2026 05:37:14 +0200 Subject: [PATCH 2/2] Bound the two settle waits that could never time out `settleBeforeMeasure` ends with a `waitForLoadState("networkidle")` whose rejection is swallowed and a `page.evaluate` awaiting `document.fonts.ready`. Both are written to end on their own terms, and neither had a bound: the config leaves `actionTimeout` and `navigationTimeout` unset, which Playwright reads as no limit, so a single sub-resource that never completes consumes the whole test budget rather than the wait. The way it surfaces is the worst part. The swallowed timeout hides the stall, the run reports whichever call was still pending when the fixture closed the page, and the message reads Test timeout of 30000ms exceeded. Error: page.evaluate: Target page, context or browser has been closed which names neither the surface nor the resource. Raising the budget changes nothing, because no budget bounds an unbounded wait. Route one font request to a handler that never answers and the current code reproduces that message exactly; with the bound in place the same run reaches the spec's own assertion in twelve seconds. Six seconds is an order of magnitude above the slowest settle measured across the suite (1.1 s, /insights at a mobile viewport). --- e2e/utils/settle.ts | 46 +++++++++++++++++++++++++++++++++-- e2e/v1341-ui-evidence.spec.ts | 6 +++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/e2e/utils/settle.ts b/e2e/utils/settle.ts index de8afc088..dac403129 100644 --- a/e2e/utils/settle.ts +++ b/e2e/utils/settle.ts @@ -1,5 +1,23 @@ import { expect, type Locator, type Page } from "@playwright/test"; +/** + * How long either settle wait in `settleBeforeMeasure` may take before the + * spec moves on. + * + * Both of those waits are written to end on their own terms; neither was + * bounded, and neither inherits a bound from anywhere, because the config + * leaves `actionTimeout` and `navigationTimeout` unset and Playwright reads + * that as "no limit". One request that never completes therefore ends the + * whole test instead of just the wait, and it ends it in the least useful way + * available: the run reports whichever call was still pending when the fixture + * tore the page down, so the message names a `page.evaluate` against a closed + * target rather than the surface that stalled. Six seconds is an order of + * magnitude above the slowest settle measured anywhere in this suite. Past + * that it is a stall, and a stall belongs in front of the spec's own + * assertions, which can say what is actually wrong. + */ +const SETTLE_BUDGET_MS = 6_000; + /** * Gate a one-shot DOM read on the element it is about to measure. * @@ -45,6 +63,30 @@ export async function settleBeforeMeasure( .catch(() => {}); // Fonts and late CSS change geometry after the element is visible, and // every measurement in this suite is geometric. - await page.waitForLoadState("networkidle").catch(() => {}); - await page.evaluate(() => document.fonts?.ready); + await page + .waitForLoadState("networkidle", { timeout: SETTLE_BUDGET_MS }) + .catch(() => {}); + await waitForFonts(page); +} + +/** + * Wait for webfonts to settle, bounded inside the page. + * + * `document.fonts.ready` belongs to the document, so awaiting it from a + * `page.evaluate` hands the test an unbounded wait on a promise no timeout + * reaches. Race it against a timer in the page instead, so a font that never + * arrives costs this wait and nothing else. + */ +export async function waitForFonts( + page: Page, + timeout = SETTLE_BUDGET_MS, +): Promise { + await page.evaluate(async (budget: number) => { + const ready = document.fonts?.ready; + if (!ready) return; + await Promise.race([ + ready, + new Promise((resolve) => window.setTimeout(resolve, budget)), + ]); + }, timeout); } diff --git a/e2e/v1341-ui-evidence.spec.ts b/e2e/v1341-ui-evidence.spec.ts index 6448d3472..bbfccb930 100644 --- a/e2e/v1341-ui-evidence.spec.ts +++ b/e2e/v1341-ui-evidence.spec.ts @@ -11,7 +11,7 @@ import { mockDashboardSnapshot, } from "./utils/mock-dashboard-snapshot"; import { mockPopulatedInsights } from "./utils/mock-populated-insights"; -import { settleBeforeMeasure } from "./utils/settle"; +import { settleBeforeMeasure, waitForFonts } from "./utils/settle"; const EVIDENCE_DIR = join(process.cwd(), "test-results", "v1341"); @@ -46,7 +46,9 @@ async function settleAndCapture( content: "*,*::before,*::after{animation:none!important;transition:none!important;caret-color:transparent!important}", }); - await page.evaluate(() => document.fonts?.ready); + // The injected stylesheet can pull a font the page had not needed yet, so + // the wait repeats after it. Bounded, for the reason `waitForFonts` gives. + await waitForFonts(page); const path = join(EVIDENCE_DIR, filename); await page.screenshot({ path, animations: "disabled" }); await testInfo.attach(filename, { path, contentType: "image/png" });