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); + }); +});