From 0edae32404a945d3a1e9fe2dc8f803b0f7b7ed5b Mon Sep 17 00:00:00 2001 From: Timur Mashara <62305878+tmashara@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:44:24 +0100 Subject: [PATCH] Document ending a hook by waiting for the state it opened A click that lands but does nothing shoots the closed state on both sides, and they match. A hook that ends with locator.waitFor turns that into a route error naming the side, through the existing hook-error path. Co-Authored-By: Claude Opus 5.5 --- docs/scoping.md | 19 +++++++++++++++++-- skills/renderdiff/SKILL.md | 2 +- skills/renderdiff/reference.md | 6 +++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/docs/scoping.md b/docs/scoping.md index c30a43a..0ad519c 100644 --- a/docs/scoping.md +++ b/docs/scoping.md @@ -71,7 +71,13 @@ it, with the clock and seeded `Math.random` already in force: ```ts export default { routes: [ - { left: "/", before: async (page) => { await page.click("[aria-label=Menu]"); } }, + { + left: "/", + before: async (page) => { + await page.click("[aria-label=Menu]"); + await page.locator("nav.menu[data-open]").waitFor({ timeout: 5_000 }); + }, + }, { left: "/docs", beforeShot: async (page) => { await page.click("[data-tab=intro]"); } }, ], // For every route without its own `before` (which replaces it), CLI routes included; @@ -90,7 +96,16 @@ is a route error, `before hook failed on left: …` (or `beforeShot …`), and i A route that ran one has `"hook": true` in `results.json` and a `hook` badge on its report card. Actions in a hook use Playwright's 30s default timeout, so check before clicking what may -not be there (as above); `hideSelectors`, `extraCss` and `lottieFrame` apply after `before` and before `beforeShot`. +not be there, but only for optional things like the cookie banner above. + +A click that lands but does nothing (before the page hydrates, or on the wrong element) +shoots the closed state, and both sides match. So don't guard the state under test: end the +hook by waiting for it, as the menu route does, with a selector that matches one element +(`waitFor` is strict; add `.first()` otherwise). A miss is a route error on that side, +`before hook failed on left: locator.waitFor: Timeout 5000ms exceeded…`. If the settle's +scroll pass could undo the state, move the hook to `beforeShot`, below. + +`hideSelectors`, `extraCss` and `lottieFrame` apply after `before` and before `beforeShot`. The settle scrolls the whole page to load lazy content, so a state the scroll changes (a scroll-spy nav, a container re-measured on scroll) doesn't survive a `before` hook. Set it in diff --git a/skills/renderdiff/SKILL.md b/skills/renderdiff/SKILL.md index 05a378d..2efe400 100644 --- a/skills/renderdiff/SKILL.md +++ b/skills/renderdiff/SKILL.md @@ -19,7 +19,7 @@ the § named at each step. | Change | Run | | --- | --- | -| CSS only, markup untouched (refactor, dead-CSS removal, token swap) | pixels + `--computed`. Expect `dom identical` on every route: then each `computed:` finding is a real style change, even on a clean route. `dom differs`: often a wrapper's class changed, re-keying its subtree (§ `--computed`). Hover menus and open menus aren't shot without a `before` hook, and dead-CSS removal breaks exactly those. | +| CSS only, markup untouched (refactor, dead-CSS removal, token swap) | pixels + `--computed`. Expect `dom identical` on every route: then each `computed:` finding is a real style change, even on a clean route. `dom differs`: often a wrapper's class changed, re-keying its subtree (§ `--computed`). Hover menus and open menus aren't shot without a `before` hook that ends waiting for the open state, and dead-CSS removal breaks exactly those. | | Markup changes (framework, template or component migration) | pixels, plus `--aria`: `aria: identical` means a screen reader gets the same page. `--computed` findings would only be hints. | | Dependency or build upgrade | pixels, plus `--computed` if the markup should be identical. | | One component | pixels with `--selector ` or `'/route @sel'`. | diff --git a/skills/renderdiff/reference.md b/skills/renderdiff/reference.md index 37c0a20..eea13de 100644 --- a/skills/renderdiff/reference.md +++ b/skills/renderdiff/reference.md @@ -128,7 +128,7 @@ Report (`--open`, for a human): green from 99.5%, amber to 97%, red below. | `page is Npx tall at Wpx wide, past the …px sanity ceiling …` | Taller than 57,600,000px ÷ width (never under 40,000px): a runaway layout, not a page. A long page still compares one element with `--selector` or `@sel` (`element is …` when that element itself is past it). | | `selector "…" matched N elements on left (/a), expected 1` | Make it unique (an id, `data-test`, `>> nth=0`), or list the route once per element. | | `selector "…" has no box on …` / `is off the page on …` | Matched an element not rendered (`display: none`, zero size) or outside the page. | -| `before hook failed on left: …` | The hook threw (`beforeShot hook failed …` for that hook). Never retried. | +| `before hook failed on left: …` | The hook threw (`beforeShot hook failed …` for that hook). Never retried. `locator.waitFor: Timeout` there means the state the hook waits for wasn't reached on that side. | | `timeout after 120s at settle on left` | That side never finished the step (`load`, `before hook`, `settle`, `beforeShot hook`, `screenshot`): a hung script, a dead renderer, a hook that never resolves. Retried once in a fresh page, except a hung hook. For a page that is just very slow, raise `RENDERDIFF_SHOT_TIMEOUT` (seconds). | A failed `--computed` snapshot (`snapshot failed on /a@1440 (…)` on the `computed:` line) is @@ -295,6 +295,10 @@ aria: identical on 2 of 3 route(s), differs /pricing banner *both* sides show, sign in, set a carousel to slide 1 on both. Not: branching on `side` to make the sides look alike, hiding or removing an element on one side. `side` is for markup that legitimately differs. Say in the verdict that a hook ran (`hook: true`). + End the hook by waiting for the state it opened, or a click that did nothing shoots the + closed state on both sides and matches: + `await page.locator(".menu[data-open]").waitFor({ timeout: 5_000 })` (strict: a unique + selector, or `.first()`). Guard with `isVisible()` only optional things like a banner. `beforeShot` (same forms) runs after settle, right before masks and the shot: for state the settle's scroll pass changes, e.g. a scroll-spy nav. Nothing settles after it, so await it. - **Signed-in pages:** `npx playwright codegen --save-storage=auth.json `, then