From a23ee61184fc801c1e233c88f75cb69afdee0835 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 25 Jul 2026 23:02:03 +0200 Subject: [PATCH] test(e2e): route cleanup happens by construction, not per spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A spec that arms `page.route(...)` in a `beforeEach` and never drops the handler races its own teardown. The context closes, an async handler is still awaiting, its next call rejects with "Test ended.", and Playwright attributes that to the test that had just passed. With `retries: 2` plus `failOnFlakyTests`, which spec bites is a matter of timing. `e2e/setup/test.ts` re-exports Playwright's `test` extended with an auto fixture that unroutes every page and the context after each test, and clears the page event listeners a spec subscribes to; `expect` rides along so a spec needs one import. Auto means a spec author cannot forget it and does not have to know it exists. Every spec now takes `test` from there, and `e2e-route-cleanup-guard.test.ts` keeps the import boundary closed with an allowlist that carries its reason. The per-file `afterEach` on the cycle spec goes away — leaving both would read as not trusting the fixture. Not covered: a page inside a context a spec builds itself. Only `share-documents.spec.ts` does that, and it owns those contexts. --- e2e/README.md | 6 +- e2e/a11y.spec.ts | 9 +- e2e/achievements.spec.ts | 2 +- e2e/admin-api-tokens-mobile.spec.ts | 2 +- e2e/ai-provider-dropdown.spec.ts | 2 +- e2e/auth-redirect.spec.ts | 2 +- e2e/central-codex.spec.ts | 2 +- e2e/chart-overlay-controls.spec.ts | 2 +- e2e/charts-mobile.spec.ts | 2 +- e2e/coach-conversation-rename.spec.ts | 2 +- e2e/coach-guided-questions.spec.ts | 2 +- e2e/codex-flow.spec.ts | 2 +- e2e/cycle-insights-mobile.spec.ts | 10 +- e2e/dashboard.spec.ts | 2 +- e2e/documents-ai.spec.ts | 3 +- e2e/documents-auto-read.spec.ts | 3 +- e2e/documents-chat.spec.ts | 3 +- e2e/documents.spec.ts | 3 +- e2e/insights-generate.spec.ts | 2 +- e2e/insights-scroll-restoration.spec.ts | 2 +- e2e/ipad-viewport.spec.ts | 2 +- e2e/locale-switch.spec.ts | 2 +- e2e/login.spec.ts | 2 +- e2e/measurement-flow.spec.ts | 2 +- e2e/medications-wizard-biweekly.spec.ts | 2 +- e2e/medications-wizard-compose.spec.ts | 3 +- e2e/medications-wizard-daily.spec.ts | 2 +- e2e/medications-wizard-monthly.spec.ts | 2 +- e2e/medications-wizard-oneshot.spec.ts | 2 +- e2e/medications-wizard-rolling.spec.ts | 2 +- e2e/medications-wizard-weekdays.spec.ts | 2 +- e2e/mobile-density.spec.ts | 2 +- e2e/mobile-viewport.spec.ts | 2 +- e2e/mood-card-mobile.spec.ts | 2 +- e2e/mood-timestamp.spec.ts | 2 +- e2e/onboarding-flicker.spec.ts | 2 +- e2e/onboarding-tour-passthrough.spec.ts | 2 +- e2e/refetch-nav.spec.ts | 2 +- e2e/settings-export.spec.ts | 2 +- e2e/settings-integrations-mobile.spec.ts | 2 +- e2e/settings-mobile-consistency.spec.ts | 2 +- e2e/settings-overscroll.spec.ts | 2 +- e2e/setup/test.ts | 90 ++++++++++ e2e/share-documents.spec.ts | 3 +- e2e/sidebar-admin-no-expand.spec.ts | 2 +- e2e/timezone-profile.spec.ts | 2 +- e2e/today-just-in.spec.ts | 2 +- e2e/v1427-admin-login-overview.spec.ts | 2 +- e2e/v1427-coach-launch.spec.ts | 2 +- e2e/v1427-insights-empty-state.spec.ts | 2 +- e2e/v1427-measurements-add-param.spec.ts | 2 +- e2e/v1427-public-pages.spec.ts | 2 +- e2e/v1427-responsive-sheet.spec.ts | 2 +- e2e/version.spec.ts | 2 +- e2e/water-capture.spec.ts | 2 +- e2e/workouts-infinite-history.spec.ts | 2 +- src/__tests__/e2e-route-cleanup-guard.test.ts | 156 ++++++++++++++++++ 57 files changed, 311 insertions(+), 70 deletions(-) create mode 100644 e2e/setup/test.ts create mode 100644 src/__tests__/e2e-route-cleanup-guard.test.ts 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 \}/); + }); +});