Files
Marc-André Bombeck a23ee61184 test(e2e): route cleanup happens by construction, not per spec
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.
2026-07-25 23:02:03 +02:00

91 lines
3.4 KiB
TypeScript

/**
* 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 };