Fail an E2E test on anything the browser logs, in all three engines #56

Merged
lilleman merged 3 commits from e2e-console-guard into main 2026-08-05 11:30:12 +02:00
6 changed files with 22 additions and 11 deletions
Showing only changes of commit 690d67728b - Show all commits
+7 -2
View File
@@ -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
+9 -5
View File
@@ -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<Page> {
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 }],
});
+1 -1
View File
@@ -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: ".",
+1 -1
View File
@@ -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();
+2 -1
View File
@@ -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\$\//);
});
+2 -1
View File
@@ -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 `<button popovertarget>` plus a `[popover]` panel instead of `<details>`/`<summary>`, so the browser owns open/close: clicking anywhere outside dismisses one, `Esc` dismisses it and returns focus to the trigger, opening one closes the others, and the panel sits in the top layer where `.table-wrap`'s `overflow` can no longer clip a row kebab. Placement is CSS anchor positioning; the panel needs `position-anchor: auto` to bind to the button that opened it — a bare `anchor()` resolves to nothing in Chromium, Firefox and WebKit alike, measured in all three before picking the approach. `data-table.ejs` stopped hand-rolling its kebab and calls the `menu` partial, so the pattern lives in one file. Each panel is named by its caller (`locale-menu`, `profile-menu`, `row-actions-1`) and the partial fails loud without an `id`, since `popovertarget` is an idref — generated ids were tried first and dropped for being unreadable and nondeterministic. `<details>` stays in the nav tree, where it means disclosure rather than popup. A browser older than the popover API flows each panel inline under its trigger, so Sign out is never stranded behind an inert button. `e2e-tests/visual.spec.ts` drives the whole behaviour — opens, anchored to its trigger, outside-click, Esc — and is tagged `@engines` so it runs in Firefox and WebKit as well as Chromium, because CSS anchor positioning is the newest thing in the app and every popup rests on it. Decisions recorded in AGENTS.md.)
- [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 `<button popovertarget>` plus a `[popover]` panel instead of `<details>`/`<summary>`, so the browser owns open/close: clicking anywhere outside dismisses one, `Esc` dismisses it and returns focus to the trigger, opening one closes the others, and the panel sits in the top layer where `.table-wrap`'s `overflow` can no longer clip a row kebab. Placement is CSS anchor positioning; the panel needs `position-anchor: auto` to bind to the button that opened it — a bare `anchor()` resolves to nothing in Chromium, Firefox and WebKit alike, measured in all three before picking the approach. `data-table.ejs` stopped hand-rolling its kebab and calls the `menu` partial, so the pattern lives in one file. Each panel is named by its caller (`locale-menu`, `profile-menu`, `row-actions-1`) and the partial fails loud without an `id`, since `popovertarget` is an idref — generated ids were tried first and dropped for being unreadable and nondeterministic. `<details>` stays in the nav tree, where it means disclosure rather than popup. A browser older than the popover API flows each panel inline under its trigger, so Sign out is never stranded behind an inert button. `e2e-tests/visual.spec.ts` drives the whole behaviour — opens, anchored to its trigger, outside-click, Esc — and runs in Firefox and WebKit as well as Chromium, because CSS anchor positioning is the newest thing in the app and every popup rests on it. Decisions recorded in AGENTS.md.)
- [x] Organize the files in src in to folders so it is easier to understand the structure of the code.
- [x] Move docs/plugin-contract.md into README.md and remove the docs folder.
- [x] The plugins/scheduling is an example and shouldn't be committed to the plugins directory since that should be empty to be able to be mounted in via docker or other means for the users/develoeprs using this application/framework. Put it in the examples folder instead.