diff --git a/e2e/README.md b/e2e/README.md index fa2e35d96..4cf2a7ef8 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -12,10 +12,12 @@ The Playwright config (`../playwright.config.ts`) builds the app (`next build`) ## Layout -- **`setup/`** — `global-setup.ts` (auth + DB seeding), `test-helpers.ts` (shared login + navigation helpers). +- **`setup/`** — `test.ts` (the suite's `test` + `expect` entry point), `global-setup.ts` (auth + DB seeding), `test-helpers.ts` (shared login + navigation helpers). - **`utils/`** — fixtures such as `mock-dashboard-snapshot.ts`. - **`*.spec.ts`** — one spec per flow: auth/login/redirect, dashboard, charts, the medication-wizard cadences (`medications-wizard-*`), insights generation + scroll restoration, doctor report, settings, mobile-viewport consistency, locale switch, accessibility (`a11y.spec.ts`), and the `v1427-*` regression set. -## Convention +## Conventions + +Import `test` and `expect` from `./setup/test`, never from `@playwright/test` — types (`Page`, `Route`, …) still come from the package. The entry point extends Playwright's `test` with an automatic fixture that drops route handlers and page listeners after every test, so a handler left in flight cannot fail the test it belonged to once the context closes. `src/__tests__/e2e-route-cleanup-guard.test.ts` enforces the import boundary. Assert against stable `data-*` attributes (`data-slot`, `data-display-step`, root display attrs), not viewport-dependent visible text. Responsive `sm:hidden` UI changes break `getByText` on desktop while the underlying element is still present — anchoring on data attributes keeps the specs stable across breakpoints. See [`../CLAUDE.md`](../CLAUDE.md). diff --git a/e2e/a11y.spec.ts b/e2e/a11y.spec.ts index 1fc1382d3..bcce6030e 100644 --- a/e2e/a11y.spec.ts +++ b/e2e/a11y.spec.ts @@ -1,11 +1,6 @@ import AxeBuilder from "@axe-core/playwright"; -import { - expect, - test, - type Locator, - type Page, - type Route, -} from "@playwright/test"; +import type { Locator, Page, Route } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { POPULATED_SUMMARIES } from "./utils/mock-dashboard-snapshot"; diff --git a/e2e/achievements.spec.ts b/e2e/achievements.spec.ts index 9786df03c..9dacf0042 100644 --- a/e2e/achievements.spec.ts +++ b/e2e/achievements.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/admin-api-tokens-mobile.spec.ts b/e2e/admin-api-tokens-mobile.spec.ts index 08bef6dd2..3f82cdb2d 100644 --- a/e2e/admin-api-tokens-mobile.spec.ts +++ b/e2e/admin-api-tokens-mobile.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/ai-provider-dropdown.spec.ts b/e2e/ai-provider-dropdown.spec.ts index f12dd9b05..ea2c00bb2 100644 --- a/e2e/ai-provider-dropdown.spec.ts +++ b/e2e/ai-provider-dropdown.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/auth-redirect.spec.ts b/e2e/auth-redirect.spec.ts index 931cb32d9..196373969 100644 --- a/e2e/auth-redirect.spec.ts +++ b/e2e/auth-redirect.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; /** * The proxy at src/proxy.ts is the single gate enforcing auth on every diff --git a/e2e/central-codex.spec.ts b/e2e/central-codex.spec.ts index dec89459b..8d2e8a4ee 100644 --- a/e2e/central-codex.spec.ts +++ b/e2e/central-codex.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/chart-overlay-controls.spec.ts b/e2e/chart-overlay-controls.spec.ts index 2c4d5f8ef..84d7f2cc8 100644 --- a/e2e/chart-overlay-controls.spec.ts +++ b/e2e/chart-overlay-controls.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/charts-mobile.spec.ts b/e2e/charts-mobile.spec.ts index a5e86cf17..3e933b4d4 100644 --- a/e2e/charts-mobile.spec.ts +++ b/e2e/charts-mobile.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { mockDashboardSnapshot } from "./utils/mock-dashboard-snapshot"; diff --git a/e2e/coach-conversation-rename.spec.ts b/e2e/coach-conversation-rename.spec.ts index 97551287e..1a8677e9f 100644 --- a/e2e/coach-conversation-rename.spec.ts +++ b/e2e/coach-conversation-rename.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { setTimeout as delay } from "node:timers/promises"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/coach-guided-questions.spec.ts b/e2e/coach-guided-questions.spec.ts index 2b3599965..638d9c74e 100644 --- a/e2e/coach-guided-questions.spec.ts +++ b/e2e/coach-guided-questions.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/codex-flow.spec.ts b/e2e/codex-flow.spec.ts index 5e929066e..ce5883886 100644 --- a/e2e/codex-flow.spec.ts +++ b/e2e/codex-flow.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/cycle-insights-mobile.spec.ts b/e2e/cycle-insights-mobile.spec.ts index a247bbddd..d45dfbbf2 100644 --- a/e2e/cycle-insights-mobile.spec.ts +++ b/e2e/cycle-insights-mobile.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; @@ -175,14 +175,6 @@ test.describe("cycle + insights mobile smoke", () => { ); }); - // These handlers call route.fetch(), which needs a live page. Teardown can - // close the context while one is still in flight, and the rejected fetch - // then fails a test that had already passed. Drop the handlers first so - // nothing is left racing the close. - test.afterEach(async ({ page }) => { - await page.unrouteAll({ behavior: "ignoreErrors" }); - }); - test("/insights has no horizontal scroll on Pixel-5", async ({ page }) => { await page.goto("/insights", { waitUntil: "domcontentloaded" }); await page.waitForLoadState("networkidle"); diff --git a/e2e/dashboard.spec.ts b/e2e/dashboard.spec.ts index 017051510..ff07c9902 100644 --- a/e2e/dashboard.spec.ts +++ b/e2e/dashboard.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { mockDashboardSnapshot } from "./utils/mock-dashboard-snapshot"; diff --git a/e2e/documents-ai.spec.ts b/e2e/documents-ai.spec.ts index f0f8abbbc..2e75854b0 100644 --- a/e2e/documents-ai.spec.ts +++ b/e2e/documents-ai.spec.ts @@ -16,7 +16,8 @@ * seeds a real `document_content_index` row (`ensureVaultAiFixture`) and drives * the real list union. */ -import { expect, test, type Page } from "@playwright/test"; +import type { Page } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/documents-auto-read.spec.ts b/e2e/documents-auto-read.spec.ts index 5583b7320..d341fbed6 100644 --- a/e2e/documents-auto-read.spec.ts +++ b/e2e/documents-auto-read.spec.ts @@ -15,7 +15,8 @@ * AI e2e, so it is pinned here too. Assertions target stable data-slots, never * viewport-dependent text. */ -import { expect, test, type Page } from "@playwright/test"; +import type { Page } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/documents-chat.spec.ts b/e2e/documents-chat.spec.ts index e57fc0b78..85afe9b79 100644 --- a/e2e/documents-chat.spec.ts +++ b/e2e/documents-chat.spec.ts @@ -22,7 +22,8 @@ * the specs time out. The seeded demo user has no AI provider, so the * no-provider state is the default. */ -import { expect, test, type Page } from "@playwright/test"; +import type { Page } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/documents.spec.ts b/e2e/documents.spec.ts index 470a67405..0232fd2fc 100644 --- a/e2e/documents.spec.ts +++ b/e2e/documents.spec.ts @@ -17,7 +17,8 @@ * they create, never on the shared doctor-flow corpus. */ import AxeBuilder from "@axe-core/playwright"; -import { expect, test, type Page } from "@playwright/test"; +import type { Page } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/insights-generate.spec.ts b/e2e/insights-generate.spec.ts index 19deb147c..1bce560a7 100644 --- a/e2e/insights-generate.spec.ts +++ b/e2e/insights-generate.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/insights-scroll-restoration.spec.ts b/e2e/insights-scroll-restoration.spec.ts index b5be7995a..8dd6e160f 100644 --- a/e2e/insights-scroll-restoration.spec.ts +++ b/e2e/insights-scroll-restoration.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/ipad-viewport.spec.ts b/e2e/ipad-viewport.spec.ts index dde8b9369..8667d8c27 100644 --- a/e2e/ipad-viewport.spec.ts +++ b/e2e/ipad-viewport.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/locale-switch.spec.ts b/e2e/locale-switch.spec.ts index fc231d80d..e66191744 100644 --- a/e2e/locale-switch.spec.ts +++ b/e2e/locale-switch.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; /** * Locale-key drift smoke check — switch the cookie between EN and DE diff --git a/e2e/login.spec.ts b/e2e/login.spec.ts index 23a29bf63..4a0b4ea49 100644 --- a/e2e/login.spec.ts +++ b/e2e/login.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; /** * Smoke checks on the login surface — must render, must wire the diff --git a/e2e/measurement-flow.spec.ts b/e2e/measurement-flow.spec.ts index 904095a1f..8457cf0e1 100644 --- a/e2e/measurement-flow.spec.ts +++ b/e2e/measurement-flow.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/medications-wizard-biweekly.spec.ts b/e2e/medications-wizard-biweekly.spec.ts index 68cc6b024..d687051fa 100644 --- a/e2e/medications-wizard-biweekly.spec.ts +++ b/e2e/medications-wizard-biweekly.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STUB_MEDICATION_ID, diff --git a/e2e/medications-wizard-compose.spec.ts b/e2e/medications-wizard-compose.spec.ts index ddba23f37..d31b51f14 100644 --- a/e2e/medications-wizard-compose.spec.ts +++ b/e2e/medications-wizard-compose.spec.ts @@ -1,4 +1,5 @@ -import { expect, test, type Route } from "@playwright/test"; +import type { Route } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STUB_MEDICATION_ID, diff --git a/e2e/medications-wizard-daily.spec.ts b/e2e/medications-wizard-daily.spec.ts index e6d773dd4..fb6b1dd7d 100644 --- a/e2e/medications-wizard-daily.spec.ts +++ b/e2e/medications-wizard-daily.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STUB_MEDICATION_ID, diff --git a/e2e/medications-wizard-monthly.spec.ts b/e2e/medications-wizard-monthly.spec.ts index 7e7a21373..885500859 100644 --- a/e2e/medications-wizard-monthly.spec.ts +++ b/e2e/medications-wizard-monthly.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STUB_MEDICATION_ID, diff --git a/e2e/medications-wizard-oneshot.spec.ts b/e2e/medications-wizard-oneshot.spec.ts index d851f9a4a..abe36b10a 100644 --- a/e2e/medications-wizard-oneshot.spec.ts +++ b/e2e/medications-wizard-oneshot.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STUB_MEDICATION_ID, diff --git a/e2e/medications-wizard-rolling.spec.ts b/e2e/medications-wizard-rolling.spec.ts index 02ecafc8e..c2ecef158 100644 --- a/e2e/medications-wizard-rolling.spec.ts +++ b/e2e/medications-wizard-rolling.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STUB_MEDICATION_ID, diff --git a/e2e/medications-wizard-weekdays.spec.ts b/e2e/medications-wizard-weekdays.spec.ts index 5386c6666..5ea977aaa 100644 --- a/e2e/medications-wizard-weekdays.spec.ts +++ b/e2e/medications-wizard-weekdays.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STUB_MEDICATION_ID, diff --git a/e2e/mobile-density.spec.ts b/e2e/mobile-density.spec.ts index af0251581..ebe54672a 100644 --- a/e2e/mobile-density.spec.ts +++ b/e2e/mobile-density.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/mobile-viewport.spec.ts b/e2e/mobile-viewport.spec.ts index af1e46888..200bf4c28 100644 --- a/e2e/mobile-viewport.spec.ts +++ b/e2e/mobile-viewport.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/mood-card-mobile.spec.ts b/e2e/mood-card-mobile.spec.ts index 9b74d77d1..e3ffa57fd 100644 --- a/e2e/mood-card-mobile.spec.ts +++ b/e2e/mood-card-mobile.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/mood-timestamp.spec.ts b/e2e/mood-timestamp.spec.ts index 3aeca210c..28daa3df8 100644 --- a/e2e/mood-timestamp.spec.ts +++ b/e2e/mood-timestamp.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/onboarding-flicker.spec.ts b/e2e/onboarding-flicker.spec.ts index d69f78d61..bd0ee08ac 100644 --- a/e2e/onboarding-flicker.spec.ts +++ b/e2e/onboarding-flicker.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/onboarding-tour-passthrough.spec.ts b/e2e/onboarding-tour-passthrough.spec.ts index 558967f53..003bd5e01 100644 --- a/e2e/onboarding-tour-passthrough.spec.ts +++ b/e2e/onboarding-tour-passthrough.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/refetch-nav.spec.ts b/e2e/refetch-nav.spec.ts index ce2f52e84..fb0f16934 100644 --- a/e2e/refetch-nav.spec.ts +++ b/e2e/refetch-nav.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/settings-export.spec.ts b/e2e/settings-export.spec.ts index b116e6a16..e2de1e22b 100644 --- a/e2e/settings-export.spec.ts +++ b/e2e/settings-export.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/settings-integrations-mobile.spec.ts b/e2e/settings-integrations-mobile.spec.ts index 55ab67f28..92397f634 100644 --- a/e2e/settings-integrations-mobile.spec.ts +++ b/e2e/settings-integrations-mobile.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/settings-mobile-consistency.spec.ts b/e2e/settings-mobile-consistency.spec.ts index a75a456d0..4ba303cf1 100644 --- a/e2e/settings-mobile-consistency.spec.ts +++ b/e2e/settings-mobile-consistency.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/settings-overscroll.spec.ts b/e2e/settings-overscroll.spec.ts index 544ecfb7b..b55eb33cc 100644 --- a/e2e/settings-overscroll.spec.ts +++ b/e2e/settings-overscroll.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/setup/test.ts b/e2e/setup/test.ts new file mode 100644 index 000000000..8893f5aa5 --- /dev/null +++ b/e2e/setup/test.ts @@ -0,0 +1,90 @@ +/** + * The E2E suite's `test` / `expect` entry point. + * + * Every spec imports from here instead of from `@playwright/test`, and the + * structural guard `src/__tests__/e2e-route-cleanup-guard.test.ts` keeps it + * that way. + * + * ## Why + * + * A spec that registers `page.route(...)` in a `beforeEach` leaves the handler + * armed until the context closes. If the handler is async — `route.fetch()`, + * an `await` before `route.fulfill()` — teardown can close the context while + * one invocation is still in flight. The callback then rejects against a dead + * target, Playwright attributes the rejection to the test that just finished, + * and a test that had already passed is reported as failed. With `retries: 2` + * plus `failOnFlakyTests` on CI, that is a release blocker whose trigger is + * timing, not code. + * + * The remedy Playwright names is `unrouteAll({ behavior: "ignoreErrors" })` + * before the test ends. Written per spec that is 36 copies of the same three + * lines and one forgotten copy away from the same failure. Written as an auto + * fixture it happens for every test in the suite, including specs nobody has + * written yet, and a spec author never has to know it exists. + * + * ## What it covers + * + * - `page.route(...)` on every page in the test's context. + * - `context.route(...)` on the context itself. + * - Page event listeners for the interactive event types a spec subscribes to. + * A listener that awaits something (a `dialog` handler, a `response` handler + * that reads a body) can reject after teardown for exactly the same reason a + * route handler can. + * + * ## What it does not cover + * + * A page that lives in a context the spec created itself — `browser.newContext()` + * inside a helper — is invisible here, because this fixture can only see the + * context Playwright built for the test. `share-documents.spec.ts` is the one + * spec that does this; it owns those contexts and has to clean them up itself. + */ +import { test as base, expect } from "@playwright/test"; + +/** + * Page event types cleared at teardown. Deliberately not the lifecycle events + * (`close`, `crash`, `frameattached`, …): those describe the teardown we are + * standing in, and nothing in the suite hangs work off them. These are the + * types a spec attaches a handler to, and the ones whose handler can still be + * running when the context goes away. + */ +const OBSERVED_PAGE_EVENTS = [ + "console", + "dialog", + "download", + "filechooser", + "pageerror", + "popup", + "request", + "requestfailed", + "requestfinished", + "response", + "websocket", + "worker", +] as const; + +export const test = base.extend<{ routeCleanup: void }>({ + routeCleanup: [ + async ({ context }, use) => { + await use(); + + // Best-effort: a spec is free to close its own context (or the browser + // can die under it), and a teardown helper must never be the thing that + // turns a green run red. Everything below is a cleanup, so a failure to + // clean up has nothing left to protect. + try { + for (const page of context.pages()) { + await page.unrouteAll({ behavior: "ignoreErrors" }); + for (const event of OBSERVED_PAGE_EVENTS) { + await page.removeAllListeners(event, { behavior: "ignoreErrors" }); + } + } + await context.unrouteAll({ behavior: "ignoreErrors" }); + } catch { + // Target already gone — nothing left to unroute. + } + }, + { auto: true }, + ], +}); + +export { expect }; diff --git a/e2e/share-documents.spec.ts b/e2e/share-documents.spec.ts index d33cfe1de..28d00a7d6 100644 --- a/e2e/share-documents.spec.ts +++ b/e2e/share-documents.spec.ts @@ -24,7 +24,8 @@ * broader corpus (`ensureVaultFixture`) supplies the ≥50 documents the picker * cap test and the not-attached foreign-id probe need. */ -import { expect, test, type Browser, type Page } from "@playwright/test"; +import type { Browser, Page } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/sidebar-admin-no-expand.spec.ts b/e2e/sidebar-admin-no-expand.spec.ts index 97051932c..2c914934e 100644 --- a/e2e/sidebar-admin-no-expand.spec.ts +++ b/e2e/sidebar-admin-no-expand.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/timezone-profile.spec.ts b/e2e/timezone-profile.spec.ts index 50dd311d4..f4865aa1d 100644 --- a/e2e/timezone-profile.spec.ts +++ b/e2e/timezone-profile.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/today-just-in.spec.ts b/e2e/today-just-in.spec.ts index 843783ce5..c4f0c61ca 100644 --- a/e2e/today-just-in.spec.ts +++ b/e2e/today-just-in.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import pg from "pg"; import { E2E_USER, STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/v1427-admin-login-overview.spec.ts b/e2e/v1427-admin-login-overview.spec.ts index 8dcaad9bf..0446eacd0 100644 --- a/e2e/v1427-admin-login-overview.spec.ts +++ b/e2e/v1427-admin-login-overview.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/v1427-coach-launch.spec.ts b/e2e/v1427-coach-launch.spec.ts index 90e0a3765..9c976824c 100644 --- a/e2e/v1427-coach-launch.spec.ts +++ b/e2e/v1427-coach-launch.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/v1427-insights-empty-state.spec.ts b/e2e/v1427-insights-empty-state.spec.ts index 64c306a3e..62bbcff16 100644 --- a/e2e/v1427-insights-empty-state.spec.ts +++ b/e2e/v1427-insights-empty-state.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/v1427-measurements-add-param.spec.ts b/e2e/v1427-measurements-add-param.spec.ts index daf5096b3..e922945a6 100644 --- a/e2e/v1427-measurements-add-param.spec.ts +++ b/e2e/v1427-measurements-add-param.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/v1427-public-pages.spec.ts b/e2e/v1427-public-pages.spec.ts index 356f635f5..74eeac6db 100644 --- a/e2e/v1427-public-pages.spec.ts +++ b/e2e/v1427-public-pages.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; /** * v1.4.27 MB6 — public surfaces. diff --git a/e2e/v1427-responsive-sheet.spec.ts b/e2e/v1427-responsive-sheet.spec.ts index 1e7a35b86..762df1d6f 100644 --- a/e2e/v1427-responsive-sheet.spec.ts +++ b/e2e/v1427-responsive-sheet.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/e2e/version.spec.ts b/e2e/version.spec.ts index 9dc11eddc..98e5d5963 100644 --- a/e2e/version.spec.ts +++ b/e2e/version.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; /** * /api/version is the public, unauthenticated endpoint that drives diff --git a/e2e/water-capture.spec.ts b/e2e/water-capture.spec.ts index d45761495..7c9e99bf4 100644 --- a/e2e/water-capture.spec.ts +++ b/e2e/water-capture.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; import { diff --git a/e2e/workouts-infinite-history.spec.ts b/e2e/workouts-infinite-history.spec.ts index c4417320c..2fa882484 100644 --- a/e2e/workouts-infinite-history.spec.ts +++ b/e2e/workouts-infinite-history.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; diff --git a/src/__tests__/e2e-route-cleanup-guard.test.ts b/src/__tests__/e2e-route-cleanup-guard.test.ts new file mode 100644 index 000000000..0e7613914 --- /dev/null +++ b/src/__tests__/e2e-route-cleanup-guard.test.ts @@ -0,0 +1,156 @@ +/** + * Structural guard on the E2E suite's route-cleanup boundary. + * + * A spec that arms `page.route(...)` in a `beforeEach` and never drops the + * handler races its own teardown: the context closes, an in-flight handler + * rejects, and Playwright reports a test that had already passed as failed. + * `e2e/setup/test.ts` removes the possibility with an auto fixture, but only + * for specs that take their `test` from there — a spec that imports straight + * from `@playwright/test` silently opts out and looks identical in review. + * + * So the entry point is the invariant, and this file is what keeps it true. + * It is a tripwire, not a proof: it shows nobody has changed the import + * boundary without editing this file. That the fixture actually cleans up is + * a runtime property, proven by the suite itself. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync, globSync } from "node:fs"; +import { join, sep } from "node:path"; + +const E2E = join(process.cwd(), "e2e"); + +/** The shared entry point, relative to `e2e/`. */ +const ENTRY = "setup/test.ts"; + +/** + * Files allowed to import `test` from `@playwright/test` directly, each with + * the reason it has to. + */ +const DIRECT_IMPORT_ALLOWLIST: Record = { + // The entry point itself — it extends Playwright's base `test`, which it + // cannot do without importing it. + [ENTRY]: "defines the extended `test` every other file imports", +}; + +function e2eFiles(): string[] { + return globSync("**/*.ts", { cwd: E2E }) + .map((p) => p.split(sep).join("/")) + .sort(); +} + +function read(rel: string): string { + return readFileSync(join(E2E, rel), "utf8"); +} + +/** + * The file with its comments removed. The entry point documents the very API + * calls it makes, so a naive substring match would be satisfied by the prose + * alone — a fixture gutted to a no-op would still read as compliant. + */ +function stripComments(src: string): string { + return src.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^\s*\/\/.*$/gm, ""); +} + +/** + * Every `from ""` import statement in a file, as + * `{ module, specifiers }` — `specifiers` is the raw brace body, or `"*"` for + * a namespace import. + */ +function imports(src: string): Array<{ module: string; specifiers: string }> { + const found: Array<{ module: string; specifiers: string }> = []; + for (const m of src.matchAll( + /import\s+(type\s+)?\{([\s\S]*?)\}\s*from\s*"([^"]+)"/g, + )) { + // `import type { … }` imports no runtime binding at all. + found.push({ module: m[3], specifiers: m[1] ? "" : m[2] }); + } + for (const m of src.matchAll(/import\s+\*\s+as\s+\w+\s+from\s*"([^"]+)"/g)) { + found.push({ module: m[1], specifiers: "*" }); + } + return found; +} + +/** Does this file import the runtime `test` binding from `module`? */ +function importsTestFrom(src: string, module: string): boolean { + return imports(src) + .filter((i) => i.module === module) + .some( + (i) => + i.specifiers === "*" || + i.specifiers + .split(",") + .map((s) => s.trim()) + .filter((s) => s && !s.startsWith("type ")) + .some((s) => /^test(\s+as\s+\w+)?$/.test(s)), + ); +} + +describe("the E2E suite takes `test` from the shared entry point", () => { + it("no file outside the allowlist imports `test` from @playwright/test", () => { + const offenders = e2eFiles().filter( + (rel) => + !(rel in DIRECT_IMPORT_ALLOWLIST) && + importsTestFrom(read(rel), "@playwright/test"), + ); + + expect(offenders).toEqual([]); + }); + + it("the allowlist names only files that exist and still need the exemption", () => { + for (const rel of Object.keys(DIRECT_IMPORT_ALLOWLIST)) { + expect( + importsTestFrom(read(rel), "@playwright/test"), + `${rel} is allowlisted but no longer imports \`test\` directly — drop the entry`, + ).toBe(true); + } + }); + + it("every spec imports `test` from the entry point", () => { + const specs = e2eFiles().filter((rel) => rel.endsWith(".spec.ts")); + // Sanity: the sweep found the suite, not an empty directory. + expect(specs.length).toBeGreaterThan(40); + + const missing = specs.filter((rel) => { + const src = read(rel); + // Specs sit at `e2e/.spec.ts`, so the entry resolves as + // `./setup/test`. A nested spec would need a different prefix; none + // exists, and this assertion is what would surface one. + return !importsTestFrom(src, "./setup/test"); + }); + + expect(missing).toEqual([]); + }); +}); + +describe("the entry point still cleans up", () => { + const src = stripComments(read(ENTRY)); + + it("registers the cleanup as an automatic fixture", () => { + // `auto: true` is the whole point: a spec author cannot forget it and does + // not have to declare it. Without the flag the fixture is dead code. + expect(src).toMatch(/\{\s*auto:\s*true\s*\}/); + }); + + it("drops route handlers on both the page and the context", () => { + expect(src).toMatch(/page\.unrouteAll\(\{\s*behavior:\s*"ignoreErrors"/); + expect(src).toMatch(/context\.unrouteAll\(\{\s*behavior:\s*"ignoreErrors"/); + }); + + it("drops page event listeners", () => { + expect(src).toMatch(/removeAllListeners\(/); + }); + + it("cleans up after the test body, not before it", () => { + // `await use()` first, cleanup after — a fixture that cleaned up before + // the test would unroute the handlers a `beforeEach` had just armed. + const useAt = src.indexOf("await use()"); + const unrouteAt = src.indexOf("unrouteAll("); + expect(useAt).toBeGreaterThan(-1); + expect(unrouteAt).toBeGreaterThan(useAt); + }); + + it("exports both `test` and `expect`, so a spec needs one import", () => { + expect(src).toMatch(/export const test\b/); + expect(src).toMatch(/export \{ expect \}/); + }); +});