diff --git a/README.md b/README.md index 310e592..f792dd4 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,8 @@ renderdiff [options] | `--left-selector `, `--right-selector ` | That side's selector, over `--selector`. One without the other side (or `--selector`) is an error. | | `--storage-state ` | Start both sides with this Playwright storage state (cookies, localStorage), e.g. signed in. Relative to cwd. Over the config's `storageState`. | | `--left-storage-state `, `--right-storage-state ` | That side's storage state, over `--storage-state`. One side alone is fine. | +| `--accept ` | Record this run's differences as accepted, with a reason. See **Accepted differences and reruns**. | +| `--rerun-failed` | Rerun only what the last run in `--out` didn't find clean or accepted, keeping the rest. | | `--dry-run` | Print the URL pairs and exit. No browser. | | `--open` | Open the report when the run finishes. | | `--progress` | A line per shot as it finishes, then the worst scores. The default on a terminal; piped, renderdiff prints one compact summary at the end. | @@ -181,10 +183,31 @@ only. | `dom sel` | On both sides and unchanged in style or position, so its own pixels changed (an image or canvas). | | `crops …` | Up to 3 PNGs per comparison, bands first, relative to `results.json`. Left is on top or on the left, and the change is boxed in red. | | `ERROR …` | Unmeasured, which is neither a pass nor a fail. Exits 3. | +| `accepted:` / `stale accepted:` | Shots whose every difference was accepted, and accepted entries the run no longer saw. See below. | +| `rerun:` | With `--rerun-failed`: how many shots were rerun and how many reused. | Read text first, then `dom`, then the crops for anything `dom` doesn't explain. Full detail, including the `--progress` and report forms: [docs/output.md](https://github.com/tmashara/renderdiff/blob/main/docs/output.md). +## Accepted differences and reruns + +A migration starts with differences that already exist between the untouched local build +and production. Record them once, then change code and re-check only what's red: + +``` +renderdiff http://localhost:3000 https://example.com / /about --accept "differs before the migration" +# …change code… +renderdiff http://localhost:3000 https://example.com / /about --rerun-failed +``` + +`--accept ""` writes every difference the run saw to `renderdiff.accepted.json` (next +to the config, or in cwd); commit it. Later, a shot whose every difference matches an entry is +listed on an `accepted:` line and counts neither as differing nor toward `--threshold`; +anything new on the same page still shows. `--rerun-failed` reruns only what the last run in +`--out` didn't find clean or accepted and merges it back; it exits 2 if the run differs from +the previous one. Matching rules, what can still hide, and stale entries: +[docs/accepted.md](https://github.com/tmashara/renderdiff/blob/main/docs/accepted.md). + ## Output Everything lands in the out dir (`./renderdiff-out` by default): @@ -243,7 +266,9 @@ Everything lands in the out dir (`./renderdiff-out` by default): "summary": { "compared": 1, "errors": 1, "mean": 0.9714, "worst": { "url": "/about", "width": 1440, "score": 0.9714 }, - "belowThreshold": 0 + "belowThreshold": 0, // accepted shots aside + "accepted": 0, // shots whose every difference was accepted + "stale": [] // accepted entries the run measured but didn't see } } ``` diff --git a/docs/accepted.md b/docs/accepted.md new file mode 100644 index 0000000..0f3fb1c --- /dev/null +++ b/docs/accepted.md @@ -0,0 +1,58 @@ +# Accepted differences and reruns + +A migration starts with differences that already exist between the untouched local build +and production. Record them once, then change code and re-check only what's red: + +``` +renderdiff http://localhost:3000 https://example.com / /about --accept "differs before the migration" +# …change code… +renderdiff http://localhost:3000 https://example.com / /about --rerun-failed +``` + +`--accept ""` runs as usual, then writes every difference it saw that isn't accepted +yet to `renderdiff.accepted.json`, next to the config file (or in cwd without one), each with +the reason. Scope it the way you scope any run: pass only the routes and `--width`s to accept. +Commit the file; it is one entry per line, so its diff reads as what was accepted. + +A shot whose every difference matches an accepted one is `accepted`: it is listed on an +`accepted:` line like `clean:`, doesn't count as differing or toward `--threshold`, and has +`"accepted": true` in `results.json` and a badge in the report. Anything else on the page +still shows: a row with some changes accepted says `1 accepted`, and its `dom` line skips +them. A shot with unstable pixels is never accepted, since part of it went unmeasured. +`summary.mean` and `summary.worst` leave accepted shots out, like the verdict line. +Accepted entries for a route and width the run measured but didn't see are **stale**: they +print on a `stale accepted:` line, sit in `summary.stale` and the report, and the next +`--accept` over that route and width prunes them. + +Each entry is one change, a band or a region, keyed by route (as the summary names it) and +width, with a fingerprint. Two changes match when they have the same kind and side, the same +`x`, width and height (regions sit on the 16px grid, bands are exact rows), the same hash of +every pixel under the change on both sides at 4 bits a channel, and, when the entry named +elements, the same `dom` elements with the same changed values. That makes a match: + +- robust to a change above it moving it down the page (`y` is left out), to unrelated + changes elsewhere (they are their own changes, and stay unaccepted), and to shade jitter + that stays within one of sixteen steps per channel; +- strict about the spot itself: any other change to what is drawn there, a change touching + it (the region grows), or a second identical change (an entry matches once) all surface. + +The cost is false alarms rather than hidden changes. Noise that crosses a 4-bit step, an +element probe that times out, or a new change that makes repeated identical rows align +differently (turning an accepted region into bands) makes an accepted change show again; +accept it again once you've looked. What can still hide is a shade change under one step +(e.g. `#000` to `#080808`) that doesn't cross a step boundary anywhere in the change. + +A shot whose pages differ in width, or in height without the rows aligning into bands, is +never accepted: the padding past the shorter page belongs to no band or region, so no +fingerprint covers it. + +`--rerun-failed` reads `results.json` in `--out`, keeps every shot that was clean or is +accepted (against the accepted file as it is now), reruns the rest — differences, unstable +shots, errors — and merges them back into `results.json` and `report.html` in route and width +order. The out dir is kept rather than wiped; only a rerun shot's own old files are replaced. +A `rerun:` line says how many shots it reran on how many routes, and how many it reused. +`meta` describes the rerun (its clock, browser and time); a reused shot was taken under the +earlier run's. It must repeat the previous run: different origins, routes, widths, config +(`meta.config` hashes masks, `before` hooks and `locate` sources, hidden selectors, block +patterns, extra CSS and storage state file names) or renderdiff version exit 2, as does a +missing `results.json`. diff --git a/skills/renderdiff/SKILL.md b/skills/renderdiff/SKILL.md index 5ec60bf..0757490 100644 --- a/skills/renderdiff/SKILL.md +++ b/skills/renderdiff/SKILL.md @@ -18,7 +18,8 @@ legitimate: [reference.md](reference.md). ## Loop 1. **Baseline first.** Before changing code, run the unmodified build against the reference - (`--out renderdiff-baseline`). Differences already there are not yours. + with `--accept "differs before the migration"`: what already differs is recorded, not + yours. Accept only what you looked at, never to get green. 2. Both origins up, with the same content and data. `--dry-run` shows the URL pairs. 3. One route, one width. A row: score, `h±N`/`w±N` height/width delta, `band`, `aligned`, `N reg`, `unstable Npx` (moving pixels, unmeasured), `text`. @@ -30,8 +31,10 @@ legitimate: [reference.md](reference.md). Bare `sel`: its own pixels changed (image, canvas). 7. `Read` the crops under each row (next to results.json) for what `dom` doesn't explain. Left is on top or left; red box = change, red seam = missing rows. -8. Fix or justify every band, region and text line. Re-run after each migration step, then - all routes at default widths; for a multi-width row, open the widest and narrowest crop. +8. Fix or justify every band, region and text line. After each migration step, + `--rerun-failed` (same arguments) reruns only what isn't clean or accepted; finish with + all routes at default widths. For a multi-width row, open the widest and narrowest crop. + `accepted:` routes are known; a row saying `1 accepted` still has something new. Exit: 0 nothing below `--threshold` (**without it, a diff still exits 0**) · 1 below threshold · 2 bad args/config/origin down · 3 a route errored (unmeasured) or crash. diff --git a/skills/renderdiff/reference.md b/skills/renderdiff/reference.md index 1e38bb4..94bdece 100644 --- a/skills/renderdiff/reference.md +++ b/skills/renderdiff/reference.md @@ -24,6 +24,9 @@ Left is your change, right is the reference: `+` is only/more on the left, `-` o | `unstable Npx` | Pixels that kept changing between shots of the same load on one origin (a ticker, a carousel, a video). Painted out on both sides and excluded from the score. A large number means a big area went unmeasured: look at the magenta on the full shots. Such a route is never on the `clean:` line. | | `ERROR selector "…" matched N elements on left (/a), expected 1` | A selector must match exactly one element per side. Make it more specific (`>> nth=0`, an id, `data-test`), or list the route once per element. | | `ERROR selector "…" has no box on …` | It matched an element that isn't rendered (`display: none`, zero size). | +| `accepted: /a /b` | Every difference on those routes matches `renderdiff.accepted.json`. Known, not verified again: say so in the verdict. | +| `N accepted` on a row | Some of that shot's changes are accepted; the rest are new. `dom` names only the new ones. | +| `stale accepted: /a@1440` | An accepted difference wasn't seen. It was fixed, or the page changed under it; `--accept` over that route and width prunes it. | | `ERROR page did not settle: Npx unstable at …` | More than 25% of the page kept changing between shots, so too little was measured to score. Mask or stop the moving part. | Report colouring (`--open`, for a human): green above 99.5%, amber to 97%, red below. @@ -38,6 +41,7 @@ Report colouring (`--open`, for a human): green above 99.5%, amber to 97%, red b | "`dom` says margin, done." | It's a computed value. Confirm the source rule changed on purpose, and still open the crop when the region is large or `dom` names nothing. | | "The crops looked fine, done." | There are at most three: bands, then the largest regions. Check the region count, any `clipped` crop in results.json, and open the diff PNG for the rest and for any height delta: padding is never cropped. | | "Masking the header gets this to 100%." | Every mask is coverage surrendered. Mask only content *proven* to vary per load (clocks, star counts, rotating quotes), with a `reason`, which the report prints verbatim. | +| "I'll `--accept` it so the run is green." | Accept only differences you opened and that exist before your change (or are intended), with a reason that says which. | | "I'll lower `--threshold` until it passes." | Threshold is a CI gate, not a dial for reaching green. | | "That route errors, I'll drop it from the list." | Shrinking the route list shrinks what you verified. Fix it, or say it's unverified. | | "One width is enough." | Migrations break at one width and not others. That's why there are five defaults. | @@ -91,7 +95,16 @@ cookie script ran wedges the renderer). - **stdout is compact when piped** (what an agent sees). `--progress` prints a line per shot instead; `--no-progress` forces the summary on a terminal. -- **The out dir is wiped at the start of every run.** To keep a previous run's evidence +- **Accepted differences** (`--accept ""`) go to `renderdiff.accepted.json` next to the + config (or in cwd), one entry per band or region, scoped by the run's routes and widths. A + match ignores `y` and needs the same box, the same pixel hash and the same named elements, so + anything else drawn in that spot still surfaces. A shot whose widths differ, or whose heights + differ without aligning, is never accepted. A new change can make an accepted one re-read + as different (repeated rows aligning differently): look, then accept again. +- **`--rerun-failed`** keeps the out dir and reruns only what wasn't clean or accepted. Give it + the same origins, routes, widths and config (hooks included) as the run it continues, or it + exits 2. +- **The out dir is wiped at the start of every run** (except `--rerun-failed`). To keep a previous run's evidence next to a new one, pass `--out renderdiff-before` / `--out renderdiff-after`. A directory holding files renderdiff didn't write is refused (exit 2). - **Routes normalize** to no trailing slash; `/` stays `/`. Globs are rejected. diff --git a/src/accept.ts b/src/accept.ts new file mode 100644 index 0000000..edce758 --- /dev/null +++ b/src/accept.ts @@ -0,0 +1,144 @@ +import fs from "node:fs"; +import type { Band, Region } from "./compare.ts"; +import { name } from "./config.ts"; +import { fmtElements } from "./format.ts"; +import type { ComparedResult, RouteResult } from "./report.ts"; + +export const ACCEPTED_FILE = "renderdiff.accepted.json"; + +/** + * One band or region, without its `y`: a change above it moves it down the page without + * changing it. `hash` and `elements` tell a new change in the same spot from the accepted one. + */ +export interface Fingerprint { + kind: "band" | "region"; + side?: "left" | "right"; + /** A region's box; a band's rows are in its hash. */ + x?: number; + w?: number; + h: number; + /** The `dom` of each element under it, sorted; absent when they weren't looked at. */ + elements?: string[]; + /** Both sides' pixels under it, 4 bits a channel. */ + hash?: string; +} + +/** A difference recorded as already there or intended, so later runs report it as accepted. */ +export interface Accepted { + /** The route as the summary names it: `/a`, `/a::/b`, `/a [.sel]`. */ + route: string; + width: number; + reason: string; + fingerprint: Fingerprint; +} + +type Change = { fp: Fingerprint; target: Region | Band }; + +function changes(r: ComparedResult): Change[] { + const els = (g: Region | Band) => + g.elements && { elements: g.elements.map((e) => fmtElements([e], Infinity)).sort() }; + return [ + ...(r.bands ?? []).map((b) => ({ + fp: { kind: "band" as const, side: b.side, h: b.h, ...els(b), hash: b.hash }, + target: b, + })), + ...r.regions.map((g) => ({ + fp: { kind: "region" as const, x: g.x, w: g.w, h: g.h, ...els(g), hash: g.hash }, + target: g, + })), + ]; +} + +/** `a` is the accepted entry: elements it recorded must be named again, not merely unprobed. */ +const same = (a: Fingerprint, b: Fingerprint) => + a.kind === b.kind && + a.side === b.side && + a.x === b.x && + a.w === b.w && + a.h === b.h && + a.hash === b.hash && + (!a.elements || a.elements.join("\n") === b.elements?.join("\n")); + +/** + * Padding no band or region covers: columns past the narrower page, or rows past the shorter + * one when they didn't align. A shot with any, or with unstable pixels, is never accepted. + */ +const acceptable = (r: ComparedResult) => + r.unstablePixels === 0 && r.widthDelta === 0 && (r.bands !== null || r.heightDelta === 0); + +export function readAccepted(file: string): Accepted[] { + if (!fs.existsSync(file)) return []; + let list: unknown; + try { + list = JSON.parse(fs.readFileSync(file, "utf8")); + } catch (e) { + throw new Error(`${file} is not valid JSON: ${e instanceof Error ? e.message : e}`); + } + const ok = (a: Record) => + typeof a.route === "string" && + typeof a.width === "number" && + typeof a.reason === "string" && + a.reason.trim() && + typeof a.fingerprint === "object" && + a.fingerprint !== null; + if (!Array.isArray(list) || !list.every(ok)) { + throw new Error(`${file} must be an array of { route, width, reason, fingerprint }`); + } + return list as Accepted[]; +} + +/** + * Mark each change an accepted entry matches, one entry per change, and a result `accepted` + * when every change matched and it is `acceptable`. Returns the changes of acceptable results + * nothing matched, and the entries for a measured route and width that matched nothing. + */ +export function applyAccepted(results: RouteResult[], list: Accepted[]) { + const used = new Set(); + const unmatched: Omit[] = []; + for (const r of results) { + if (r.status !== "compared") continue; + const mine = list.filter((a) => a.route === name(r) && a.width === r.width); + const cs = r.diffPixels > 0 ? changes(r) : []; + let missed = 0; + for (const { fp, target } of cs) { + const hit = mine.find((a) => !used.has(a) && same(a.fingerprint, fp)); + if (hit) { + used.add(hit); + target.accepted = hit.reason; + } else { + delete target.accepted; + missed++; + if (acceptable(r)) unmatched.push({ route: name(r), width: r.width, fingerprint: fp }); + } + } + r.accepted = cs.length > 0 && !missed && acceptable(r); + } + const measured = new Set( + results.filter((r) => r.status === "compared").map((r) => `${name(r)}@${r.width}`), + ); + const stale = list.filter((a) => !used.has(a) && measured.has(`${a.route}@${a.width}`)); + return { unmatched, stale }; +} + +const key = (a: Accepted) => JSON.stringify([a.route, a.width, a.reason, a.fingerprint]); + +/** + * Drop `stale` and add `added` to the file as it is now, so a concurrent `--accept` isn't lost, + * sorted by route then width, through a rename so a crash never leaves it half written. + * Returns false, writing nothing, when there is nothing to change. + */ +export function updateAccepted(file: string, stale: Accepted[], added: Accepted[]): boolean { + if (!stale.length && !added.length) return false; + const drop = new Set(stale.map(key)); + const kept = readAccepted(file).filter((a) => !drop.has(key(a))); + const have = new Set(kept.map(key)); + const list = [...kept, ...added.filter((a) => !have.has(key(a)))].sort((a, b) => + a.route < b.route ? -1 : a.route > b.route ? 1 : a.width - b.width, + ); + // One entry per line, so a diff of the file reads as the differences it adds and drops. + const rows = list.map((a) => ` ${JSON.stringify(a)}`).join(",\n"); + const tmp = `${file}.${process.pid}.tmp`; + fs.writeFileSync(tmp, rows ? `[\n${rows}\n]\n` : "[]\n"); + fs.renameSync(tmp, file); + return true; +} diff --git a/src/cli.ts b/src/cli.ts index eb1355b..a0c7432 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -10,6 +10,8 @@ * Per-site knobs (masks for content that drifts, extra selectors to hide, defaults for * the flags below) live in a `renderdiff.config.ts` / `.mjs` in the working directory. */ +import path from "node:path"; +import { ACCEPTED_FILE } from "./accept.ts"; import { label, loadConfig, @@ -34,6 +36,9 @@ interface Args extends CliOverrides { routes: string[]; /** From --selector / --left-selector / --right-selector; replaces every route's own. */ selector?: Sides; + /** The reason to accept this run's differences with. */ + accept?: string; + rerunFailed: boolean; } /** `Number(raw)`, but reject anything that isn't a finite number with a flag-named error. */ @@ -49,6 +54,7 @@ function parseArgs(argv: string[]): Args { const a: Args = { dryRun: false, open: false, + rerunFailed: false, help: false, version: false, routes: [], @@ -79,7 +85,11 @@ function parseArgs(argv: string[]): Args { else if (v === "--open") a.open = true; else if (v === "--progress") a.progress = true; else if (v === "--no-progress") a.progress = false; - else if (v === "--left") a.left = next(); + else if (v === "--rerun-failed") a.rerunFailed = true; + else if (v === "--accept") { + a.accept = next().trim(); + if (!a.accept) throw new Error("--accept needs a reason"); + } else if (v === "--left") a.left = next(); else if (v === "--right") a.right = next(); else if (v === "--out") a.outDir = next(); else if (v === "--config") a.config = next(); @@ -132,6 +142,9 @@ function parseArgs(argv: string[]): Args { throw new Error(`--${has}-selector needs --${lacks}-selector or --selector`); } if (left && right) a.selector = { left, right }; + if (a.dryRun && (a.rerunFailed || a.accept !== undefined)) { + throw new Error(`${a.rerunFailed ? "--rerun-failed" : "--accept"} needs a run, not --dry-run`); + } if (state.both || state.left || state.right) { a.storageState = { left: state.left ?? state.both, right: state.right ?? state.both }; } @@ -160,6 +173,10 @@ const HELP = ` npx playwright codegen --save-storage=auth.json A before hook in the config runs on each page before it is shot, e.g. to open a menu. + --accept "" records this run's differences in renderdiff.accepted.json (next to + the config, or in cwd); later runs report them as accepted, and anything new still shows. + --rerun-failed reruns only what the previous run in --out didn't find clean or accepted. + Writes report.html, results.json and crops of each change to --out. Exits 1 if a route falls below --threshold, 3 if one errored (unmeasured) or the run crashed, 2 on bad arguments, bad config or an origin that is down. A diff without --threshold exits 0. @@ -179,6 +196,9 @@ const HELP = ` Start both sides with this storage state JSON. Never commit it. --left-storage-state , --right-storage-state That side's storage state, over --storage-state. + --accept Accept this run's differences, scoped by its routes and widths. + --rerun-failed Rerun what the last run in --out didn't find clean or accepted, + keeping the rest. Same origins, routes and widths as that run. --dry-run Print the URL pairs and exit. No browser. --open Open the report when the run finishes. --progress A line per shot as it finishes. The default on a terminal; @@ -304,6 +324,9 @@ try { cfg, open: args.open, progress: args.progress ?? !!process.stdout.isTTY, + accept: args.accept ?? null, + rerun: args.rerunFailed, + acceptedFile: path.join(configPath ? path.dirname(configPath) : process.cwd(), ACCEPTED_FILE), }), ); } catch (e) { diff --git a/src/compare.ts b/src/compare.ts index 1cc86d0..3a696c0 100644 --- a/src/compare.ts +++ b/src/compare.ts @@ -29,6 +29,10 @@ export interface Region { pixels: number; /** The elements under it; absent when not looked at. */ elements?: ElementDiff[]; + /** Both sides' pixels under it, hashed, for matching it against accepted ones. */ + hash?: string; + /** The reason of the accepted difference it matched. */ + accepted?: string; } /** Rows one side has that the other lacks, at `y` in that side's own coordinates. */ @@ -38,6 +42,10 @@ export interface Band { h: number; /** The elements in its rows; absent when not looked at. */ elements?: ElementDiff[]; + /** Its rows' pixels, hashed, for matching it against accepted ones. */ + hash?: string; + /** The reason of the accepted difference it matched. */ + accepted?: string; } /** Left is the changed version, right the reference: `added` is on left only, `removed` on right only. */ @@ -153,6 +161,7 @@ export async function compare(left: Shot, right: Shot): Promise { target: found[i]!, })), ]; + for (const s of spots) s.target.hash = pixelHash(l, r, w, [s.left, s.right]); // Padding is magenta against real content, so a height difference lands in // `diffPixels` and drags the score down. Intended: a page 400px shorter than the // one it's compared against has not reached parity. `heightDelta` names the cause. @@ -464,6 +473,28 @@ function inPlace(p: ReturnType, ha: Uint32Array, hb: Uint32Array) return { bands, at, rows }; } +/** + * FNV-1a over every pixel of the change on both sides, 4 bits a channel, as hex: shade jitter + * under one step survives, any other change to what is drawn there doesn't. + */ +function pixelHash(l: Buffer, r: Buffer, stride: number, boxes: [Box, Box]): string { + let hash = 0x811c9dc5; + for (const [raw, b] of [ + [l, boxes[0]], + [r, boxes[1]], + ] as const) { + for (let y = b.y; y < b.y + b.h; y++) { + for (let x = b.x, o = (y * stride + x) * 4; x < b.x + b.w; x++, o += 4) { + hash = Math.imul( + hash ^ ((raw[o]! >> 4) | (raw[o + 1]! & 0xf0) | ((raw[o + 2]! >> 4) << 8)), + 16777619, + ); + } + } + } + return (hash >>> 0).toString(16).padStart(8, "0"); +} + /** Columns holding anything but each row's x=0 colour, over rows `y0..y1`; the full width if none. */ function inkX(raw: Buffer, stride: number, w: number, y0: number, y1: number) { const px = new Uint32Array(raw.buffer, raw.byteOffset, raw.length / 4); diff --git a/src/index.ts b/src/index.ts index b6db13a..a02d0e6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -38,3 +38,4 @@ export { export { type Band, type Box, type Region, type TextDiff } from "./compare.ts"; export { type ElementDiff } from "./locate.ts"; +export { type Accepted, type Fingerprint } from "./accept.ts"; diff --git a/src/report.ts b/src/report.ts index d7d9b03..ca4b8bf 100644 --- a/src/report.ts +++ b/src/report.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import type { Accepted } from "./accept.ts"; import type { Band, Box, Region, TextDiff } from "./compare.ts"; import { CONTEXT_OPTIONS, name, type ResolvedConfig, type Sides } from "./config.ts"; import type { ElementDiff } from "./locate.ts"; @@ -40,6 +41,8 @@ export interface ComparedResult extends BaseResult { images: { left: string; right: string; diff: string }; /** At most three: bands, tallest first, then the largest regions. Not one per region. */ crops: CropResult[]; + /** Every band and region matched an accepted one, and no padding or unstable pixel went unmatched. */ + accepted: boolean; } /** A crop of one change: left and right panels, a 1px red box round it, or a 2px red seam. */ @@ -74,7 +77,10 @@ export interface RunMeta { right: string; widths: number[]; threshold: number | null; - /** The instant both sides' clocks start at on every load, ISO. */ + /** + * The instant both sides' clocks start at on every load of this run, ISO. A `--rerun-failed` + * shot reused from an earlier run was taken under that run's clock. + */ clock: string; blockUrlPatterns: string[]; hideSelectors: string[]; @@ -84,6 +90,8 @@ export interface RunMeta { browser: { name: string; version: string }; /** The storage state file's basename per side, or null. Never its contents. */ storageState: { left: string | null; right: string | null }; + /** A hash of everything that decides how a shot is taken; `--rerun-failed` needs it unchanged. */ + config: string; } export interface Summary { @@ -92,8 +100,12 @@ export interface Summary { /** Null when nothing was compared. */ mean: number | null; worst: { url: string; width: number; score: number } | null; - /** Comparisons under `meta.threshold`; 0 when no threshold was set. */ + /** Comparisons under `meta.threshold`, accepted ones aside; 0 when no threshold was set. */ belowThreshold: number; + /** Comparisons whose every difference was accepted. */ + accepted: number; + /** Accepted differences for a measured route and width that weren't seen: prune them. */ + stale: Accepted[]; } /** The shape of `results.json`, written to the out dir on every run. */ @@ -114,9 +126,15 @@ export function stats(rows: ComparedResult[]) { const round = (key: string, v: unknown) => typeof v === "number" && /^(score|alignedScore|mean)$/.test(key) ? floor4(v) : v; -export function writeResults(results: RouteResult[], meta: RunMeta, outDir: string): string { +export function writeResults( + results: RouteResult[], + meta: RunMeta, + outDir: string, + stale: Accepted[], +): string { const compared = results.filter((r): r is ComparedResult => r.status === "compared"); - const { mean, worst } = compared.length ? stats(compared) : { mean: null, worst: null }; + const open = compared.filter((r) => !r.accepted); + const { mean, worst } = open.length ? stats(open) : { mean: null, worst: null }; const out: Results = { meta, results, @@ -126,7 +144,11 @@ export function writeResults(results: RouteResult[], meta: RunMeta, outDir: stri mean, worst: worst && { url: worst.url, width: worst.width, score: worst.score }, belowThreshold: - meta.threshold === null ? 0 : compared.filter((r) => r.score < meta.threshold!).length, + meta.threshold === null + ? 0 + : compared.filter((r) => !r.accepted && r.score < meta.threshold!).length, + accepted: compared.filter((r) => r.accepted).length, + stale, }, }; const rows = results.map((r) => ` ${JSON.stringify(r, round)}`).join(",\n"); @@ -150,13 +172,21 @@ const band = (score: number) => (score >= 0.995 ? "good" : score >= 0.97 ? "warn function section(rows: ComparedResult[]): string { if (!rows.length) return ""; - const { sorted, mean, worst } = stats(rows); + const { sorted } = stats(rows); + const open = rows.filter((r) => !r.accepted); + const agg = open.length ? stats(open) : null; return `
-

mean ${pct(mean)} · - worst ${pct(worst.score)} - (${esc(name(worst))} @${worst.width}) · ${sorted.length} comparison(s)

+

${ + agg + ? `mean ${pct(agg.mean)} · + worst ${pct(agg.worst.score)} + (${esc(name(agg.worst))} @${agg.worst.width}) · ` + : "" + }${sorted.length} comparison(s)${ + rows.length - open.length ? `, ${rows.length - open.length} accepted` : "" + }

${sorted.map(card).join("\n")}
`; } @@ -210,6 +240,11 @@ function findings(r: ComparedResult): string { return items.length ? `
${items.join("")}
` : ""; } +const reasons = (r: ComparedResult) => + [ + ...new Set([...(r.bands ?? []), ...r.regions].flatMap((g) => (g.accepted ? [g.accepted] : []))), + ].join("; "); + function card(r: ComparedResult): string { const imgs = r.images; return ` @@ -219,6 +254,7 @@ function card(r: ComparedResult): string { ${esc(name(r))} @${r.width}px ${r.hook ? `before hook` : ""} + ${r.accepted ? `accepted: ${esc(reasons(r))}` : ""} ${r.heightDelta ? `height ${signed(r.heightDelta)}px` : ""} ${r.widthDelta ? `width ${signed(r.widthDelta)}px` : ""} ${r.unstablePixels ? `${r.unstablePixels.toLocaleString("en-US")} px unstable` : ""} @@ -238,6 +274,7 @@ export async function writeReport( results: RouteResult[], meta: RunMeta, cfg: ResolvedConfig, + stale: Accepted[], ): Promise { const compared = results.filter((r): r is ComparedResult => r.status === "compared"); const errors = results.filter((r): r is ErrorResult => r.status === "error"); @@ -262,7 +299,7 @@ export async function writeReport( margin:0 0 1rem; padding:.75rem 1rem 1rem; } .card header { display:flex; align-items:center; gap:.75rem; flex-wrap:wrap; margin-bottom:.75rem } .score { font-weight:700; font-variant-numeric:tabular-nums; min-width:4.5rem } - .w, .px, .delta, .unstable, .aligned, .hook { color:var(--mut); font-size:12px } + .w, .px, .delta, .unstable, .aligned, .hook, .accepted { color:var(--mut); font-size:12px } .findings { font-size:12px; color:var(--mut); margin:0 0 .75rem } .findings p { margin:.25rem 0 0 } .findings ul { margin:.25rem 0; font:12px/1.4 ui-monospace,SFMono-Regular,Menlo,monospace } @@ -304,6 +341,14 @@ ${ : "" } +${ + stale.length + ? `

Stale accepted differences

+

Accepted, but not seen on this run: prune them from the accepted file.

+
    ${stale.map((a) => `
  • ${esc(a.route)} @${a.width}px — ${esc(a.reason)}
    • `).join("")}
` + : "" +} +
What was neutralized

Blocked on both sides — analytics, consent, and A/B tooling that would otherwise diff --git a/src/run.ts b/src/run.ts index c191c93..65ca000 100644 --- a/src/run.ts +++ b/src/run.ts @@ -1,8 +1,10 @@ import { spawn } from "node:child_process"; +import { createHash } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; -import { chromium, errors, type Page } from "playwright"; +import { chromium, errors, type Browser, type Page } from "playwright"; +import { applyAccepted, readAccepted, updateAccepted } from "./accept.ts"; import { cancelUnstable, capture, HookError, prepare, type Shot } from "./capture.ts"; import { compare, crops, type Band } from "./compare.ts"; import { locate, type ElementDiff } from "./locate.ts"; @@ -23,6 +25,7 @@ import { writeResults, type ComparedResult, type ErrorResult, + type Results, type RouteResult, } from "./report.ts"; @@ -43,6 +46,11 @@ interface RunOpts { open: boolean; /** A line per shot as it lands; otherwise one compact summary at the end. */ progress: boolean; + /** Accept every difference not yet accepted, with this reason. */ + accept: string | null; + /** Rerun only what the previous results.json in the out dir didn't find clean or accepted. */ + rerun: boolean; + acceptedFile: string; } const slug = (url: string) => @@ -88,29 +96,130 @@ function wipeable(dir: string): boolean { return fs.statSync(dir).isDirectory() && fs.readdirSync(dir).every(ours); } -export async function run({ routes, cfg, open, progress }: RunOpts): Promise { - if (!wipeable(cfg.outDir)) { +const reusable = (r: RouteResult) => + r.status === "compared" && ((r.diffPixels === 0 && r.unstablePixels === 0) || r.accepted); + +const files = (r: RouteResult) => + r.status === "compared" ? [...Object.values(r.images), ...r.crops.map((c) => c.path)] : []; + +/** Everything that decides how a shot is taken, hook and `locate` sources included. */ +function configHash(cfg: ResolvedConfig, routes: Route[]): string { + const { threshold, outDir, concurrency, storageState, ...rest } = cfg; + const json = JSON.stringify( + { + ...rest, + storageState: [storageState.left, storageState.right].map((f) => f && path.basename(f)), + routes, + }, + (_, v) => (typeof v === "function" ? String(v) : v), + ); + return createHash("sha256").update(json).digest("hex").slice(0, 16); +} + +/** The previous run's results when this run repeats it, else why it doesn't. */ +function previous(file: string, now: Record): Results | string { + if (!fs.existsSync(file)) return `no ${file} to rerun from; run without --rerun-failed first`; + let prev: Results; + try { + prev = JSON.parse(fs.readFileSync(file, "utf8")); + } catch { + return `${file} is not valid JSON; run without --rerun-failed`; + } + const was: Record = { + ...prev.meta, + routes: [...new Set(prev.results?.map(name))], + }; + for (const [key, value] of Object.entries(now)) { + if (key === "config" && value !== was[key]) { + return ( + `the config (masks, hooks, hidden selectors, blocked URLs, CSS, storage state) ` + + `differs from the one that wrote ${file}; run without --rerun-failed to start over` + ); + } + if (JSON.stringify(value) !== JSON.stringify(was[key])) { + return ( + `${key} ${JSON.stringify(value)} differ from the previous run's ${JSON.stringify(was[key])} ` + + `in ${file}; run without --rerun-failed to start over` + ); + } + } + return prev; +} + +export async function run({ + routes, + cfg, + open, + progress, + accept, + rerun, + acceptedFile, +}: RunOpts): Promise { + const names = routes.map(name); + const setup = { + left: cfg.left, + right: cfg.right, + widths: cfg.widths, + blockUrlPatterns: cfg.blockUrlPatterns, + hideSelectors: cfg.hideSelectors, + masks: cfg.masks.map(({ urlPath, reason }) => ({ urlPath, reason })), + storageState: { + left: cfg.storageState.left && path.basename(cfg.storageState.left), + right: cfg.storageState.right && path.basename(cfg.storageState.right), + }, + }; + let accepted; + try { + accepted = readAccepted(acceptedFile); + } catch (e) { + console.error(`✗ ${e instanceof Error ? e.message : String(e)}`); + return 2; + } + const config = configHash(cfg, routes); + const prev = rerun + ? previous(path.join(cfg.outDir, "results.json"), { + version: version(), + left: cfg.left, + right: cfg.right, + widths: cfg.widths, + routes: names, + config, + }) + : null; + if (typeof prev === "string") { + console.error(`✗ --rerun-failed: ${prev}`); + return 2; + } + if (!prev && !wipeable(cfg.outDir)) { console.error( `✗ output directory ${cfg.outDir} holds files renderdiff didn't write — ` + `refusing to wipe it. Pass --out with a new or empty directory.`, ); return 2; } - fs.rmSync(cfg.outDir, { recursive: true, force: true }); + if (!prev) fs.rmSync(cfg.outDir, { recursive: true, force: true }); fs.mkdirSync(cfg.outDir, { recursive: true }); + if (prev) applyAccepted(prev.results, accepted); + const before = new Map(prev?.results.map((r) => [`${name(r)}@${r.width}`, r])); + const results: RouteResult[] = []; const jobs: Job[] = []; - for (const route of routes) for (const width of cfg.widths) jobs.push({ route, width }); + for (const route of routes) { + for (const width of cfg.widths) { + const had = before.get(`${name(route)}@${width}`); + if (had && reusable(had)) results.push(had); + else jobs.push({ route, width }); + } + } const stems = slugs(routes); - const results: RouteResult[] = []; - const { browser, channel } = await launch(); + const launched = jobs.length ? await launch() : null; const queue = [...jobs]; let done = 0; // Whole minute, so an HH:MM display reads the same on both sides. const now = Math.floor(Date.now() / 60_000) * 60_000; - const worker = async () => { + const worker = async (browser: Browser) => { // One context per origin so cookies/localStorage never cross between them. const leftCtx = await browser.newContext({ ...CONTEXT_OPTIONS, @@ -195,6 +304,7 @@ export async function run({ routes, cfg, open, progress }: RunOpts): Promise worker(launched.browser))); + await launched.browser.close(); + } + const browserMeta = launched + ? { name: launched.channel ?? "chromium", version: launched.browser.version() } + : prev!.meta.browser; + + // A rerun overwrites a shot's files in place; drop the ones it no longer writes, ours only. + const kept = new Set(results.flatMap(files)); + for (const { route, width } of jobs) { + const had = before.get(`${name(route)}@${width}`); + for (const f of had ? files(had) : []) { + if (!kept.has(f) && ours(f)) fs.rmSync(path.join(cfg.outDir, f), { force: true }); + } + } // Workers finish out of order; job order keeps results.json stable across runs. - const names = routes.map(name); const order = (r: RouteResult) => names.indexOf(name(r)) * cfg.widths.length + cfg.widths.indexOf(r.width); results.sort((a, b) => order(a) - order(b)); + let { unmatched, stale } = applyAccepted(results, accepted); + if (accept !== null) { + const added = unmatched.map(({ route, width, fingerprint }) => ({ + route, + width, + reason: accept, + fingerprint, + })); + const pruned = stale.length; + if (updateAccepted(acceptedFile, stale, added)) { + ({ stale } = applyAccepted(results, readAccepted(acceptedFile))); + } + console.log(`accepted ${added.length} difference(s), pruned ${pruned} stale — ${acceptedFile}`); + } + + const { left, right, widths, ...rest } = setup; const meta = { tool: "renderdiff" as const, version: version(), generatedAt: new Date().toISOString(), - left: cfg.left, - right: cfg.right, - widths: cfg.widths, + left, + right, + widths, threshold: cfg.threshold, clock: new Date(now).toISOString(), - blockUrlPatterns: cfg.blockUrlPatterns, - hideSelectors: cfg.hideSelectors, - masks: cfg.masks.map(({ urlPath, reason }) => ({ urlPath, reason })), + ...rest, browser: browserMeta, - storageState: { - left: cfg.storageState.left && path.basename(cfg.storageState.left), - right: cfg.storageState.right && path.basename(cfg.storageState.right), - }, + config, }; - const reportPath = await writeReport(results, meta, cfg); - const resultsPath = writeResults(results, meta, cfg.outDir); - + const reportPath = await writeReport(results, meta, cfg, stale); + const resultsPath = writeResults(results, meta, cfg.outDir, stale); + + if (prev) { + const rerunRoutes = new Set(jobs.map((j) => name(j.route))).size; + console.log( + `rerun: ${jobs.length} shot(s) on ${rerunRoutes} route(s), ` + + `${results.length - jobs.length} reused, ${names.length - rerunRoutes} route(s) untouched`, + ); + } + if (stale.length) { + console.log( + `stale accepted: ${stale.map((a) => `${a.route}@${a.width}`).join(" ")} — ${acceptedFile}`, + ); + } if (progress) summarize(results, reportPath, resultsPath); else console.log(compact(results, names, resultsPath, cfg.threshold)); @@ -279,7 +424,8 @@ export async function run({ routes, cfg, open, progress }: RunOpts): Promise r.status === "compared" && cfg.threshold !== null && r.score < cfg.threshold, + (r) => + r.status === "compared" && !r.accepted && cfg.threshold !== null && r.score < cfg.threshold, ); if (below.length) { console.error(`\n✗ ${below.length} route(s) below ${pct(cfg.threshold!)}.`); @@ -338,8 +484,9 @@ async function shoot( function summarize(results: RouteResult[], reportPath: string, resultsPath: string) { const compared = results.filter((r): r is ComparedResult => r.status === "compared"); - if (compared.length) { - const { sorted, mean, worst } = stats(compared); + const open = compared.filter((r) => !r.accepted); + if (open.length) { + const { sorted, mean, worst } = stats(open); console.log(`\nmean ${pct(mean)} worst ${pct(worst.score)} (${name(worst)} @${worst.width})`); for (const r of sorted.slice(0, 10)) { console.log( @@ -349,6 +496,13 @@ function summarize(results: RouteResult[], reportPath: string, resultsPath: stri } } + const accepted = compared.filter((r) => r.accepted); + if (accepted.length) { + console.log( + `\nAccepted (${accepted.length}): ${accepted.map((r) => `${name(r)}@${r.width}`).join(" ")}`, + ); + } + const errors = results.filter((r): r is ErrorResult => r.status === "error"); if (errors.length) { console.log(`\nErrors (${errors.length}):`); @@ -362,13 +516,13 @@ function summarize(results: RouteResult[], reportPath: string, resultsPath: stri /** At most this many elements per summary line. */ const DOM_ELEMENTS = 3; -/** Located elements in crop order, bands tallest first then regions, one per selector. */ +/** Located elements in crop order, bands tallest first then regions, one per selector; none accepted. */ function elementsOf(r: { - regions: { elements?: ElementDiff[] }[]; - bands: (Band & { elements?: ElementDiff[] })[] | null; + regions: { elements?: ElementDiff[]; accepted?: string }[]; + bands: Band[] | null; }): ElementDiff[] { - const all = [...[...(r.bands ?? [])].sort((a, b) => b.h - a.h), ...r.regions].flatMap( - (g) => g.elements ?? [], + const all = [...[...(r.bands ?? [])].sort((a, b) => b.h - a.h), ...r.regions].flatMap((g) => + g.accepted ? [] : (g.elements ?? []), ); return all .filter((e, i) => all.findIndex((o) => o.selector === e.selector) === i) @@ -391,19 +545,29 @@ export function compact( ): string { const out: string[] = []; const clean: string[] = []; + const accepted: string[] = []; let differ = 0; for (const route of routes) { const rows = results.filter((r) => name(r) === route); const ok = rows.filter((r): r is ComparedResult => r.status === "compared"); // Unstable pixels went unmeasured, so a shot with any is not clean, only unchanged. const same = ok.filter((r) => r.diffPixels === 0 && r.unstablePixels === 0); - const changed = ok.filter((r) => !same.includes(r)); + const acc = ok.filter((r) => r.accepted); + const changed = ok.filter((r) => !same.includes(r) && !r.accepted); if (same.length === rows.length) { clean.push(route); continue; } + if (same.length + acc.length === rows.length) { + accepted.push(route); + continue; + } if (changed.some((r) => r.diffPixels > 0)) differ++; - out.push(route + (same.length ? ` clean @${same.map((r) => r.width).join(",")}` : "")); + out.push( + route + + (same.length ? ` clean @${same.map((r) => r.width).join(",")}` : "") + + (acc.length ? ` accepted @${acc.map((r) => r.width).join(",")}` : ""), + ); const groups = new Map(); for (const r of changed) { const key = JSON.stringify([ @@ -428,10 +592,12 @@ export function compact( if (r.status === "error") failed.set(r.error, [...(failed.get(r.error) ?? []), r.width]); for (const [error, widths] of failed) out.push(` @${widths.join(",")} ERROR ${error}`); } + if (accepted.length) out.unshift(`accepted: ${accepted.join(" ")}`); if (clean.length) out.unshift(`clean: ${clean.join(" ")}`); - const compared = results.filter((r): r is ComparedResult => r.status === "compared"); + const all = results.filter((r): r is ComparedResult => r.status === "compared"); + const compared = all.filter((r) => !r.accepted); const shots = compared.filter((r) => r.diffPixels > 0).length; - const errors = results.length - compared.length; + const errors = results.length - all.length; const worst = compared.length ? stats(compared).worst : null; out.push( `renderdiff: ${differ} of ${routes.length} routes differ (${shots} of ${results.length} shots), ` + @@ -453,6 +619,9 @@ function row(g: ComparedResult[]): string { return lo === hi ? f(lo) : `${f(lo)}–${f(hi)}`; }; const regions = g.map((x) => x.regions.length); + const accepted = g.map( + (x) => [...(x.bands ?? []), ...x.regions].filter((c) => c.accepted).length, + ); const unstable = Math.max(...g.map((x) => x.unstablePixels)); // Band y varies with width; a collapsed row shows the first one's, marked approximate. const y = (b: Band) => `@y${g.length > 1 ? "~" : ""}${b.y}`; @@ -469,6 +638,7 @@ function row(g: ComparedResult[]): string { r.alignedScore !== null ? `aligned ${pct(r.alignedScore)}` : "", Math.max(...regions) ? `${span(regions, String)} reg` : "", unstable ? `unstable ${unstable.toLocaleString("en-US")}px` : "", + Math.max(...accepted) ? `${span(accepted, String)} accepted` : "", text.length ? `text ${text.slice(0, TEXT_LINES).join(" ")}` + (text.length > TEXT_LINES ? ` (+${text.length - TEXT_LINES} more)` : "") diff --git a/tests/helpers/cli.ts b/tests/helpers/cli.ts index 559fe13..2adfa47 100644 --- a/tests/helpers/cli.ts +++ b/tests/helpers/cli.ts @@ -46,6 +46,8 @@ export interface CliOptions { cwd?: string; /** Skip appending `--out`. Only for runs that never reach the browser (`--dry-run`). */ noOut?: boolean; + /** Reuse an out dir from an earlier run, e.g. for `--rerun-failed`. Must be a `CliResult.outDir`. */ + outDir?: string; timeoutMs?: number; /** Extra environment variables for the subprocess. */ env?: Record; @@ -66,7 +68,7 @@ export interface CliResult { export function runCli(args: string[], opts: CliOptions = {}): Promise { const cwd = opts.cwd ?? tempDir("renderdiff-cwd-"); - const outDir = path.join(tempDir("renderdiff-out-"), "out"); + const outDir = opts.outDir ?? path.join(tempDir("renderdiff-out-"), "out"); const argv = opts.noOut ? [...args] : [...args, "--out", outDir]; return new Promise((resolve, reject) => { diff --git a/tests/helpers/parse.ts b/tests/helpers/parse.ts index 7cc0f07..22e0ca1 100644 --- a/tests/helpers/parse.ts +++ b/tests/helpers/parse.ts @@ -45,11 +45,18 @@ export function result( export interface Summary { /** Routes on the `clean:` line; a selector route is `/a [sel]`, as is its key in `routes`. */ clean: string[]; - /** Each differing route: widths that were clean, then one row per collapsed width group. */ + /** Routes on the `accepted:` line: every width clean or accepted. */ + accepted: string[]; + /** `route@width` of each stale accepted difference, from the `stale accepted:` line. */ + stale: string[]; + /** The `rerun:` line of a `--rerun-failed` run, else null. */ + rerun: { shots: number; routes: number; reused: number; untouched: number } | null; + /** Each differing route: widths that were clean or accepted, then one row per collapsed width group. */ routes: Map< string, { clean: number[]; + accepted: number[]; rows: { widths: number[]; text: string; dom: string | null; crops: string[] }[]; errors: { widths: number[]; message: string }[]; } @@ -77,13 +84,25 @@ export function parseSummary(stdout: string): Summary { const v = lines.map((l) => VERDICT.exec(l)).find(Boolean); if (!v) throw new Error(`no verdict line in:\n${stdout}`); // A selector can hold spaces, so a route ends where the next `/` starts or the line ends. - const clean = /^clean: (.*)$/m.exec(stdout)?.[1]!.match(/\/\S*(?: \[.*?\])?(?= \/|$)/g) ?? []; + const listed = (label: string) => + new RegExp(`^${label}: (.*)$`, "m").exec(stdout)?.[1]!.match(/\/\S*(?: \[.*?\])?(?= \/|$)/g) ?? + []; + const stale = /^stale accepted: (.*) — .*$/m.exec(stdout)?.[1]!.split(" ") ?? []; + const rerun = + /^rerun: (\d+) shot\(s\) on (\d+) route\(s\), (\d+) reused, (\d+) route\(s\) untouched$/m.exec( + stdout, + ); const routes: Summary["routes"] = new Map(); let at: ReturnType; for (const line of lines) { let m; - if ((m = /^(\/\S*(?: \[.*\])?)(?: clean @([\d,]+))?$/.exec(line))) { - at = { clean: m[2] ? m[2].split(",").map(Number) : [], rows: [], errors: [] }; + if ((m = /^(\/\S*(?: \[.*\])?)(?: clean @([\d,]+))?(?: accepted @([\d,]+))?$/.exec(line))) { + at = { + clean: m[2] ? m[2].split(",").map(Number) : [], + accepted: m[3] ? m[3].split(",").map(Number) : [], + rows: [], + errors: [], + }; routes.set(m[1]!, at); } else if (at && (m = /^ @([\d,]+) ERROR (.*)$/.exec(line))) { at.errors.push({ widths: m[1]!.split(",").map(Number), message: m[2]! }); @@ -96,7 +115,15 @@ export function parseSummary(stdout: string): Summary { } } return { - clean, + clean: listed("clean"), + accepted: listed("accepted"), + stale, + rerun: rerun && { + shots: Number(rerun[1]), + routes: Number(rerun[2]), + reused: Number(rerun[3]), + untouched: Number(rerun[4]), + }, routes, verdict: { differ: Number(v[1]), @@ -238,6 +265,8 @@ export interface ReportCard { crops: string[]; /** The card carries the "before hook" badge. */ hook: boolean; + /** The accepted badge's reasons, or null. */ + accepted: string | null; } export interface ReportErrorRow { @@ -249,6 +278,8 @@ export interface ReportErrorRow { export interface Report { cards: ReportCard[]; errors: ReportErrorRow[]; + /** The stale accepted differences section; `message` is the reason. */ + stale: ReportErrorRow[]; /** Block globs listed in the "What was neutralized" disclosure. */ blockPatterns: string[]; /** Mask `reason` strings listed in the same disclosure. */ @@ -265,6 +296,8 @@ const unesc = (s: string) => .replace(/"/g, '"') .replace(/&/g, "&"); +const unescOr = (s: string | undefined) => (s === undefined ? null : unesc(s)); + function between(html: string, start: string, end: string): string { const i = html.indexOf(start); if (i === -1) return ""; @@ -323,14 +356,18 @@ export function parseReport(html: string): Report { images: [...card.matchAll(/ m[1]!), crops: [...card.matchAll(/ m[1]!), hook: card.includes('before hook'), + accepted: unescOr(/accepted: ([^<]*)<\/span>/.exec(card)?.[1]), }); } - const errors: ReportErrorRow[] = []; - const errorSection = between(html, '

Errors

', ""); - for (const m of errorSection.matchAll(/
  • ([^<]*)<\/code> @(\d+)px — ([\s\S]*?)<\/li>/g)) { - errors.push({ route: unesc(m[1]!), width: Number(m[2]), message: m[3]! }); - } + const listed = (section: string) => + [...section.matchAll(/
  • ([^<]*)<\/code> @(\d+)px — ([\s\S]*?)<\/li>/g)].map((m) => ({ + route: unesc(m[1]!), + width: Number(m[2]), + message: m[3]!, + })); + const errors = listed(between(html, '

    Errors

    ', "")); + const stale = listed(between(html, "

    Stale accepted differences

    ", "")); const neutralized = between(html, "
    ", "
    "); const blockPatterns = [...neutralized.matchAll(/
  • ([^<]*)<\/code><\/li>/g)].map( @@ -338,7 +375,7 @@ export function parseReport(html: string): Report { ); const maskReasons = [...neutralized.matchAll(/
  • ((?!)[^<]*)<\/li>/g)].map((m) => m[1]!); - return { cards, errors, blockPatterns, maskReasons, neutralized }; + return { cards, errors, stale, blockPatterns, maskReasons, neutralized }; } /** The report card for one comparison. Throws with context when absent. */ diff --git a/tests/unit/cli-args.test.ts b/tests/unit/cli-args.test.ts index 7432974..0b47aef 100644 --- a/tests/unit/cli-args.test.ts +++ b/tests/unit/cli-args.test.ts @@ -54,6 +54,8 @@ describe("help and version", () => { "--storage-state", "--left-storage-state", "--right-storage-state", + "--accept", + "--rerun-failed", ]) { assert.ok(r.stdout.includes(flag), `--help does not mention ${flag}`); } @@ -349,6 +351,18 @@ describe("storage state", () => { }); }); +describe("accept and rerun", () => { + test("--accept needs a reason", async () => { + await rejects(["--dry-run", L, R, "/a", "--accept"], "--accept needs a value"); + await rejects(["/a", "--left", L, "--right", R, "--accept", " "], "--accept needs a reason"); + }); + + test("--accept and --rerun-failed need a run, not --dry-run", async () => { + await rejects(["--dry-run", L, R, "/a", "--accept", "known"], "--accept needs a run"); + await rejects(["--dry-run", L, R, "/a", "--rerun-failed"], "--rerun-failed needs a run"); + }); +}); + describe("preflight", () => { // Port 1 is reserved and never listening, so this is a closed port rather than a race // against something we just shut down. diff --git a/tests/visual/accepted.test.ts b/tests/visual/accepted.test.ts new file mode 100644 index 0000000..03e967b --- /dev/null +++ b/tests/visual/accepted.test.ts @@ -0,0 +1,248 @@ +/** + * Accepted differences and --rerun-failed: the loop a migration runs in. Record what already + * differs, change code, re-check only what's red. + * + * A `before` hook that restyles the left side stands in for "changed code": the fixture pages + * are static, and a hook is the one way to change a side between two runs of the same route. + * Each acceptance is paired with a control, so the mechanism is shown hiding only what it was + * told to: a new change elsewhere on an accepted page, and a different change in the same spot, + * both still surface. + */ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { after, before, describe, test } from "node:test"; +import { cleanupTempDirs, context, renderdiffArgs, runCli, tempDir } from "../helpers/cli.ts"; +import { card, parseReport, parseSummary, result, results } from "../helpers/parse.ts"; +import { startFixtureSite, type FixtureSite } from "../helpers/server.ts"; + +let site: FixtureSite; + +before(async () => { + site = await startFixtureSite(); +}); + +after(async () => { + await site?.close(); + cleanupTempDirs(); +}); + +const args = (routes: string[], extra: string[] = [], widths = [1440]) => [ + ...renderdiffArgs({ left: site.left, right: site.right, routes, widths, concurrency: 2 }), + ...extra, +]; + +/** A config in `dir` whose hook restyles one side, so the accepted file is found beside it. */ +function leftCss(dir: string, css: string, side = "left") { + const file = path.join(dir, "hook.mjs"); + fs.writeFileSync( + file, + `export default { before: async (page, { side }) => { if (side === "${side}") await page.addStyleTag({ content: ${JSON.stringify(css)} }); } };\n`, + ); + return ["--config", file]; +} + +const THIRD = ".strip:first-of-type > .swatch:nth-child(3)"; + +describe("accepted differences", () => { + let dir: string; + const file = () => path.join(dir, "renderdiff.accepted.json"); + + before(async () => { + dir = tempDir(); + const run = await runCli(args(["/", "/color", "/nav"], ["--accept", "known before the move"]), { + cwd: dir, + }); + assert.equal(run.code, 0, context(run)); + assert.match(run.stdout, /^accepted 2 difference\(s\), pruned 0 stale — .*$/m, context(run)); + }); + + test("--accept records each change with its route, width, fingerprint and reason", () => { + const list = JSON.parse(fs.readFileSync(file(), "utf8")) as { + route: string; + width: number; + reason: string; + fingerprint: { kind: string }; + }[]; + // /color is one region; /nav is one band. + assert.deepEqual( + list.map((a) => [a.route, a.width, a.fingerprint.kind, a.reason]), + [ + ["/color", 1440, "region", "known before the move"], + ["/nav", 1440, "band", "known before the move"], + ], + fs.readFileSync(file(), "utf8"), + ); + }); + + test("a later run reports them as accepted, not as differing, and passes the threshold", async () => { + const run = await runCli(args(["/", "/color", "/nav"], ["--threshold", "0.999"]), { + cwd: dir, + }); + assert.equal(run.code, 0, context(run)); + const s = parseSummary(run.stdout); + assert.deepEqual([s.clean, s.accepted, [...s.routes.keys()]], [["/"], ["/color", "/nav"], []]); + assert.deepEqual([s.verdict.differ, s.verdict.shots, s.verdict.below?.count], [0, 0, 0]); + const res = results(run.outDir); + assert.equal(result(res, "/color", 1440).accepted, true); + assert.equal(result(res, "/color", 1440).regions[0]!.accepted, "known before the move"); + assert.deepEqual([res.summary.accepted, res.summary.belowThreshold], [2, 0]); + const report = parseReport(run.report()!); + assert.equal(card(report, "/nav", 1440).accepted, "known before the move"); + assert.equal(card(report, "/", 1440).accepted, null); + }); + + test("a new change elsewhere on an accepted page still surfaces, the accepted one aside", async () => { + const run = await runCli(args(["/color"], [...leftCss(dir, ".notes h2 { color: #2e7d32 }")]), { + cwd: dir, + }); + const s = parseSummary(run.stdout); + assert.deepEqual(s.accepted, [], context(run)); + const [row] = s.routes.get("/color")!.rows; + assert.match(row!.text, / 2 reg 1 accepted/, context(run)); + // The dom line names the new heading, not the swatch already accepted. + assert.match(row!.dom ?? "", /h2 color/); + assert.doesNotMatch(row!.dom ?? "", /rgb\(198, 40, 40\)/, "the accepted red swatch"); + const r = result(results(run.outDir), "/color", 1440); + assert.equal(r.accepted, false); + assert.deepEqual(r.regions.map((g) => g.accepted ?? null).sort(), [ + "known before the move", + null, + ]); + }); + + test("a different change in the accepted spot is not the accepted one", async () => { + const run = await runCli(args(["/color"], leftCss(dir, `${THIRD} { background: #2e7d32 }`)), { + cwd: dir, + }); + const s = parseSummary(run.stdout); + assert.deepEqual(s.accepted, [], context(run)); + const r = result(results(run.outDir), "/color", 1440); + assert.equal(r.regions.length, 1, context(run)); + assert.equal(r.regions[0]!.accepted, undefined); + assert.equal(s.verdict.differ, 1); + }); + + test("the accepted band redrawn at the same size and tone is not the accepted one", async () => { + // Overlining the right-only link instead of underlining it changes no compared property, no text and no size, and + // barely moves the band's mean colour: only its pixels say it is new. + const run = await runCli( + args(["/nav"], leftCss(dir, 'a[href="#pricing"] { text-decoration: overline }', "right")), + { cwd: dir }, + ); + const r = result(results(run.outDir), "/nav", 1440); + assert.deepEqual( + r.bands?.map((b) => [b.side, b.h]), + [["right", 24]], + context(run), + ); + assert.equal(r.accepted, false, context(run)); + assert.deepEqual(parseSummary(run.stdout).accepted, []); + }); + + test("an accepted difference no longer seen is listed as stale; unchecked routes aren't", async () => { + const run = await runCli(args(["/color"], leftCss(dir, `${THIRD} { background: #c62828 }`)), { + cwd: dir, + }); + assert.equal(run.code, 0, context(run)); + const s = parseSummary(run.stdout); + assert.deepEqual([s.clean, s.stale], [["/color"], ["/color@1440"]], context(run)); + assert.deepEqual( + parseReport(run.report()!).stale.map((a) => [a.route, a.width, a.message]), + [["/color", 1440, "known before the move"]], + ); + assert.equal(results(run.outDir).summary.stale.length, 1); + }); + + test("--accept is scoped to the run's routes and widths, and prunes their stale entries", async () => { + const dir2 = tempDir(); + fs.copyFileSync(file(), path.join(dir2, "renderdiff.accepted.json")); + const hook = leftCss(dir2, `${THIRD} { background: #c62828 }`); + const run = await runCli(args(["/color"], [...hook, "--accept", "fixed"]), { cwd: dir2 }); + assert.match(run.stdout, /^accepted 0 difference\(s\), pruned 1 stale/m, context(run)); + const at = path.join(dir2, "renderdiff.accepted.json"); + const list = JSON.parse(fs.readFileSync(at, "utf8")) as { route: string }[]; + assert.deepEqual( + list.map((a) => a.route), + ["/nav"], + ); + const mtime = fs.statSync(at).mtimeMs; + const again = await runCli(args(["/color"], [...hook, "--accept", "fixed"]), { cwd: dir2 }); + assert.match(again.stdout, /^accepted 0 difference\(s\), pruned 0 stale/m, context(again)); + assert.equal(fs.statSync(at).mtimeMs, mtime, "nothing to change, so the file isn't rewritten"); + assert.deepEqual(fs.readdirSync(dir2).sort(), ["hook.mjs", "renderdiff.accepted.json"]); + }); +}); + +describe("--rerun-failed", () => { + test("reruns only what wasn't clean or accepted, and leaves the rest untouched", async () => { + const dir = tempDir(); + const routes = ["/", "/color", "/nav"]; + const first = await runCli(args(routes, [], [1440, 375])); + assert.equal(first.code, 0, context(first)); + // Accept /color elsewhere: the rerun reads it from cwd and matches it against results.json. + const accepted = await runCli(args(["/color"], ["--accept", "known"], [1440, 375]), { + cwd: dir, + }); + assert.equal(accepted.code, 0, context(accepted)); + + const index = first.file("index.1440.left.png"); + const mtime = fs.statSync(index).mtimeMs; + const was = results(first.outDir); + const hits = { home: site.hits("/"), color: site.hits("/color"), nav: site.hits("/nav") }; + + const run = await runCli(args(routes, ["--rerun-failed"], [1440, 375]), { + cwd: dir, + outDir: first.outDir, + }); + assert.equal(run.code, 0, context(run)); + assert.deepEqual( + [site.hits("/"), site.hits("/color")], + [hits.home, hits.color], + "clean and accepted routes are not loaded again", + ); + assert.ok(site.hits("/nav") > hits.nav, context(run)); + const s = parseSummary(run.stdout); + assert.deepEqual(s.rerun, { shots: 2, routes: 1, reused: 4, untouched: 2 }, context(run)); + assert.deepEqual([s.clean, s.accepted, [...s.routes.keys()]], [["/"], ["/color"], ["/nav"]]); + + const now = results(run.outDir); + assert.deepEqual( + now.results.map((r) => `${r.url}@${r.width}`), + ["/@1440", "/@375", "/color@1440", "/color@375", "/nav@1440", "/nav@375"], + ); + assert.deepEqual(result(now, "/", 1440), result(was, "/", 1440)); + assert.equal(fs.statSync(index).mtimeMs, mtime, "a reused shot's files are not rewritten"); + assert.equal(parseReport(run.report()!).cards.length, 6); + for (const r of now.results) { + if (r.status === "compared") assert.ok(fs.existsSync(run.file(r.images.diff)), r.url); + } + }); + + test("a run that doesn't repeat the previous one is refused", async () => { + const first = await runCli(args(["/color"])); + assert.equal(first.code, 0, context(first)); + const again = (a: string[]) => runCli(a, { outDir: first.outDir }); + for (const [a, needle] of [ + [args(["/color"], ["--rerun-failed"], [375]), /--rerun-failed: widths \[375\] differ/], + [args(["/nav"], ["--rerun-failed"]), /--rerun-failed: routes \["\/nav"\] differ/], + [ + [...args(["/color"], ["--rerun-failed"]), "--right", site.left], + /--rerun-failed: right .* differ/, + ], + [ + args(["/color"], ["--rerun-failed", ...leftCss(tempDir(), "h1 { color: red }")]), + /--rerun-failed: the config \(masks, hooks/, + ], + ] as const) { + const r = await again([...a]); + assert.equal(r.code, 2, context(r)); + assert.match(r.stderr, needle); + } + assert.ok(first.report(), "a refused rerun leaves the out dir alone"); + + const fresh = await runCli(args(["/color"], ["--rerun-failed"])); + assert.equal(fresh.code, 2, context(fresh)); + assert.match(fresh.stderr, /no .*results\.json to rerun from/); + }); +}); diff --git a/tests/visual/controls.test.ts b/tests/visual/controls.test.ts index 8e6ed0e..90d4456 100644 --- a/tests/visual/controls.test.ts +++ b/tests/visual/controls.test.ts @@ -202,7 +202,7 @@ for (const route of ["/color", "/nav"]) { assert.equal(r.widthDelta, c.widthDelta); assert.equal(r.unstablePixels, c.unstablePixels); assert.deepEqual( - r.regions.slice(0, 5).map(({ elements, ...g }) => g), + r.regions.slice(0, 5).map(({ elements, hash, ...g }) => g), c.regions.map(({ dom, ...g }) => g), ); assert.deepEqual(r.text, { added: c.added, removed: c.removed });