From e808f87fbd0f98e4f94d742dd0ac7736783453a5 Mon Sep 17 00:00:00 2001 From: lilleman Date: Wed, 5 Aug 2026 11:00:25 +0200 Subject: [PATCH] Fail an E2E test on anything the browser logs, in all three engines --- AGENTS.md | 18 ++++++++++- README.md | 6 ++++ e2e-tests/auth-refresh.spec.ts | 2 +- e2e-tests/console-guard.ts | 54 ++++++++++++++++++++++++++++++++ e2e-tests/devstack-login.spec.ts | 2 +- e2e-tests/full-flow.spec.ts | 7 +++-- e2e-tests/language.spec.ts | 9 ++---- e2e-tests/oauth-login.spec.ts | 2 +- e2e-tests/playwright.config.ts | 14 ++++++--- e2e-tests/visual.spec.ts | 13 ++++---- src/e2e-console-guard.test.ts | 34 ++++++++++++++++++++ todo.md | 2 +- 12 files changed, 138 insertions(+), 25 deletions(-) create mode 100644 e2e-tests/console-guard.ts create mode 100644 src/e2e-console-guard.test.ts diff --git a/AGENTS.md b/AGENTS.md index c833aa8..89397e2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -193,7 +193,7 @@ them. Revisit only if the stated reason stops holding. point), and the panel sits in the top layer so a row kebab is no longer clipped by `.table-wrap`'s `overflow`. Four rules hold it together, none of them cosmetic. The panel carries **`position-anchor: auto`** — a bare `anchor()` resolves to nothing in Chromium, Firefox *and* - WebKit alike, which is why the `@engines`-tagged test in `visual.spec.ts` runs in all three rather + WebKit alike, which is why the popover test in `visual.spec.ts` runs in all three rather than resting on a one-time manual measurement. The panel stays the trigger's **next sibling inside the `.menu` wrapper**, because the open-state style and the old-browser fallback both read that adjacency, and a two-element partial cannot be dropped into an arbitrary layout. The `menu` partial @@ -214,6 +214,22 @@ them. Revisit only if the stated reason stops holding. instead of failing loud; the `every icon resolves to a defined ` e2e test catches it for anything reaching the nav. Removing an id is a core edit, so weigh it per icon rather than sweeping the registry — a few ids are registered ahead of a caller (see `todo.md`). +- **Anything the browser logs fails the E2E test that provoked it.** Every spec takes its `test` from + `e2e-tests/console-guard.ts`, which watches every page a test opens: a console error or warning, or + an uncaught exception, fails that test. A zero-JS app has nothing to say in the console, so the bar + is *zero* rather than a curated list of tolerated noise — and the two exceptions are explicit and + narrow: one module-level allowance for the COOP header Chromium drops because the e2e stacks serve + plain http over container hostnames (a deployment serves https, where it applies), and + `allowConsole(re)` for a test whose own page provokes a message on purpose — the 404 spec, whose + navigation Chromium and WebKit log. `src/e2e-console-guard.test.ts` locks the wiring in the *unit* + gate, because a spec importing `test` straight from Playwright would run unwatched and green. +- **The Ory-free specs run in all three engines; the Ory-backed ones stay on Chromium.** + `visual.spec.ts` + `language.spec.ts` are side-effect-free, so three parallel runs don't collide, + and a console message only appears in the engine that renders the page — the reason the per-test + `@engines` tag is gone: the whole Ory-free suite is the engine matrix now (`ORY_FREE` in + `e2e-tests/playwright.config.ts`). The rest write users, groups and sessions to one shared backend, + where a second engine's run would race the first, so widening them means giving each engine its own + stack. Screenshots are written per project name for the same reason. Decided 2026-08-05. ## Docker only — no host tooling diff --git a/README.md b/README.md index 2745e9a..dceba3b 100644 --- a/README.md +++ b/README.md @@ -1500,6 +1500,12 @@ Screenshots + an HTML report land in `e2e-tests/artifacts/` (git-ignored). Every is covered end-to-end; tests are independent and run **fully in parallel** for speed ([AGENTS.md](AGENTS.md)) — keep new tests side-effect-free so the suite stays fast. +**Anything the browser logs fails the test.** Specs import `test` from +`e2e-tests/console-guard.ts`, which watches every page a test opens — a console error or warning, or +an uncaught exception, fails that test; a page that provokes one on purpose allows it explicitly with +`allowConsole(/…/)`. The Ory-free suites run in **Chromium, Firefox and WebKit**, so each engine's +console is read; the Ory-backed suites share one backend and stay on Chromium. + ### The full gate (one command) `ci.sh` is the whole gate in one reproducible command — typecheck → unit tests → diff --git a/e2e-tests/auth-refresh.spec.ts b/e2e-tests/auth-refresh.spec.ts index 2b1606f..e7ad2fb 100644 --- a/e2e-tests/auth-refresh.spec.ts +++ b/e2e-tests/auth-refresh.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./console-guard.ts"; // Full-stack auth E2E: token timeout + silent re-mint ("stay signed in"). Runs against the // real Ory stack via e2e-tests/compose.auth.yml, where the session→JWT TTL is shortened to 8s and the diff --git a/e2e-tests/console-guard.ts b/e2e-tests/console-guard.ts new file mode 100644 index 0000000..2c654c6 --- /dev/null +++ b/e2e-tests/console-guard.ts @@ -0,0 +1,54 @@ +import { expect, test as base, type BrowserContext, type Page } from "@playwright/test"; + +// The `test` every spec imports: it fails a test whose browser logged a console error or warning, +// or threw, at any step — in whichever engine ran it. A zero-JS app has nothing to say in the +// console, so anything there is a defect (a broken sub-resource, a rejected attribute, an engine +// refusing a feature) that no assertion looks for. +// +// One module-level buffer is enough: a Playwright worker runs one test at a time, so the reset at +// setup and the assertion at teardown bracket exactly the test in between. +const problems: string[] = []; +const allowed: RegExp[] = []; + +// The one message the stack itself provokes: the runner reaches `web`/`proxy` by container name over +// plain http, and only a `localhost` origin is trustworthy without TLS — so Chromium drops the COOP +// header the app sends and says so on every page. Over https, where a deployment serves, it applies. +const EXPECTED = [/^console\.error: The Cross-Origin-Opener-Policy header has been ignored/]; + +// Allow a message for the current test only, when the page under test provokes it on purpose. +export function allowConsole(...patterns: RegExp[]): void { + allowed.push(...patterns); +} + +function watch(page: Page): void { + page.on("console", (msg) => { + const type = msg.type(); + if (type === "error" || type === "warning") problems.push(`console.${type}: ${msg.text()}`); + }); + page.on("pageerror", (err) => problems.push(`pageerror: ${err.message}`)); +} + +// Every page of the context, however it is opened — `context.newPage()` fires this event too, so +// watching the context is the whole job and a page must never be watched a second time on top. +function watchContext(context: BrowserContext): BrowserContext { + context.on("page", watch); + return context; +} + +// A spec that opens its own context — a page shared across a serial describe — goes through this. +export function watchedPage(context: BrowserContext): Promise { + return watchContext(context).newPage(); +} + +export const test = base.extend<{ consoleGuard: void }>({ + context: async ({ context }, use) => { await use(watchContext(context)); }, + consoleGuard: [async ({}, use) => { + problems.length = 0; + allowed.length = 0; + await use(); + const unexpected = problems.filter((p) => ![...EXPECTED, ...allowed].some((re) => re.test(p))); + expect(unexpected, "the browser logged nothing while this test ran").toEqual([]); + }, { auto: true }], +}); + +export { expect }; diff --git a/e2e-tests/devstack-login.spec.ts b/e2e-tests/devstack-login.spec.ts index b01d830..820165c 100644 --- a/e2e-tests/devstack-login.spec.ts +++ b/e2e-tests/devstack-login.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./console-guard.ts"; // Regression: the from-scratch dev experience the README/banner advertises must work. `docker compose // up`, open the printed login URL (http://localhost:3000), sign in as the seeded admin → you land on diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index 141ef8a..c6fe508 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -1,4 +1,5 @@ -import { type Browser, type Page, expect, test } from "@playwright/test"; +import type { Browser, Page } from "@playwright/test"; +import { expect, test, watchedPage } from "./console-guard.ts"; import { randomUUID } from "node:crypto"; // Full browser E2E: the real Playwright UI against the live stack via the same-origin @@ -27,7 +28,7 @@ async function loginPassword(page: Page): Promise { // The themed Kratos page in another language: our own chrome, Kratos' own strings mapped by id, and // the card's own links keeping the choice (they are rendered by the flow body, not by the menu). test("the login page speaks the visitor's language, links included", async ({ browser }) => { - const page = await (await browser.newContext()).newPage(); + const page = await watchedPage(await browser.newContext()); await page.goto("/login?locale=sv-SE"); await expect(page.locator("html")).toHaveAttribute("lang", "sv-SE"); await expect(page.getByRole("heading", { name: "Logga in" })).toBeVisible(); @@ -43,7 +44,7 @@ test.describe.serial("authenticated admin journey", () => { test.beforeAll(async ({ browser: b }) => { browser = b; - page = await (await browser.newContext()).newPage(); + page = await watchedPage(await browser.newContext()); test.setTimeout(90_000); await loginPassword(page); }); diff --git a/e2e-tests/language.spec.ts b/e2e-tests/language.spec.ts index a48abdd..0e2faf2 100644 --- a/e2e-tests/language.spec.ts +++ b/e2e-tests/language.spec.ts @@ -1,7 +1,6 @@ import { readFileSync } from "node:fs"; import { createPrivateKey, sign } from "node:crypto"; -import { mkdir } from "node:fs/promises"; -import { expect, test } from "@playwright/test"; +import { expect, test, watchedPage } from "./console-guard.ts"; // Language switching in a real browser, Ory-free (the visual stack). Proves the whole path a // visitor takes: pick a language, read the page in it, and stay in it while clicking around — @@ -9,7 +8,6 @@ import { expect, test } from "@playwright/test"; const BASE_URL = process.env.BASE_URL ?? "http://localhost:3000"; const SESSION_COOKIE = "plainpages_jwt"; -const SHOTS = "artifacts/screenshots"; // Same trick as visual.spec.ts: sign a session JWT with the committed dev tokenizer key so the // gated pages render without standing up Ory. @@ -38,8 +36,7 @@ test("the switcher changes language, and the choice survives clicking through th await expect(page).toHaveURL(/locale=sv-SE/); await expect(page.getByRole("heading", { name: "Startpanel" })).toBeVisible(); // the starter dashboard, in Swedish await expect(page.getByRole("link", { name: "Översikt", exact: true })).toBeVisible(); // the menu too - await mkdir(SHOTS, { recursive: true }); - await page.screenshot({ fullPage: true, path: `${SHOTS}/live-05-swedish.png` }); + await page.screenshot({ fullPage: true, path: `artifacts/screenshots/${test.info().project.name}/live-05-swedish.png` }); // Clicking a menu item keeps Swedish — the host carries the choice onto the links it renders, // and the plugin's own page is translated from its own catalog. The section's own label comes @@ -66,7 +63,7 @@ test("the switcher changes language, and the choice survives clicking through th test("a browser that asks for Swedish gets it without touching the URL", async ({ browser }) => { const context = await browser.newContext({ locale: "sv" }); // a browser set to Swedish, no region - const page = await context.newPage(); + const page = await watchedPage(context); await page.goto(`${BASE_URL}/`); await expect(page.locator("html")).toHaveAttribute("lang", "sv-SE"); diff --git a/e2e-tests/oauth-login.spec.ts b/e2e-tests/oauth-login.spec.ts index 3c1973f..bfe2e33 100644 --- a/e2e-tests/oauth-login.spec.ts +++ b/e2e-tests/oauth-login.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./console-guard.ts"; // Full-stack OAuth2 login + consent E2E: another app logs in *through* plainpages. Hydra // starts an authorization flow and hands the browser to web's /oauth2/login; web resolves it via diff --git a/e2e-tests/playwright.config.ts b/e2e-tests/playwright.config.ts index a8bcd92..d9c23fc 100644 --- a/e2e-tests/playwright.config.ts +++ b/e2e-tests/playwright.config.ts @@ -3,6 +3,9 @@ import { defineConfig, devices } from "@playwright/test"; // Visual + functional checks against the live app (the `web` compose service, BASE_URL). Run via // e2e-tests/compose.visual.yml. Parallel per the project's E2E principle; deterministic colorScheme/viewport // so the rendered design is stable across runs. + +const ORY_FREE = /(visual|language)\.spec\.ts$/; + export default defineConfig({ testDir: ".", outputDir: "artifacts/test-output", @@ -15,11 +18,14 @@ export default defineConfig({ screenshot: "only-on-failure", viewport: { width: 1280, height: 800 }, }, - // CSS anchor positioning is the newest platform feature in the app and every popup menu rests on - // it, so the tests tagged @engines run in all three engines; the rest stay on chromium. + // The Ory-free suites run in all three engines: the console guard (console-guard.ts) only sees an + // engine's warnings when that engine renders the page, and the newest platform features in the app + // (popover, CSS anchor positioning, `:has()`) are exactly where engines disagree. They stay + // side-effect-free, so three parallel runs of them don't collide. The Ory-backed suites write to + // one shared backend and stay on chromium. projects: [ { name: "chromium", use: { ...devices["Desktop Chrome"] } }, - { name: "firefox", grep: /@engines/, use: { ...devices["Desktop Firefox"] } }, - { name: "webkit", grep: /@engines/, use: { ...devices["Desktop Safari"] } }, + { name: "firefox", testMatch: ORY_FREE, use: { ...devices["Desktop Firefox"] } }, + { name: "webkit", testMatch: ORY_FREE, use: { ...devices["Desktop Safari"] } }, ], }); diff --git a/e2e-tests/visual.spec.ts b/e2e-tests/visual.spec.ts index 317f188..90e177b 100644 --- a/e2e-tests/visual.spec.ts +++ b/e2e-tests/visual.spec.ts @@ -1,14 +1,14 @@ import { createPrivateKey, sign } from "node:crypto"; import { readFileSync } from "node:fs"; -import { mkdir } from "node:fs/promises"; -import { expect, test, type Page } from "@playwright/test"; +import type { Page } from "@playwright/test"; +import { allowConsole, expect, test } from "./console-guard.ts"; -const SHOTS = "artifacts/screenshots"; const BASE_URL = process.env.BASE_URL ?? "http://localhost:3000"; const SESSION_COOKIE = "plainpages_jwt"; // src/auth/login.ts — web verifies it against the committed dev JWKS +// Per engine: the three projects run this suite in parallel and would otherwise write one file. const shot = (page: Page, name: string): Promise => - page.screenshot({ fullPage: true, path: `${SHOTS}/${name}.png` }); + page.screenshot({ fullPage: true, path: `artifacts/screenshots/${test.info().project.name}/${name}.png` }); // Sign a session JWT with the committed dev tokenizer key (bind-mounted at /repo/jwks.json), so the // gated dashboard renders for a "signed-in" user without standing up Ory — web verifies it @@ -22,8 +22,6 @@ function devSession(permissions: string[] = []): string { return `${input}.${sign("SHA256", Buffer.from(input), { dsaEncoding: "ieee-p1363", key }).toString("base64url")}`; } -test.beforeAll(async () => { await mkdir(SHOTS, { recursive: true }); }); - // The dashboard is gated: a page navigation needs a session. Plant one per test — a plain // member (no permissions) so the gated scheduling nav stays filtered out. test.beforeEach(async ({ context }) => { @@ -74,7 +72,7 @@ test("theme switch flips the palette with no JavaScript", async ({ page }) => { // longer has to click the trigger again to get rid of one. Driven through the language picker; the // profile menu is the same block. Anchoring is asserted too — without `position-anchor` the panel // silently detaches and lands in the middle of the viewport. -test("a popover menu sits on its trigger and closes on an outside click or Esc — no JavaScript @engines", async ({ page }) => { +test("a popover menu sits on its trigger and closes on an outside click or Esc — no JavaScript", async ({ page }) => { await page.goto("/dashboard"); const trigger = page.locator('button[aria-label="Language"]'); const panel = page.locator('button[aria-label="Language"] + .menu-pop'); @@ -138,6 +136,7 @@ test("the public landing at / is ungated and links to sign in + register", async }); test("unknown routes serve the 404 page (a real user-facing flow, covered end-to-end)", async ({ page }) => { + allowConsole(/status of 404/); // Chromium and WebKit log the status of the navigation under test const res = await page.goto("/no-such-page"); expect(res?.status()).toBe(404); await expect(page.getByRole("heading", { name: "Page not found" })).toBeVisible(); diff --git a/src/e2e-console-guard.test.ts b/src/e2e-console-guard.test.ts new file mode 100644 index 0000000..19d6dc4 --- /dev/null +++ b/src/e2e-console-guard.test.ts @@ -0,0 +1,34 @@ +// Guards the E2E console guard: a spec that imports `test` straight from Playwright runs unwatched, +// and a browser warning nobody looks at is exactly what the guard exists to catch — so the wiring is +// asserted here, in the fast unit gate, rather than discovered by a silent gap in an E2E run. A text +// guard: @playwright/test is installed in the e2e-tests image, not in the one running these tests. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync, readdirSync } from "node:fs"; + +const read = (p: string) => readFileSync(new URL(`../e2e-tests/${p}`, import.meta.url), "utf8"); +const specs = readdirSync(new URL("../e2e-tests/", import.meta.url)).filter((f) => f.endsWith(".spec.ts")); + +test("every spec takes its `test` from the console guard, never straight from Playwright", () => { + assert.ok(specs.length >= 5, "scans the E2E specs"); + for (const spec of specs) { + const source = read(spec); + assert.match(source, /^import \{[^}]*\btest\b[^}]*\} from "\.\/console-guard\.ts";$/m, `${spec} imports test from the guard`); + assert.doesNotMatch(source, /^import \{(?![^}]*\btype\b)[^}]*\} from "@playwright\/test";$/m, `${spec} imports no value from @playwright/test`); + } +}); + +test("the guard reads console errors, warnings and uncaught page errors, and fails on what it kept", () => { + const guard = read("console-guard.ts"); + assert.match(guard, /type === "error" \|\| type === "warning"/); + assert.match(guard, /page\.on\("pageerror"/); + assert.match(guard, /expect\(unexpected, .*\)\.toEqual\(\[\]\)/); +}); + +test("the Ory-free specs run in all three engines, so each engine's console is read", () => { + const config = read("playwright.config.ts"); + for (const engine of ["firefox", "webkit"]) { + assert.match(config, new RegExp(`name: "${engine}", testMatch: ORY_FREE`), `${engine} runs the Ory-free specs`); + } + assert.match(config, /const ORY_FREE = \/\(visual\|language\)\\\.spec\\\.ts\$\//); +}); diff --git a/todo.md b/todo.md index 066505e..5683e4d 100644 --- a/todo.md +++ b/todo.md @@ -2,7 +2,6 @@ ## Unfinnished work -- [ ] In Playwright tests, check for warnings and errors in all browsers on all the steps. If they exist, that is a failure we need to fix. - [ ] In Playwright tests, try different resolutions and sizes, from BIG desktop down to tiny phone. - [ ] Record the browser floor Plainpages actually requires, and whether the fallback is the contract or a courtesy. The stylesheet already needs `:has()` (Dec 2023); the menus now need the popover API (Safari 17, Sep 2023) and CSS anchor positioning for placement (newer still, and unguarded — the `@supports` test covers popover only). An iPadOS 16 tablet — capped at Safari 16, and exactly the "tablet on a factory floor, old thin client at a reception desk" README → Overview sells the zero-JS stance on — therefore gets panels flowing inline rather than working menus. Either state a supported floor in the README or accept the fallback as the answer for those devices; nobody has rendered that path on real hardware. Raised by the architecture review 2026-08-05. - [ ] Decide whether the profile dropdown still earns a dropdown. With the dead Profile link gone it holds one item, Sign out, behind a click — and its "Signed in as X" head only repeats the name and email the trigger already shows. Either put Sign out in the footer directly, or give the menu a second reason to exist. Overlaps the outside-click item above. Raised by review 2026-08-05. @@ -27,6 +26,7 @@ Prioritized. Overall verdict: architecture is sound (contract-first plugin API, ## Finnished work +- [x] In Playwright tests, check for warnings and errors in all browsers on all the steps. If they exist, that is a failure we need to fix. (Every spec takes its `test` from `e2e-tests/console-guard.ts`, which watches every page a test opens — `console.error`, `console.warning`, and uncaught page errors — and fails the test that provoked one, at whatever step. The bar is zero rather than a curated tolerance list: the app ships no client JavaScript, so a message means a broken sub-resource, a rejected attribute, or an engine refusing a feature. Two narrow escapes, both explicit: the COOP header Chromium drops because the e2e stacks serve plain http over container hostnames (a deployment serves https, where it applies), and per-test `allowConsole(/…/)` — used once, by the 404 spec, whose own navigation Chromium and WebKit log. **All browsers** is now literal for the Ory-free suites: `visual.spec.ts` + `language.spec.ts` run in Chromium, Firefox *and* WebKit — the per-test `@engines` tag is gone, and screenshots are written per project so the three don't fight over one file — which is what makes an engine-specific message visible at all. The Ory-backed suites write users, groups and sessions to one shared backend, so they stay on Chromium; widening them needs a stack per engine. Nothing in the app had to be fixed: the sweep found only the two above. Verified by negative control — an injected `console.warn` failed the test in all three engines and an injected `console.error` failed on full-flow's shared serial page — which also caught the guard registering that page twice. `src/e2e-console-guard.test.ts` locks the wiring in the *unit* gate, since a spec importing `test` straight from Playwright would run unwatched and green.) - [x] Don't run tests when only markdown files in the root have changed. (Already shipped for *any* `*.md`, anywhere in the tree — `ci.sh`'s `docs_only()` no-ops the gate when every path changed since `main` ends in `.md`, and the workflow still pushes the commit-hash image so a merged docs commit stays releasable. Kept wider than "in the root" deliberately: no test reads a markdown file, so a nested `examples/plugins/admin/README.md` edit is as safe to skip as `README.md`, and narrowing it would spend the full gate on one. What was actually broken was rename detection — `git mv src/app.ts notes.md` names only the destination under `git diff --name-only`, and collapses to a single `R src/app.ts -> notes.md` line under `git status --porcelain`, so **moving code onto a `.md` path skipped the gate over a source file that was gone**. Both channels now pass `--no-renames`; verified against a scratch repo across ten scenarios — docs-only, mixed, empty diff, dirty tree, untracked code, deleted doc, and the rename staged *and* committed — the last two failing before the fix and passing after. `src/ci-gate.test.ts` locks both flags; it stays a text guard because the test image is `node:alpine` with neither `git` nor `bash`.) - [x] The little menues, like when choosing language or clicking my username, they do not dissapear when clicking outside them, I must click the original trigger or choose something. See if there are more modern ways of handling this with HTML and CSS. I think there is a modal-thing or something? (The modern thing is the **Popover API**. All three popup menus — language picker, profile, row kebab — are now a `