diff --git a/AGENTS.md b/AGENTS.md index 89397e2..c197024 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -221,8 +221,13 @@ them. Revisit only if the stated reason stops holding. 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. + navigation Chromium and WebKit log. Each record carries the message's origin URL, so that allowance + can name the page under test and still see a sub-resource of it 404. `src/e2e-console-guard.test.ts` + locks the wiring in the *unit* gate: a spec importing `test` straight from Playwright — or minting a + page with a raw `newPage()` instead of `watchedPage()` — would run unwatched and green. The buffer + clears at teardown rather than setup so a `beforeAll` is watched too (full-flow runs a whole login in + one); the accepted cost is that a page outliving its test, as a serial describe's does, can log late + and fail the next test instead of its own. Verified by negative control in all three engines. - **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 diff --git a/e2e-tests/console-guard.ts b/e2e-tests/console-guard.ts index 2c654c6..ef9389f 100644 --- a/e2e-tests/console-guard.ts +++ b/e2e-tests/console-guard.ts @@ -5,8 +5,10 @@ import { expect, test as base, type BrowserContext, type Page } from "@playwrigh // 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. +// One module-level buffer is enough: a Playwright worker runs one test at a time. It is cleared at +// teardown, not at setup, so what a `beforeAll` provoked — full-flow's whole login runs in one — +// still lands on the first test rather than being wiped before it. The cost of the same choice: a +// page that outlives its test (a serial describe's) can log late and fail the next test instead. const problems: string[] = []; const allowed: RegExp[] = []; @@ -23,7 +25,9 @@ export function allowConsole(...patterns: RegExp[]): void { function watch(page: Page): void { page.on("console", (msg) => { const type = msg.type(); - if (type === "error" || type === "warning") problems.push(`console.${type}: ${msg.text()}`); + // The origin is part of the record: a 404 reads the same whether it was the page or its + // stylesheet, and a failure nobody can locate is half a failure. + if (type === "error" || type === "warning") problems.push(`console.${type}: ${msg.text()} @ ${msg.location().url}`); }); page.on("pageerror", (err) => problems.push(`pageerror: ${err.message}`)); } @@ -43,10 +47,10 @@ export function watchedPage(context: BrowserContext): Promise { 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))); + problems.length = 0; + allowed.length = 0; expect(unexpected, "the browser logged nothing while this test ran").toEqual([]); }, { auto: true }], }); diff --git a/e2e-tests/playwright.config.ts b/e2e-tests/playwright.config.ts index d9c23fc..3476741 100644 --- a/e2e-tests/playwright.config.ts +++ b/e2e-tests/playwright.config.ts @@ -4,7 +4,7 @@ import { defineConfig, devices } from "@playwright/test"; // 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$/; +const ORY_FREE = /\/(visual|language)\.spec\.ts$/; export default defineConfig({ testDir: ".", diff --git a/e2e-tests/visual.spec.ts b/e2e-tests/visual.spec.ts index 90e177b..1048cae 100644 --- a/e2e-tests/visual.spec.ts +++ b/e2e-tests/visual.spec.ts @@ -136,7 +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 + allowConsole(/status of 404 .*\/no-such-page$/); // the navigation under test, which Chromium and WebKit log — not a sub-resource of it 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 index 19d6dc4..083a698 100644 --- a/src/e2e-console-guard.test.ts +++ b/src/e2e-console-guard.test.ts @@ -15,6 +15,7 @@ test("every spec takes its `test` from the console guard, never straight from Pl 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`); + assert.doesNotMatch(source, /\.newPage\(/, `${spec} takes its page from the fixture or watchedPage(), never a raw newPage()`); } }); @@ -30,5 +31,5 @@ test("the Ory-free specs run in all three engines, so each engine's console is r 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\$\//); + assert.match(config, /const ORY_FREE = \/\\\/\(visual\|language\)\\\.spec\\\.ts\$\//); }); diff --git a/todo.md b/todo.md index 5683e4d..56b54c9 100644 --- a/todo.md +++ b/todo.md @@ -3,6 +3,7 @@ ## Unfinnished work - [ ] In Playwright tests, try different resolutions and sizes, from BIG desktop down to tiny phone. +- [ ] Decide whether `e2e-tests/` should be typechecked. It is outside `tsconfig.include`, so the gate never checks the most logic-bearing file in it (`console-guard.ts`) — Playwright strips its types without checking them. Including it needs the DOM lib and `@playwright/test` present wherever `npm run typecheck` runs, which today is the `web` image that installs neither. Raised by review 2026-08-05. - [ ] 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. - [ ] When copy+paste the verification code from the email, it doesn't work because it does not trim whitechars around the code in the form. It should trim automatically. @@ -28,7 +29,7 @@ Prioritized. Overall verdict: architecture is sound (contract-first plugin API, - [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 `