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"]'), }, ]; 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" });