From cda69ce3d0a5b93065a8bb129824882275bebf4e Mon Sep 17 00:00:00 2001 From: kevinccbsg Date: Sun, 6 Sep 2026 18:38:23 +0200 Subject: [PATCH] feat: layout snapshot support (beta) Adds the twd-cli half of twd.matchLayout, which lands in twd-js 1.10.0. matchLayout is off in the browser sidebar on purpose, because the sidebar resizes the page and a developer's window is an arbitrary size, so twd-cli is where a layout snapshot is actually decided. Two flags, kept separate the way Jest separates them, because they close two different holes: --update-snapshots rewrites references that already exist --ci forbids creating one, so a brand new test cannot write its own baseline on the first CI run and pass forever They are injected with evaluateOnNewDocument rather than evaluate, so they are set before any script on the page runs and matchLayout sees them on first read. BEHAVIOUR CHANGE: page.setViewport now runs on every run, not just when recording. A normal run used to inherit Puppeteer's implicit size. This is deliberate: layout snapshots are only reproducible if the viewport is fixed and explicit, and relying on the implicit default would mean a Puppeteer upgrade could change it and invalidate every committed reference at once, silently. The default is 1280x800 and `viewport` in twd.config.json pins your own. While recording, record.viewport still wins, so recording is unchanged. A failure writes .failed.png next to the reference, but in CI that machine is gone by the time anyone looks. So the run also writes a self-contained .twd/snapshot-report.html with every capture embedded as a data URI: one file, one artifact, opens in any browser. Captures from earlier runs are cleared first, since twd-js overwrites a capture on failure but never removes one when the snapshot later passes, and a stale capture showing an already-fixed failure is worse than no report. Deliberately not built: a "3 snapshots written, 1 updated" summary line. It would have guarded against leaving --update-snapshots on in a workflow, but that is a user error and this is a beta nobody has run in anger yet. Dropping it also means twd-cli needs nothing from twd-js beyond the window flags: the report is built entirely from the PNGs on disk. See section 8 of the design doc. --- README.md | 65 ++++++ bin/twd-cli.js | 5 +- .../2026-09-06-layout-snapshots-design.md | 204 ++++++++++++++++++ src/config.js | 17 ++ src/index.js | 48 ++++- src/parseArgs.js | 12 +- src/snapshotReport.js | 158 ++++++++++++++ tests/config.test.js | 68 +++++- tests/parseArgs.test.js | 48 ++++- tests/runTests.test.js | 99 ++++++++- tests/snapshotReport.test.js | 136 ++++++++++++ 11 files changed, 849 insertions(+), 11 deletions(-) create mode 100644 docs/superpowers/specs/2026-09-06-layout-snapshots-design.md create mode 100644 src/snapshotReport.js create mode 100644 tests/snapshotReport.test.js diff --git a/README.md b/README.md index b772d13..ba2738b 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,8 @@ Create a `twd.config.json` file in your project root: | `chunkSize` | number | `10` | How many tests run per browser call. Smaller values make the failure limit and timeouts more granular (less work lost if one chunk hangs); larger values reduce overhead. `0` runs everything in one call | | `contracts` | array | — | OpenAPI contract validation specs (see [Contract Validation](#contract-validation)) | | `contractReportPath` | string | — | Path to write a markdown report for CI/PR integration | +| `viewport` | object | `{ "width": 1280, "height": 800 }` | Browser viewport for every run. Layout snapshots are only reproducible when this is fixed and explicit. While recording, `record.viewport` wins | +| `snapshotDir` | string | `"__twd_snapshots__"` | Where layout snapshot references and failure captures live. Must match the `dir` given to the `twdSnapshot` Vite plugin | | `record` | object | see below | Video recording settings (see [Recording](#recording)) | **Partial Results on Timeout or Crash:** Tests run in chunks (controlled by `chunkSize`), so on a `protocolTimeout` or unexpected crash mid-run, results from completed chunks are printed instead of being lost entirely. @@ -141,6 +143,69 @@ Flags: `--record`, `--record-dir `, `--record-speed `, `--record-pace < Full explanations, including why `postRoll` is on by default and the measured frame rate cost of `speed`, are in the [Recording Runs](https://brikev.github.io/twd/recording) docs. +## Layout snapshots (beta) + +`twd-js` 1.10.0 adds `twd.matchLayout`, which watches the **geometry** of a page +and fails when it moves. It is off in the browser sidebar on purpose, because +the sidebar resizes the page and a developer's window is an arbitrary size, so +**twd-cli is where a layout snapshot is actually decided.** + +```bash +# Compare against the committed references +npx twd-cli run + +# Accept the current layout as the new reference +npx twd-cli run --update-snapshots + +# A missing reference is a failure, never created +npx twd-cli run --ci +``` + +### The two flags are separate on purpose + +| Flag | What it does | +|------|--------------| +| `--update-snapshots` | Rewrites references that already exist. Without it, a changed layout fails, which is the point | +| `--ci` | Forbids *creating* a reference. Without it, a brand new test writes its own baseline on the first CI run and passes forever, and nobody finds out | + +They close two different holes, which is why they are two flags rather than one +mode. `--ci` outranks `--update-snapshots`: both set, with no reference on disk, +is a failure and not a write. + +### Seeing what changed + +A failure writes `.failed.png` next to the reference: your page as it +rendered, with the rows that diverged boxed in red. In CI the machine that +produced it is gone by the time anyone looks, so twd-cli also writes a +self-contained **`.twd/snapshot-report.html`** with every capture embedded. + +One file, one artifact, opens in any browser: + +```yaml +- name: Upload layout snapshot failures + if: failure() + uses: actions/upload-artifact@v4 + with: + name: layout-snapshots + path: .twd/snapshot-report.html +``` + +Captures from earlier runs are cleared before each run, so the report only ever +shows failures from the run you are looking at. The committed `.snap` references +next to them are never touched. + +### Two things to know + +**The viewport changed.** twd-cli now sets an explicit viewport on every run +(`1280x800` by default), not just when recording. Before, a normal run inherited +Puppeteer's implicit size. A test that happened to depend on the old size can +start behaving differently. Set `viewport` in `twd.config.json` to pin your own. + +**`snapshotDir` has to match the Vite plugin.** twd-cli and the `twdSnapshot` +plugin are separate processes that never talk, so the directory is configured +twice. If the report comes out empty when you expected failures, this is the +first thing to check. + ## How It Works **Important**: Puppeteer is **not** used as a testing framework here. It simply provides a headless browser to load your application — the same way a user would open Chrome. Once the page loads, all test execution happens inside the real browser context through the [TWD runner](https://brikev.github.io/twd/). Your tests interact with real DOM, real components, and real browser APIs — Puppeteer just opens the door and gets out of the way. diff --git a/bin/twd-cli.js b/bin/twd-cli.js index 5c6a15b..6f7255f 100755 --- a/bin/twd-cli.js +++ b/bin/twd-cli.js @@ -10,13 +10,16 @@ const command = process.argv[2]; if (command === 'run') { try { - const { testFilters, record, shard, reportDir } = parseRunArgs(process.argv.slice(3)); + const { testFilters, record, shard, reportDir, updateSnapshots, ci } = + parseRunArgs(process.argv.slice(3)); const { runTests } = await import('../src/index.js'); const hasFailures = await runTests({ testFilters, recordOverrides: record, shard, reportDir, + updateSnapshots, + ci, }); process.exit(hasFailures ? 1 : 0); } catch (error) { diff --git a/docs/superpowers/specs/2026-09-06-layout-snapshots-design.md b/docs/superpowers/specs/2026-09-06-layout-snapshots-design.md new file mode 100644 index 0000000..16286ce --- /dev/null +++ b/docs/superpowers/specs/2026-09-06-layout-snapshots-design.md @@ -0,0 +1,204 @@ +# Layout snapshots in twd-cli - Design + +Date: 2026-09-06 +Status: Approved. Scoped down to a POC on 2026-09-06, see section 8. +Scope: `twd-cli` only. +Companion: `twd-js` ships `twd.matchLayout` in 1.10.0. Its design lives in that +repo at `specs/2026-09-06-matchlayout-implementation-design.md`, and the spike +behind it at `specs/2026-09-03-matchlayout-design.md`. + +## 1. Why twd-cli is involved at all + +`twd.matchLayout` captures a DOM node to a grid of bits and compares it against +a committed `.snap` reference, failing when the page geometry moved. It is +deliberately **off by default in the browser sidebar**: the sidebar resizes the +page, and a developer's viewport is whatever their window happens to be, so a +reference created there would fail for everybody else. + +That makes `twd-cli` the place where a layout snapshot is actually decided. This +document covers what `twd-cli` has to do to hold up that end. + +## 2. What this adds + +| Piece | Where | +|---|---| +| `--update-snapshots` and `--ci` flags | `src/parseArgs.js` | +| `viewport` and `snapshotDir` config keys | `src/config.js` | +| An always-applied viewport | `src/index.js` | +| Window flag injection before navigation | `src/index.js` | +| A self-contained HTML failure report | `src/snapshotReport.js` (new) | + +## 3. Flags + +Two boolean flags, parsed like the existing `--record`: + +``` +npx twd-cli run --update-snapshots +npx twd-cli run --ci +``` + +They stay two separate flags on purpose, the way Jest separates them, because +they close two different holes: + +- **`--update-snapshots`** rewrites references that already exist. Without it a + changed layout fails, which is the point. +- **`--ci`** forbids *creating* a reference. Without it, a brand new test writes + its own baseline on the first CI run and passes, forever, and nobody finds + out. That is the expensive failure, because it makes no noise. + +`--ci` outranks `--update-snapshots`. Both set, with no reference on disk, is a +failure and not a write. + +**That precedence is not implemented here.** `twd-cli` only sets both window +flags and lets `matchLayout` decide, because the decision needs the reference +that only the browser side has fetched. Do not reimplement the ordering in the +CLI: two copies of a rule drift. + +## 4. Config + +Two new top-level keys, with defaults: + +```json +{ + "viewport": { "width": 1280, "height": 800 }, + "snapshotDir": "__twd_snapshots__" +} +``` + +`snapshotDir` has to match the `dir` option given to the `twdSnapshot` Vite +plugin. The two live in different processes that never talk, so this is +duplication that cannot be designed away. It gets documented rather than +hidden. + +`viewport` is flat, unlike `record.viewport`, which stays nested under `record` +and keeps its own meaning as the video's dimensions. + +## 5. The viewport, and the behaviour change it brings + +`src/index.js` currently calls `page.setViewport()` **only when recording**. +Every other run inherits Puppeteer's implicit default. + +That is not good enough for snapshots. The whole anti-flaky promise of +`matchLayout` is that the viewport under `twd-cli` is fixed. Today it would be +fixed only by accident, and a Puppeteer upgrade that changed its default would +invalidate every committed reference at once, silently. + +So `page.setViewport(config.viewport)` runs on **every** run. When recording, +`record.viewport` still wins, so recording behaves exactly as it does today. + +**This changes existing behaviour.** Runs that do not record move from +Puppeteer's implicit size to 1280x800. A test that happens to depend on the old +size can start failing. This belongs in the CHANGELOG in plain words, not in a +footnote. + +## 6. Injecting the flags + +In `src/index.js`, **before `page.goto`**: + +```js +await page.evaluateOnNewDocument((f) => { + window.__TWD_SNAPSHOTS__ = true; + if (f.update) window.__TWD_UPDATE_SNAPSHOTS__ = true; + if (f.ci) window.__TWD_SNAPSHOT_CI__ = true; +}, { update, ci }); +``` + +`evaluateOnNewDocument`, never `evaluate`. It runs before any script on the +page, so the flags are already set by the time `matchLayout` reads them. The +`twdSnapshot` Vite plugin sets its own flag with `??=` precisely so this +injection wins. + +`__TWD_SNAPSHOTS__` is always true under `twd-cli`. That is the entire point: +this is where the verdict lives. + +## 7. The HTML report + +A failure writes `.failed.png` next to the reference, on the machine that +ran the test. In CI that machine disappears, so the picture is unreachable +exactly when it is most needed. + +`src/snapshotReport.js` reads `/*.failed.png`, embeds each one as a +`data:` URI, and writes a single self-contained page to **`.twd/snapshot-report.html`**. + +Two decisions worth stating: + +- **It goes in `.twd/`, not next to the PNGs.** `__twd_snapshots__/` holds the + `.snap` files that get committed. `.twd/` holds run output. Keeping ephemeral + artefacts out of a committed directory is worth the extra path. +- **Self-contained, so one artifact carries everything.** A CI job uploads one + file and the reviewer opens it in a browser, instead of downloading a zip of + loose PNGs and matching them up by filename. + +The report needs nothing from `twd-js`. The PNGs are already on disk, and the +snapshot name is the filename minus `.failed.png`. That keeps the two packages +uncoupled for this half of the feature. + +It is written only when there is at least one failure. A clean run leaves no +file. + +The path is fixed for the beta rather than configurable. One less knob to +document while nobody has asked for it, and `--report-dir` already exists for +sharding and means something different. + +## 8. No summary line, and no dependency on twd-js + +An earlier draft counted snapshots in the final block (`3 snapshots written, 1 +updated`), to make it visible when `--update-snapshots` had been left on and had +quietly rewritten every reference. + +**Dropped.** A flag left on in a workflow is a user error, and this is a beta +that nobody has run in a real environment yet. Building a guard against a +failure mode we have not actually seen is guessing, and it would have cost +either a new `window` contract in `twd-js` or a before-and-after hash of every +`.snap` on disk. + +The consequence, stated plainly so it is a decision and not a surprise: a run +with `--update-snapshots` rewrites references and reports nothing. Revisit this +only if it bites someone in real use. + +What this buys: **`twd-cli` needs nothing at all from `twd-js` beyond the window +flags it already reads.** Failures still surface through the normal path, since +`matchLayout` throws and the test fails with its message and a non-zero exit +code, and the HTML report is built purely from the PNGs on disk. + +## 9. Error handling + +- **Snapshot directory missing.** Not an error. It means no snapshots ran, or + none failed. No report, no line, no warning. +- **A PNG that cannot be read.** Skip that entry, keep the rest of the report, + and note the skipped file in the report itself. One unreadable file must not + cost the reviewer the other nine. +- **`.twd/` not writable.** Warn and carry on. Failing a run because the report + could not be written would turn a diagnostic aid into a new failure mode. The + test results are what the exit code is for. + +## 10. Testing + +`twd-cli` keeps one `tests/*.test.js` per `src/` file, and that pattern holds: + +- `tests/parseArgs.test.js`: both flags in `--flag` form, combined, absent, and + that they do not disturb the existing flags. +- `tests/config.test.js`: the two new keys default correctly, a partial + `viewport` in the file merges rather than wiping the default, and + `record.viewport` still behaves as before. +- `tests/snapshotReport.test.js`: a directory with two failure PNGs yields one + HTML file containing two `data:` URIs and both snapshot names; an empty or + missing directory yields no file; an unreadable file is skipped with the rest + intact. +- `tests/testSummary.test.js`: the line appears with counts, is omitted for an + empty list, and handles written-only and updated-only. + +The Puppeteer wiring in `src/index.js` (injection and `setViewport`) is not unit +tested, matching how the rest of that file is treated. It is exercised by +`test-example-app` manually. + +## 11. Out of scope + +- Merging snapshot results across shards. The two features are both beta and + combining them now would design against guesses. +- Any GitHub Actions job summary integration. Rejected outright: it would put + noise on pull requests. +- Uploading images anywhere external so they can be linked. That is the model + this whole feature exists to avoid. +- Recovering the ASCII preview in the failure message. That is `twd-js` work and + is happening separately. diff --git a/src/config.js b/src/config.js index f278367..fafeb4d 100644 --- a/src/config.js +++ b/src/config.js @@ -32,6 +32,15 @@ export const DEFAULT_RECORD = { ffmpegPath: 'ffmpeg', }; +// The viewport every run gets, snapshots or not. Layout snapshots are only +// reproducible if the size is fixed and explicit: relying on puppeteer's +// implicit default would mean a puppeteer upgrade could change it and +// invalidate every committed reference at once, silently. +// +// Deliberately NOT record.viewport, which is the video's dimensions and means +// something different. When recording, that one still wins. +export const DEFAULT_VIEWPORT = { width: 1280, height: 800 }; + const DEFAULT_CONFIG = { url: 'http://localhost:5173', timeout: 10000, @@ -44,6 +53,10 @@ const DEFAULT_CONFIG = { protocolTimeout: 300000, maxFailures: 10, chunkSize: 10, + viewport: DEFAULT_VIEWPORT, + // Must match the `dir` given to the twdSnapshot vite plugin. Two processes + // that never talk to each other, so this duplication cannot be designed away. + snapshotDir: '__twd_snapshots__', record: DEFAULT_RECORD, }; @@ -58,6 +71,10 @@ export function loadConfig() { return { ...DEFAULT_CONFIG, ...userConfig, + // Two levels, like record.viewport below: a flat spread would let + // `{ "viewport": { "width": 375 } }` drop the height and hand puppeteer + // an undefined. + viewport: { ...DEFAULT_VIEWPORT, ...(userConfig.viewport || {}) }, record: { ...DEFAULT_RECORD, ...userRecord, diff --git a/src/index.js b/src/index.js index ec15987..4b06ed2 100644 --- a/src/index.js +++ b/src/index.js @@ -14,6 +14,7 @@ import { resolveRecordFilename } from './recordFilename.js'; import { selectShardIds } from './shard.js'; import { buildRunReport } from './runReport.js'; import { writeRunReport, DEFAULT_REPORT_DIR, COVERAGE_FILE } from './reportFiles.js'; +import { writeSnapshotReport, clearFailureCaptures } from './snapshotReport.js'; import { assertFfmpegAvailable, applyRecordingFraming, @@ -46,7 +47,14 @@ function recordedFileSize(absPath) { } export async function runTests(options = {}) { - const { testFilters = [], recordOverrides = {}, shard = null, reportDir = null } = options; + const { + testFilters = [], + recordOverrides = {}, + shard = null, + reportDir = null, + updateSnapshots = false, + ci = false, + } = options; const sharded = Boolean(shard); let browser; let config; @@ -96,9 +104,12 @@ export async function runTests(options = {}) { const page = await browser.newPage(); - if (recording) { - await page.setViewport(record.viewport); - } + // Every run gets an explicit viewport, not just a recorded one. Layout + // snapshots are only reproducible if the size is fixed and stated: relying + // on puppeteer's implicit default would mean an upgrade could change it and + // invalidate every committed reference at once, without a word. + // record.viewport still wins while recording, since it sets the video size. + await page.setViewport(recording ? record.viewport : config.viewport); // Register mock collector for contract validation const collectedMocks = new Map(); @@ -114,6 +125,21 @@ export async function runTests(options = {}) { }); } + // evaluateOnNewDocument, never evaluate: this runs before any script on the + // page, so the flags are already set by the time matchLayout reads them. + // The twdSnapshot vite plugin sets its own flag with ??= precisely so this + // injection wins. __TWD_SNAPSHOTS__ is always on here because twd-cli is + // where a layout snapshot is actually decided. + await page.evaluateOnNewDocument((flags) => { + window.__TWD_SNAPSHOTS__ = true; + if (flags.update) window.__TWD_UPDATE_SNAPSHOTS__ = true; + if (flags.ci) window.__TWD_SNAPSHOT_CI__ = true; + }, { update: updateSnapshots, ci }); + + // Drop captures from earlier runs before this one can add its own, so the + // report cannot show a failure that has since been fixed. + clearFailureCaptures(path.resolve(workingDir, config.snapshotDir)); + // Navigate to your development server startedAt = Date.now(); console.log(`Navigating to ${config.url} ...`); @@ -437,6 +463,20 @@ export async function runTests(options = {}) { maxFailures: config.maxFailures, })); + const snapshotReport = writeSnapshotReport( + path.resolve(workingDir, config.snapshotDir), + '.twd' + ); + if (snapshotReport) { + const skipped = snapshotReport.skipped.length + ? `, ${snapshotReport.skipped.length} could not be read` + : ''; + console.log( + `Layout snapshot failures: ${snapshotReport.count} captured${skipped}. ` + + `Open ${snapshotReport.reportPath}` + ); + } + // Written last, and only for a sharded run. A run that threw never gets // here on purpose: its artifact stays absent, and `merge` reports the gap as // "a shard job likely failed before uploading", which is the accurate diff --git a/src/parseArgs.js b/src/parseArgs.js index 873897f..d0f2ffb 100644 --- a/src/parseArgs.js +++ b/src/parseArgs.js @@ -14,6 +14,12 @@ export function parseRunArgs(argv) { const record = {}; let shard = null; let reportDir = null; + // Two separate flags on purpose, the way Jest separates them. They close two + // different holes: --update-snapshots rewrites references that already exist, + // --ci forbids creating one that does not. The precedence between them is + // decided in twd-js, which is the only side that has fetched the reference. + let updateSnapshots = false; + let ci = false; for (let i = 0; i < argv.length; i++) { const token = argv[i]; @@ -32,6 +38,10 @@ export function parseRunArgs(argv) { const { value, consumed } = readValue(argv, token, '--report-dir', i); if (value !== undefined) reportDir = value; i += consumed - 1; + } else if (token === '--update-snapshots') { + updateSnapshots = true; + } else if (token === '--ci') { + ci = true; } else if (token === '--record') { record.enabled = true; } else if (token === '--record-dir' || token.startsWith('--record-dir=')) { @@ -55,7 +65,7 @@ export function parseRunArgs(argv) { } } - return { testFilters, record, shard, reportDir }; + return { testFilters, record, shard, reportDir, updateSnapshots, ci }; } // `twd-cli merge [--out ]`. The directory is the first positional diff --git a/src/snapshotReport.js b/src/snapshotReport.js new file mode 100644 index 0000000..3759ff4 --- /dev/null +++ b/src/snapshotReport.js @@ -0,0 +1,158 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +export const SNAPSHOT_REPORT_FILE = 'snapshot-report.html'; + +const SUFFIX = '.failed.png'; + +const ESCAPES = { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }; + +// Snapshot names come from test files, so they are author-controlled rather +// than hostile, but they land in markup and a stray `<` would silently break +// the page for everything after it. +const escapeHtml = (value) => String(value).replace(/[&<>"']/g, (c) => ESCAPES[c]); + +function render(failures, skipped) { + const cards = failures + .map( + ({ name, dataUri }) => `
+

${escapeHtml(name)}

+ Layout diff for ${escapeHtml(name)} +
` + ) + .join('\n'); + + const skippedBlock = skipped.length + ? `
+

Could not be read

+

These capture files exist but could not be opened, so they are not shown above.

+
    ${skipped.map((f) => `
  • ${escapeHtml(f)}
    • `).join('')}
+
` + : ''; + + return ` + + + +TWD layout snapshot failures + + + +

Layout snapshot failures

+

+ Each capture below is the page as it rendered on this run, with the rows that + diverged from the committed reference marked. The reference itself was not + changed. Accept a change with npx twd-cli run --update-snapshots. +

+
    +
  • changed
    • +
  • new area
    • +
+${cards} +${skippedBlock} + + +`; +} + +/** + * Deletes the `.failed.png` captures left by earlier runs. + * + * Needed because twd-js overwrites a capture when a snapshot fails but never + * removes one when that snapshot later passes. Without this sweep a fixed + * layout keeps its old capture forever and the report shows a failure that no + * longer exists, which is worse than having no report at all. + * + * Only ever touches files ending in `.failed.png` inside the configured + * directory. The `.snap` references next to them are committed and are never + * touched. Missing directory is a no-op. + * + * @returns the number of captures removed. + */ +export function clearFailureCaptures(snapshotDir) { + let files; + try { + files = fs.readdirSync(snapshotDir).filter((f) => f.endsWith(SUFFIX)); + } catch { + return 0; + } + + let removed = 0; + for (const file of files) { + try { + fs.rmSync(path.join(snapshotDir, file), { force: true }); + removed++; + } catch { + // A capture we cannot delete is not worth failing a run over. It will + // show up in the report, which is the visible outcome anyway. + } + } + return removed; +} + +/** + * Builds one self-contained HTML page from the `.failed.png` captures a + * run left behind, and writes it to `outDir`. + * + * Self-contained on purpose: the images go in as `data:` URIs so a CI job can + * upload a single artifact and the reviewer opens one file, instead of + * downloading a zip of loose PNGs and matching them up by filename. In CI the + * machine that produced them is gone by the time anyone looks. + * + * Needs nothing from twd-js: the captures are already on disk and the snapshot + * name is the filename minus the suffix. + * + * @returns `{ reportPath, count, skipped }`, or `null` when there was nothing to + * report or the file could not be written. + */ +export function writeSnapshotReport(snapshotDir, outDir) { + let files; + try { + files = fs.readdirSync(snapshotDir).filter((f) => f.endsWith(SUFFIX)).sort(); + } catch { + // No directory means no snapshot ever ran here. Not an error. + return null; + } + + if (files.length === 0) return null; + + const failures = []; + const skipped = []; + for (const file of files) { + try { + const bytes = fs.readFileSync(path.join(snapshotDir, file)); + failures.push({ + name: file.slice(0, -SUFFIX.length), + dataUri: `data:image/png;base64,${bytes.toString('base64')}`, + }); + } catch { + // One unreadable file must not cost the reviewer the other nine. + skipped.push(file); + } + } + + try { + fs.mkdirSync(outDir, { recursive: true }); + const reportPath = path.join(outDir, SNAPSHOT_REPORT_FILE); + fs.writeFileSync(reportPath, render(failures, skipped)); + return { reportPath, count: failures.length, skipped }; + } catch (error) { + // The report is a diagnostic aid. Failing the run because it could not be + // written would turn it into a new failure mode of its own. + console.warn(`Warning: could not write the snapshot report: ${error.message}`); + return null; + } +} diff --git a/tests/config.test.js b/tests/config.test.js index d8387f8..7887847 100644 --- a/tests/config.test.js +++ b/tests/config.test.js @@ -35,6 +35,8 @@ describe('loadConfig', () => { protocolTimeout: 300000, maxFailures: 10, chunkSize: 10, + viewport: { width: 1280, height: 800 }, + snapshotDir: '__twd_snapshots__', record: DEFAULT_RECORD, }); expect(fs.existsSync).toHaveBeenCalledWith(path.resolve(mockCwd, 'twd.config.json')); @@ -63,6 +65,8 @@ describe('loadConfig', () => { protocolTimeout: 300000, maxFailures: 10, chunkSize: 10, + viewport: { width: 1280, height: 800 }, + snapshotDir: '__twd_snapshots__', record: DEFAULT_RECORD, }); expect(fs.readFileSync).toHaveBeenCalledWith( @@ -91,7 +95,12 @@ describe('loadConfig', () => { const config = loadConfig(); - expect(config).toEqual({ ...userConfig, record: DEFAULT_RECORD }); + expect(config).toEqual({ + ...userConfig, + viewport: { width: 1280, height: 800 }, + snapshotDir: '__twd_snapshots__', + record: DEFAULT_RECORD, + }); }); it('should return defaults and warn when config file has invalid JSON', () => { @@ -114,6 +123,8 @@ describe('loadConfig', () => { protocolTimeout: 300000, maxFailures: 10, chunkSize: 10, + viewport: { width: 1280, height: 800 }, + snapshotDir: '__twd_snapshots__', record: DEFAULT_RECORD, }); expect(consoleWarnSpy).toHaveBeenCalledWith( @@ -275,4 +286,57 @@ describe('loadConfig', () => { expect(record.viewport).toEqual({ width: 1280, height: 720, deviceScaleFactor: 1 }); }); -}); \ No newline at end of file +}); +describe('loadConfig snapshot settings', () => { + const mockCwd = '/mock/project'; + const originalCwd = process.cwd; + + beforeEach(() => { + process.cwd = vi.fn(() => mockCwd); + vi.clearAllMocks(); + }); + + afterEach(() => { + process.cwd = originalCwd; + }); + + const withConfig = (userConfig) => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(JSON.stringify(userConfig)); + }; + + it('defaults the viewport to 1280x800', () => { + vi.mocked(fs.existsSync).mockReturnValue(false); + + expect(loadConfig().viewport).toEqual({ width: 1280, height: 800 }); + }); + + it('defaults snapshotDir to __twd_snapshots__', () => { + vi.mocked(fs.existsSync).mockReturnValue(false); + + expect(loadConfig().snapshotDir).toBe('__twd_snapshots__'); + }); + + it('merges a partial viewport over the default instead of wiping it', () => { + // A flat spread would drop `height` and hand puppeteer an undefined, so the + // viewport gets the same two level merge record.viewport already has. + withConfig({ viewport: { width: 375 } }); + + expect(loadConfig().viewport).toEqual({ width: 375, height: 800 }); + }); + + it('lets snapshotDir be overridden', () => { + withConfig({ snapshotDir: 'snapshots' }); + + expect(loadConfig().snapshotDir).toBe('snapshots'); + }); + + it('keeps record.viewport independent of the top level viewport', () => { + // record.viewport is the video's dimensions and means something different. + withConfig({ viewport: { width: 375, height: 667 } }); + + const config = loadConfig(); + expect(config.viewport).toEqual({ width: 375, height: 667 }); + expect(config.record.viewport).toEqual({ width: 1280, height: 720, deviceScaleFactor: 1 }); + }); +}); diff --git a/tests/parseArgs.test.js b/tests/parseArgs.test.js index d9103a4..5985635 100644 --- a/tests/parseArgs.test.js +++ b/tests/parseArgs.test.js @@ -3,7 +3,7 @@ import { parseRunArgs, parseMergeArgs } from "../src/parseArgs.js"; describe("parseRunArgs", () => { it("returns empty filters when no args", () => { - expect(parseRunArgs([])).toEqual({ testFilters: [], record: {}, shard: null, reportDir: null }); + expect(parseRunArgs([])).toEqual({ testFilters: [], record: {}, shard: null, reportDir: null, updateSnapshots: false, ci: false }); }); it("parses a single --test ", () => { @@ -12,6 +12,8 @@ describe("parseRunArgs", () => { record: {}, shard: null, reportDir: null, + updateSnapshots: false, + ci: false, }); }); @@ -21,6 +23,8 @@ describe("parseRunArgs", () => { record: {}, shard: null, reportDir: null, + updateSnapshots: false, + ci: false, }); }); @@ -30,11 +34,13 @@ describe("parseRunArgs", () => { record: {}, shard: null, reportDir: null, + updateSnapshots: false, + ci: false, }); }); it("ignores a trailing --test with no value", () => { - expect(parseRunArgs(['--test'])).toEqual({ testFilters: [], record: {}, shard: null, reportDir: null }); + expect(parseRunArgs(['--test'])).toEqual({ testFilters: [], record: {}, shard: null, reportDir: null, updateSnapshots: false, ci: false }); }); it("ignores unknown tokens", () => { @@ -43,6 +49,8 @@ describe("parseRunArgs", () => { record: {}, shard: null, reportDir: null, + updateSnapshots: false, + ci: false, }); }); @@ -81,6 +89,8 @@ describe("parseRunArgs", () => { record: { enabled: true, speed: 0.5 }, shard: null, reportDir: null, + updateSnapshots: false, + ci: false, }); }); @@ -105,6 +115,8 @@ describe("parseRunArgs", () => { record: { enabled: true, pace: 500 }, shard: null, reportDir: null, + updateSnapshots: false, + ci: false, }); }); @@ -140,8 +152,40 @@ describe('parseRunArgs shard and report flags', () => { record: { enabled: true }, shard: { index: 2, total: 4 }, reportDir: null, + updateSnapshots: false, + ci: false, }); }); + + it("defaults both snapshot flags to false", () => { + const { updateSnapshots, ci } = parseRunArgs([]); + expect(updateSnapshots).toBe(false); + expect(ci).toBe(false); + }); + + it("parses --update-snapshots", () => { + expect(parseRunArgs(['--update-snapshots']).updateSnapshots).toBe(true); + }); + + it("parses --ci", () => { + expect(parseRunArgs(['--ci']).ci).toBe(true); + }); + + it("parses both snapshot flags together", () => { + // They are two different holes and both can be open at once. twd-js decides + // the precedence; the CLI only reports what was asked for. + const { updateSnapshots, ci } = parseRunArgs(['--update-snapshots', '--ci']); + expect(updateSnapshots).toBe(true); + expect(ci).toBe(true); + }); + + it("leaves the other flags alone when snapshot flags are present", () => { + const result = parseRunArgs(['--ci', '--test', 'Login', '--report-dir', './out']); + expect(result.testFilters).toEqual(['Login']); + expect(result.reportDir).toBe('./out'); + expect(result.ci).toBe(true); + }); + }); describe('parseMergeArgs', () => { diff --git a/tests/runTests.test.js b/tests/runTests.test.js index fc59cc8..8bd3af6 100644 --- a/tests/runTests.test.js +++ b/tests/runTests.test.js @@ -42,12 +42,29 @@ function createMockPage({ handlers = [], testStatus = [], recorder } = {}) { .mockResolvedValueOnce(handlers) // enumeration pass returns handler metadata .mockResolvedValue(testStatus), // each chunk run returns its testStatus array exposeFunction: vi.fn(), + evaluateOnNewDocument: vi.fn(), setViewport: vi.fn(), addStyleTag: vi.fn(), screencast: vi.fn().mockResolvedValue(recorder ?? { stop: vi.fn() }), }; } +// Runs a function destined for evaluateOnNewDocument against a stand-in window, +// and hands back what it wrote. +function runInjected(inject, flags) { + const had = 'window' in globalThis; + const previous = globalThis.window; + const win = {}; + globalThis.window = win; + try { + inject(flags); + } finally { + if (had) globalThis.window = previous; + else delete globalThis.window; + } + return win; +} + function createMockBrowser(page) { return { newPage: vi.fn().mockResolvedValue(page), @@ -66,6 +83,8 @@ const defaultMockConfig = { retryCount: 2, maxFailures: 10, chunkSize: 50, + viewport: { width: 1280, height: 800 }, + snapshotDir: '__twd_snapshots__', }; describe("runTests", () => { @@ -203,6 +222,8 @@ describe("runTests", () => { ]; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -280,6 +301,8 @@ describe("runTests", () => { const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -349,6 +372,8 @@ describe("runTests", () => { ]; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -371,6 +396,8 @@ describe("runTests", () => { ]; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn().mockResolvedValueOnce(registry), @@ -396,6 +423,8 @@ describe("runTests", () => { ]; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -422,6 +451,8 @@ describe("runTests", () => { ]; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -526,6 +557,8 @@ describe("runTests", () => { ]; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -561,6 +594,8 @@ describe("runTests", () => { ]; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -589,6 +624,8 @@ describe("runTests", () => { ]; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -621,6 +658,8 @@ describe("runTests", () => { ]; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -651,6 +690,8 @@ describe("runTests", () => { timeoutError.name = 'ProtocolError'; const page = { goto: vi.fn(), + evaluateOnNewDocument: vi.fn(), + setViewport: vi.fn(), waitForSelector: vi.fn(), exposeFunction: vi.fn(), evaluate: vi.fn() @@ -707,6 +748,60 @@ describe("runTests recording", () => { vi.restoreAllMocks(); }); + it("injects the snapshot flags before navigating, never after", async () => { + // Order is the whole point. evaluateOnNewDocument runs before any script on + // the page, so matchLayout sees the flags on first read. Doing this after + // goto would set them too late and every snapshot would silently skip. + const page = createMockPage({ + handlers: [{ id: '1', name: 'test1', type: 'test' }], + testStatus: [{ id: '1', status: 'pass' }], + }); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + expect(page.evaluateOnNewDocument).toHaveBeenCalled(); + expect(page.evaluateOnNewDocument.mock.invocationCallOrder[0]) + .toBeLessThan(page.goto.mock.invocationCallOrder[0]); + }); + + it("turns snapshots on and both modes off by default", async () => { + const page = createMockPage({ + handlers: [{ id: '1', name: 'test1', type: 'test' }], + testStatus: [{ id: '1', status: 'pass' }], + }); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + const [inject, flags] = page.evaluateOnNewDocument.mock.calls[0]; + expect(flags).toEqual({ update: false, ci: false }); + + // The injected function is serialised into the browser, so run it here + // against a stand-in global to see what it actually sets. + expect(runInjected(inject, flags)).toEqual({ __TWD_SNAPSHOTS__: true }); + expect(runInjected(inject, { update: true, ci: false })).toEqual({ + __TWD_SNAPSHOTS__: true, + __TWD_UPDATE_SNAPSHOTS__: true, + }); + expect(runInjected(inject, { update: false, ci: true })).toEqual({ + __TWD_SNAPSHOTS__: true, + __TWD_SNAPSHOT_CI__: true, + }); + }); + + it("passes --update-snapshots and --ci through to the page", async () => { + const page = createMockPage({ + handlers: [{ id: '1', name: 'test1', type: 'test' }], + testStatus: [{ id: '1', status: 'pass' }], + }); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests({ updateSnapshots: true, ci: true }); + + expect(page.evaluateOnNewDocument.mock.calls[0][1]).toEqual({ update: true, ci: true }); + }); + it("does not touch any recording API when recording is disabled", async () => { vi.mocked(loadConfig).mockReturnValue({ ...defaultMockConfig }); const page = createMockPage({ @@ -717,9 +812,11 @@ describe("runTests recording", () => { await runTests(); - expect(page.setViewport).not.toHaveBeenCalled(); expect(page.addStyleTag).not.toHaveBeenCalled(); expect(page.screencast).not.toHaveBeenCalled(); + // The viewport is no longer a recording-only concern: every run gets an + // explicit one so layout snapshots are reproducible. + expect(page.setViewport).toHaveBeenCalledWith({ width: 1280, height: 800 }); }); it("sets the viewport, injects framing and starts the screencast when enabled", async () => { diff --git a/tests/snapshotReport.test.js b/tests/snapshotReport.test.js new file mode 100644 index 0000000..b77f614 --- /dev/null +++ b/tests/snapshotReport.test.js @@ -0,0 +1,136 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { + writeSnapshotReport, + clearFailureCaptures, + SNAPSHOT_REPORT_FILE, +} from '../src/snapshotReport.js'; + +let root; +let snapshotDir; +let outDir; + +// A tiny but real PNG header, so the bytes that reach the data URI are not empty. +const PNG_BYTES = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + +beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'twd-snap-report-')); + snapshotDir = path.join(root, '__twd_snapshots__'); + outDir = path.join(root, '.twd'); + fs.mkdirSync(snapshotDir, { recursive: true }); +}); + +afterEach(() => { + fs.rmSync(root, { recursive: true, force: true }); +}); + +const addFailure = (name) => + fs.writeFileSync(path.join(snapshotDir, `${name}.failed.png`), PNG_BYTES); + +describe('writeSnapshotReport', () => { + it('writes nothing when the snapshot directory does not exist', () => { + // Not an error: it just means no snapshot ever ran here. + const result = writeSnapshotReport(path.join(root, 'nope'), outDir); + + expect(result).toBeNull(); + expect(fs.existsSync(outDir)).toBe(false); + }); + + it('writes nothing when there are no failures', () => { + // A .snap on its own is a passing reference, not a failure. + fs.writeFileSync(path.join(snapshotDir, 'landing.snap'), 'hash abcd'); + + const result = writeSnapshotReport(snapshotDir, outDir); + + expect(result).toBeNull(); + expect(fs.existsSync(path.join(outDir, SNAPSHOT_REPORT_FILE))).toBe(false); + }); + + it('embeds every failure as a data URI in one self-contained file', () => { + // Self-contained is the whole point: one artifact the reviewer opens, + // instead of a zip of loose PNGs to match up by filename. + addFailure('landing'); + addFailure('checkout'); + + const result = writeSnapshotReport(snapshotDir, outDir); + + expect(result.count).toBe(2); + const html = fs.readFileSync(result.reportPath, 'utf8'); + expect(html.match(/data:image\/png;base64,/g)).toHaveLength(2); + expect(html).toContain('landing'); + expect(html).toContain('checkout'); + expect(html).not.toContain('.failed.png"'); + }); + + it('names each snapshot by its file, minus the .failed.png suffix', () => { + addFailure('landing-mobile'); + + const html = fs.readFileSync(writeSnapshotReport(snapshotDir, outDir).reportPath, 'utf8'); + + expect(html).toContain('landing-mobile'); + }); + + it('skips a file it cannot read and keeps the rest of the report', () => { + // One unreadable file must not cost the reviewer the other nine. A + // directory named like a PNG makes readFileSync throw EISDIR portably. + addFailure('landing'); + fs.mkdirSync(path.join(snapshotDir, 'broken.failed.png')); + + const result = writeSnapshotReport(snapshotDir, outDir); + + expect(result.count).toBe(1); + expect(result.skipped).toEqual(['broken.failed.png']); + const html = fs.readFileSync(result.reportPath, 'utf8'); + expect(html).toContain('landing'); + expect(html).toContain('broken.failed.png'); + }); + + it('returns null and does not throw when the output directory cannot be written', () => { + // The report is a diagnostic aid. Failing the run because it could not be + // written would turn it into a new failure mode of its own. + addFailure('landing'); + const blocked = path.join(root, 'blocked'); + fs.writeFileSync(blocked, 'not a directory'); + + expect(() => writeSnapshotReport(snapshotDir, blocked)).not.toThrow(); + expect(writeSnapshotReport(snapshotDir, blocked)).toBeNull(); + }); + + it('escapes a snapshot name so it cannot inject markup into the report', () => { + addFailure(''); + + const html = fs.readFileSync(writeSnapshotReport(snapshotDir, outDir).reportPath, 'utf8'); + + expect(html).not.toContain(' { + it('removes stale captures so the report only shows this run', () => { + // twd-js overwrites a capture on failure but never deletes one when the + // snapshot later passes, so without the sweep a fixed layout keeps showing. + addFailure('landing'); + addFailure('checkout'); + + expect(clearFailureCaptures(snapshotDir)).toBe(2); + expect(writeSnapshotReport(snapshotDir, outDir)).toBeNull(); + }); + + it('never touches the committed .snap references next to them', () => { + const reference = path.join(snapshotDir, 'landing.snap'); + fs.writeFileSync(reference, 'hash abcd'); + addFailure('landing'); + + clearFailureCaptures(snapshotDir); + + expect(fs.existsSync(reference)).toBe(true); + expect(fs.readFileSync(reference, 'utf8')).toBe('hash abcd'); + }); + + it('is a no-op when the directory does not exist', () => { + expect(clearFailureCaptures(path.join(root, 'nope'))).toBe(0); + }); +});