From 9b8f7844f1d864a606c82a70b69378ff9e23d2fa Mon Sep 17 00:00:00 2001 From: kevinccbsg Date: Tue, 28 Jul 2026 19:03:24 +0200 Subject: [PATCH 1/4] docs(plans): add implementation plan for --record-pace --- .../plans/2026-07-28-cli-record-pace.md | 379 ++++++++++++++++++ 1 file changed, 379 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-28-cli-record-pace.md diff --git a/docs/superpowers/plans/2026-07-28-cli-record-pace.md b/docs/superpowers/plans/2026-07-28-cli-record-pace.md new file mode 100644 index 0000000..647a8a3 --- /dev/null +++ b/docs/superpowers/plans/2026-07-28-cli-record-pace.md @@ -0,0 +1,379 @@ +# twd-cli Record Pace Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let `npx twd-cli run --record --record-pace 500` slow a recorded run down to a watchable speed. + +**Architecture:** twd-js exposes a `window.__twdSetPace(ms)` tooling global that spaces out its own command loop. twd-cli gains one config key, one CLI flag, and one `page.evaluate` call that drives it. All of it is gated on recording being enabled, so a normal run is untouched. + +**Tech Stack:** Node ESM, Puppeteer, Vitest. + +Spec: `twd/specs/2026-07-28-twd-js-command-pacing-design.md` + +## Prerequisite + +The twd-js half must ship first. This plan calls `window.__twdSetPace`, which does not exist until then. If that global is missing at runtime the `page.evaluate` throws, so do not merge this ahead of the twd release. + +## Global Constraints + +- Repo is `/Users/kevinccbsg/brikev/twd-cli`. This plan does NOT touch the `twd` repo. +- Node ESM only. Every relative import needs the `.js` extension. +- **Pacing must only ever happen inside a recorded run.** Every new call site is gated on `record.enabled`. +- Recording still defaults to off, and existing tests build config objects with no `record` key, so keep using optional chaining and never assume the key exists. +- `record.pace` defaults to `0`, meaning no pacing. There is no non-zero default, because the right value depends on the app. +- No test may require a real browser. +- Follow the existing one-test-file-per-module convention in `tests/`, Vitest. +- Conventional Commits. +- **Commit messages must NOT contain any `Co-Authored-By` trailer or Claude Code attribution.** End the message at its last content line. +- Do not use em-dashes. `README.md` has pre-existing ones in untouched sections; leave those, just do not add new ones. + +## File Structure + +| File | Responsibility | +|---|---| +| `src/config.js` (modify) | `record.pace` default, inside the existing `DEFAULT_RECORD`. | +| `src/parseArgs.js` (modify) | `--record-pace ` joins the three existing record flags. | +| `src/index.js` (modify) | One `page.evaluate` after `startRecording`. | +| `bin/twd-cli.js` (modify) | Help text row. | +| `README.md` (modify) | Recording options table row and a note on when to use it. | + +--- + +### Task 1: The pace knob + +Config default and CLI flag together, since neither is useful alone and both are a few lines. + +**Files:** +- Modify: `src/config.js`, `src/parseArgs.js` +- Test: `tests/config.test.js`, `tests/parseArgs.test.js` + +**Interfaces:** +- Consumes: nothing. +- Produces: `loadConfig().record.pace` (number, default `0`), and `parseRunArgs(argv).record.pace` set only when the flag is passed. + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/config.test.js` inside the existing `describe('loadConfig', ...)`: + +```js + it('defaults record.pace to 0 so recording never paces unless asked', () => { + vi.mocked(fs.existsSync).mockReturnValue(false); + + expect(loadConfig().record.pace).toBe(0); + }); + + it('merges a partial record.pace without dropping the other record defaults', () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue( + JSON.stringify({ record: { pace: 500 } }) + ); + + const { record } = loadConfig(); + + expect(record.pace).toBe(500); + expect(record.postRoll).toBe(500); + expect(record.format).toBe('mp4'); + expect(record.viewport).toEqual({ width: 1280, height: 720, deviceScaleFactor: 1 }); + }); +``` + +Add to `tests/parseArgs.test.js` inside the existing `describe`: + +```js + it("parses --record-pace as a number, both forms", () => { + expect(parseRunArgs(['--record-pace', '500']).record).toEqual({ pace: 500 }); + expect(parseRunArgs(['--record-pace=250']).record).toEqual({ pace: 250 }); + }); + + it("ignores a non-numeric or non-positive --record-pace", () => { + expect(parseRunArgs(['--record-pace', 'slow']).record).toEqual({}); + expect(parseRunArgs(['--record-pace', '0']).record).toEqual({}); + expect(parseRunArgs(['--record-pace', '-1']).record).toEqual({}); + }); + + it("ignores a trailing --record-pace with no value", () => { + expect(parseRunArgs(['--record-pace']).record).toEqual({}); + }); + + it("combines --record-pace with --record and a test filter", () => { + expect(parseRunArgs(['--record', '--test', 'checkout', '--record-pace=500'])).toEqual({ + testFilters: ['checkout'], + record: { enabled: true, pace: 500 }, + }); + }); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npx vitest --run tests/config.test.js tests/parseArgs.test.js` +Expected: the new tests FAIL. `record.pace` is `undefined` and the flag is not parsed. + +One existing test will also fail once Step 3 lands: `'includes fully populated record defaults when no config file exists'` asserts the whole `record` object with `toEqual`. Step 5 fixes it. + +- [ ] **Step 3: Add the config default** + +In `src/config.js`, add to `DEFAULT_RECORD` immediately after `speed`: + +```js + // Milliseconds twd-js holds after each command, driven through + // window.__twdSetPace. 0 disables pacing. No non-zero default: the right + // value depends on the app. + pace: 0, +``` + +- [ ] **Step 4: Add the CLI flag** + +In `src/parseArgs.js`, add a branch alongside the existing `--record-speed` handling. It mirrors that branch exactly, including the positive-finite guard: + +```js + } else if (token === '--record-pace' || token.startsWith('--record-pace=')) { + const { value, consumed } = readValue(token, '--record-pace', i); + const parsed = Number(value); + if (value !== undefined && Number.isFinite(parsed) && parsed > 0) { + record.pace = parsed; + } + i += consumed - 1; +``` + +- [ ] **Step 5: Update the existing whole-object assertion** + +In `tests/config.test.js`, the test `'includes fully populated record defaults when no config file exists'` compares the entire `record` object. Add `pace: 0,` to the expected object, immediately after `speed: 1,` so it matches the declaration order. + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: `npx vitest --run tests/config.test.js tests/parseArgs.test.js` +Expected: PASS. + +- [ ] **Step 7: Run the full suite** + +Run: `npx vitest --run` +Expected: PASS, every file. + +- [ ] **Step 8: Commit** + +```bash +git add src/config.js src/parseArgs.js tests/config.test.js tests/parseArgs.test.js +git commit -m "feat(record): add the record.pace knob and --record-pace flag" +``` + +--- + +### Task 2: Drive the pace and document it + +**Files:** +- Modify: `src/index.js`, `bin/twd-cli.js`, `README.md` +- Test: `tests/runTests.test.js` + +**Interfaces:** +- Consumes: `record.pace` from Task 1. Calls `window.__twdSetPace(ms)`, provided by twd-js, which returns the pace actually applied after clamping. +- Produces: nothing consumed by later tasks. + +- [ ] **Step 1: Write the failing test** + +Add a new describe block at the end of `tests/runTests.test.js`: + +```js +describe("runTests pacing", () => { + const paceConfig = { + enabled: true, + dir: './twd-artifacts', + filename: null, + format: 'mp4', + viewport: { width: 1280, height: 720, deviceScaleFactor: 1 }, + fps: 30, + speed: 1, + pace: 500, + preRoll: 0, + postRoll: 500, + hideSidebar: true, + ffmpegPath: 'ffmpeg', + }; + + beforeEach(() => { + vi.clearAllMocks(); + // An earlier describe's restoreAllMocks puts these back to their real + // implementations, so neuter them again. + vi.mocked(assertFfmpegAvailable).mockReset(); + vi.mocked(holdOpeningFrame).mockReset(); + vi.mocked(holdFinalFrame).mockReset(); + vi.mocked(fs.statSync).mockReset(); + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + function pacedPage(applied = 500) { + const page = createMockPage({ + handlers: [{ id: '1', name: 'test1', type: 'test' }], + testStatus: [{ id: '1', status: 'pass' }], + }); + // Enumeration, then the setPace evaluate, then the chunk. + page.evaluate = vi.fn() + .mockResolvedValueOnce([{ id: '1', name: 'test1', type: 'test' }]) + .mockResolvedValueOnce(applied) + .mockResolvedValue([{ id: '1', status: 'pass' }]); + return page; + } + + it("sets the pace in the page when recording with a pace", async () => { + vi.mocked(loadConfig).mockReturnValue({ ...defaultMockConfig, record: paceConfig }); + const page = pacedPage(); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + expect(page.evaluate).toHaveBeenCalledWith(expect.any(Function), 500); + }); + + it("does not set a pace when record.pace is 0", async () => { + vi.mocked(loadConfig).mockReturnValue({ + ...defaultMockConfig, + record: { ...paceConfig, pace: 0 }, + }); + const page = createMockPage({ + handlers: [{ id: '1', name: 'test1', type: 'test' }], + testStatus: [{ id: '1', status: 'pass' }], + }); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + expect(page.evaluate).not.toHaveBeenCalledWith(expect.any(Function), 0); + }); + + it("does not set a pace when recording is disabled", async () => { + vi.mocked(loadConfig).mockReturnValue({ ...defaultMockConfig }); + const page = createMockPage({ + handlers: [{ id: '1', name: 'test1', type: 'test' }], + testStatus: [{ id: '1', status: 'pass' }], + }); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + expect(page.evaluate).not.toHaveBeenCalledWith(expect.any(Function), 500); + }); + + it("warns when twd-js clamps the requested pace", async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.mocked(loadConfig).mockReturnValue({ + ...defaultMockConfig, + record: { ...paceConfig, pace: 99999 }, + }); + const page = pacedPage(5000); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('5000')); + }); +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `npx vitest --run tests/runTests.test.js` +Expected: the new tests FAIL, no evaluate call carries the pace. + +- [ ] **Step 3: Drive the pace** + +In `src/index.js`, immediately after `recorder = await startRecording(page, record, recordOutputPath);` and before `await holdOpeningFrame(record.preRoll);`: + +```js + if (record.pace) { + // twd-js spaces out its own command loop. It clamps, so report back + // what actually took effect. + const applied = await page.evaluate((ms) => window.__twdSetPace(ms), record.pace); + if (applied !== record.pace) { + console.warn(`Warning: pace clamped to ${applied}ms (requested ${record.pace}ms).`); + } + } +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `npx vitest --run tests/runTests.test.js` +Expected: PASS. + +- [ ] **Step 5: Run the full suite** + +Run: `npx vitest --run` +Expected: PASS, every file. + +- [ ] **Step 6: Update the help text** + +In `bin/twd-cli.js`, add to the Options section after the `--record-speed` line, aligning the description column with its siblings: + +``` + --record-pace Slow the run itself so the video is watchable +``` + +- [ ] **Step 7: Document the option** + +In `README.md`, add a row to the Recording Options table immediately after the `speed` row: + +```markdown +| `pace` | number | `0` | Milliseconds twd-js holds after each command, so the run itself is slower and the video is watchable at full frame rate. Unlike `speed`, this does not cost frame rate, because the execution is paced rather than the video stretched. `0` disables it | +``` + +Then add this subsection immediately after the existing "Making the video longer" subsection: + +````markdown +#### Pace versus speed + +`speed` and `pace` both make a video longer, in opposite ways. + +`speed` is an ffmpeg filter applied after recording. It stretches the same frames +over a longer timeline, so the effective frame rate falls in proportion: +measured, identical activity gives 30fps at `speed: 1`, 15.3fps at `0.5` and +7.7fps at `0.25`. It also slows the dead air exactly as much as the interesting +moments. + +`pace` slows the run itself. twd-js holds briefly after each command, so frames +are captured at full rate and the pauses land where something just happened. +Typing is also spaced out per keystroke, so text appears character by character. + +Prefer `pace`. Reach for `speed` only when you cannot afford a slower run. + +```bash +npx twd-cli run --record --record-pace 500 --test "checkout flow" +``` + +Two things to know. A paced run takes substantially longer, so pace with a +`--test` filter rather than across a whole suite: a chunk is `chunkSize` tests +inside a single browser call bounded by `protocolTimeout`, and enough pacing will +exceed it. And pacing inserts real delays between actions, which can hide race +conditions, so a paced run is even less representative of CI than a recorded run +already is. +```` + +- [ ] **Step 8: Verify nothing regressed** + +Run: `npx vitest --run` +Expected: PASS, every file. + +- [ ] **Step 9: Commit** + +```bash +git add src/index.js bin/twd-cli.js README.md tests/runTests.test.js +git commit -m "feat(record): drive twd-js command pacing from --record-pace" +``` + +--- + +## Manual verification + +Everything above is mocked, so one real run is worth doing against an example app with its dev server running, using a twd-js build that includes the pacing hook: + +```bash +npx twd-cli run --record --record-pace 500 --test "" +open twd-artifacts/*.mp4 +``` + +Check: the clip is noticeably longer than the same run without `--record-pace`, the pauses land after each action rather than at random, text appears character by character, and the frame rate has not dropped (`ffprobe -v error -show_entries stream=avg_frame_rate -of csv=p=0 twd-artifacts/*.mp4` should still report 30). + +## Not in this plan + +The twd-js half (`src/pace.ts`, the `window.__twdSetPace` hook, and the userEvent and visit wiring) is a separate plan in the `twd` repo and must ship first. From 3f0fc2fba13fc7565e27f237d653edd8e5706b61 Mon Sep 17 00:00:00 2001 From: kevinccbsg Date: Tue, 28 Jul 2026 20:55:25 +0200 Subject: [PATCH 2/4] feat(record): add the record.pace knob and --record-pace flag --- src/config.js | 5 +++++ src/parseArgs.js | 7 +++++++ tests/config.test.js | 22 ++++++++++++++++++++++ tests/parseArgs.test.js | 23 +++++++++++++++++++++++ 4 files changed, 57 insertions(+) diff --git a/src/config.js b/src/config.js index cb91b22..b9c8ab0 100644 --- a/src/config.js +++ b/src/config.js @@ -13,6 +13,11 @@ export const DEFAULT_RECORD = { viewport: { width: 1280, height: 720, deviceScaleFactor: 1 }, fps: 30, speed: 1, + // Milliseconds twd-js holds after each command, driven through + // window.__twdSetPace. Unlike `speed`, this slows the run itself rather than + // stretching the video, so it does not cost frame rate. 0 disables pacing. + // No non-zero default: the right value depends on the app. + pace: 0, // A beat on the opening state before the first test runs. Cosmetic, off by // default. preRoll: 0, diff --git a/src/parseArgs.js b/src/parseArgs.js index 9b78d67..75f9965 100644 --- a/src/parseArgs.js +++ b/src/parseArgs.js @@ -29,6 +29,13 @@ export function parseRunArgs(argv) { record.speed = parsed; } i += consumed - 1; + } else if (token === '--record-pace' || token.startsWith('--record-pace=')) { + const { value, consumed } = readValue(token, '--record-pace', i); + const parsed = Number(value); + if (value !== undefined && Number.isFinite(parsed) && parsed > 0) { + record.pace = parsed; + } + i += consumed - 1; } } diff --git a/tests/config.test.js b/tests/config.test.js index 5475d6c..1d2a9f6 100644 --- a/tests/config.test.js +++ b/tests/config.test.js @@ -190,6 +190,7 @@ describe('loadConfig', () => { viewport: { width: 1280, height: 720, deviceScaleFactor: 1 }, fps: 30, speed: 1, + pace: 0, preRoll: 0, postRoll: 500, hideSidebar: true, @@ -253,4 +254,25 @@ describe('loadConfig', () => { expect(config.record.enabled).toBe(false); expect(config.record.format).toBe('mp4'); }); + + it('defaults record.pace to 0 so recording never paces unless asked', () => { + vi.mocked(fs.existsSync).mockReturnValue(false); + + expect(loadConfig().record.pace).toBe(0); + }); + + it('merges a partial record.pace without dropping the other record defaults', () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue( + JSON.stringify({ record: { pace: 500 } }) + ); + + const { record } = loadConfig(); + + expect(record.pace).toBe(500); + expect(record.postRoll).toBe(500); + expect(record.format).toBe('mp4'); + expect(record.viewport).toEqual({ width: 1280, height: 720, deviceScaleFactor: 1 }); + }); + }); \ No newline at end of file diff --git a/tests/parseArgs.test.js b/tests/parseArgs.test.js index 8cdf671..b2eb066 100644 --- a/tests/parseArgs.test.js +++ b/tests/parseArgs.test.js @@ -73,4 +73,27 @@ describe("parseRunArgs", () => { record: { enabled: true, speed: 0.5 }, }); }); + + it("parses --record-pace as a number, both forms", () => { + expect(parseRunArgs(['--record-pace', '500']).record).toEqual({ pace: 500 }); + expect(parseRunArgs(['--record-pace=250']).record).toEqual({ pace: 250 }); + }); + + it("ignores a non-numeric or non-positive --record-pace", () => { + expect(parseRunArgs(['--record-pace', 'slow']).record).toEqual({}); + expect(parseRunArgs(['--record-pace', '0']).record).toEqual({}); + expect(parseRunArgs(['--record-pace', '-1']).record).toEqual({}); + }); + + it("ignores a trailing --record-pace with no value", () => { + expect(parseRunArgs(['--record-pace']).record).toEqual({}); + }); + + it("combines --record-pace with --record and a test filter", () => { + expect(parseRunArgs(['--record', '--test', 'checkout', '--record-pace=500'])).toEqual({ + testFilters: ['checkout'], + record: { enabled: true, pace: 500 }, + }); + }); + }); From a6cc5fd91a174fd0a34eb24feb9f4666deab9938 Mon Sep 17 00:00:00 2001 From: kevinccbsg Date: Tue, 28 Jul 2026 20:58:30 +0200 Subject: [PATCH 3/4] feat(record): drive twd-js command pacing from --record-pace twd-js spaces out its own command loop, so frames are captured at full rate rather than the video being stretched afterwards the way record.speed does. Measured, speed 0.25 drops the effective frame rate to 7.7fps and slows the dead air as much as the interesting moments; pacing does neither. Degrades rather than crashes on an older twd-js: the in-page call returns null when the hook is absent, so the run still completes and still records, unpaced, with a warning. Without that a missing hook would throw a bare TypeError and take the whole run down, which every user would hit until twd-js ships the hook. --- README.md | 36 ++++++++++--- bin/twd-cli.js | 5 +- src/index.js | 22 ++++++++ tests/runTests.test.js | 113 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 167 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 0b6048d..661c02f 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,7 @@ All keys live under `record` in `twd.config.json`. | `viewport` | object | `{ "width": 1280, "height": 720, "deviceScaleFactor": 1 }` | Applied only when recording. `width` and `height` set the video dimensions. `deviceScaleFactor` does **not** change the output resolution (Puppeteer measures the recording in CSS pixels), it only changes the page environment under test: raising it makes `srcset` and `image-set` pick 2x assets and sends dpr-branching code down a different path | | `fps` | number | `30` | Capture frame rate | | `speed` | number | `1` | Playback speed, e.g. `0.5` for half speed. This is a **uniform stretch of the whole timeline**, not per-command pacing: it slows the fast parts and the already-slow parts equally and cannot hold on a just-clicked element | +| `pace` | number | `0` | Milliseconds twd-js holds after each command, so the run itself is slower. Unlike `speed`, this does not cost frame rate, because the execution is paced rather than the video stretched. See [Pace versus speed](#pace-versus-speed). `0` disables it | | `preRoll` | number | `0` | Milliseconds to hold the opening state before the first test runs. Purely cosmetic | | `postRoll` | number | `500` | Milliseconds to hold the final state after the last test. **Not cosmetic:** without it the last thing your test did never appears in the video at all. See [Why the ending needs a hold](#why-the-ending-needs-a-hold). Set `0` only if you do not care about the ending | | `hideSidebar` | boolean | `true` | Hide the TWD sidebar during capture so the frame is just your app | @@ -157,17 +158,38 @@ Measured against real Chrome: stopping immediately ended two states early, and a overlay after the last test, which forces the real final frame through and then holds it. This is why it defaults to on. -#### Making the video longer +#### Pace versus speed `postRoll` fixes the *ending*, not the *pace*. Tests run in milliseconds, so a -two-test run is around a second of video. Two things help today: +two-test run is around a second of video. `speed` and `pace` both make that +longer, in opposite ways. -- `record.speed` (or `--record-speed 0.5`) stretches the whole timeline -- `record.preRoll` and `record.postRoll` stop it starting and ending abruptly +`speed` is an ffmpeg filter applied after recording. It stretches the same +frames over a longer timeline, so the effective frame rate falls in proportion: +measured on identical activity, 30fps at `speed: 1`, 15.3fps at `0.5` and 7.7fps +at `0.25`. It also slows the dead air exactly as much as the interesting moments. -Both are blunt. Per-command pacing, where the video dwells on each click and -assertion, has to happen inside `twd-js` because that is where the command loop -lives, and it is not part of this feature yet. +`pace` slows the run itself. twd-js holds briefly after each command, so frames +are captured at full rate and the pauses land where something just happened. +Typing is spaced out per keystroke too, so text appears character by character. + +Prefer `pace`. Reach for `speed` only when you cannot afford a slower run. + +```bash +npx twd-cli run --record --record-pace 500 --test "checkout flow" +``` + +Values between 200 and 500 tend to read well. + +Two things to know. A paced run takes substantially longer, so pace with a +`--test` filter rather than across a whole suite: a chunk is `chunkSize` tests +inside a single browser call bounded by `protocolTimeout`, and enough pacing +will exceed it. And pacing inserts real delays between actions, which can hide +race conditions, so a paced run is even less representative of CI than a +recorded run already is. + +Pacing needs a `twd-js` version that provides the hook. On an older version the +run still completes and still records, but unpaced, with a warning saying so. ## How It Works diff --git a/bin/twd-cli.js b/bin/twd-cli.js index cd81e16..decd045 100755 --- a/bin/twd-cli.js +++ b/bin/twd-cli.js @@ -36,9 +36,10 @@ Options: --record Record the run to a video file (requires ffmpeg) --record-dir Output directory (default ./twd-artifacts) --record-speed Playback speed, e.g. 0.5 for half speed + --record-pace Slow the run itself, e.g. 500. Keeps full frame rate - --record-dir and --record-speed only set values. Recording still has to be - turned on with --record or "record": { "enabled": true } in twd.config.json. + These three only set values. Recording still has to be turned on with + --record or "record": { "enabled": true } in twd.config.json. Create a twd.config.json file in your project root to customize settings. `); diff --git a/src/index.js b/src/index.js index d604e1c..515b1b5 100644 --- a/src/index.js +++ b/src/index.js @@ -180,6 +180,28 @@ export async function runTests(options = {}) { recordOutput = path.join(record.dir, filename); recordOutputPath = path.resolve(workingDir, recordOutput); recorder = await startRecording(page, record, recordOutputPath); + + if (record.pace) { + // twd-js spaces out its own command loop, so frames are captured at + // full rate rather than the video being stretched afterwards. + // Returns null when the installed twd-js predates the pacing hook, so + // an older version degrades to an unpaced recording instead of + // crashing the run with a bare TypeError. + const applied = await page.evaluate((ms) => { + if (typeof window.__twdSetPace !== 'function') return null; + return window.__twdSetPace(ms); + }, record.pace); + + if (applied === null) { + console.warn( + 'Warning: --record-pace needs a newer twd-js (no pacing hook found). Recording unpaced.' + ); + } else if (applied !== record.pace) { + // twd-js clamps, so report what actually took effect. + console.warn(`Warning: pace clamped to ${applied}ms (requested ${record.pace}ms).`); + } + } + await holdOpeningFrame(record.preRoll); } diff --git a/tests/runTests.test.js b/tests/runTests.test.js index 1666bf3..41215cc 100644 --- a/tests/runTests.test.js +++ b/tests/runTests.test.js @@ -1070,3 +1070,116 @@ describe("runTests pre-roll and post-roll", () => { expect(holdFinalFrame).not.toHaveBeenCalled(); }); }); + +describe("runTests pacing", () => { + const paceConfig = { + enabled: true, + dir: './twd-artifacts', + filename: null, + format: 'mp4', + viewport: { width: 1280, height: 720, deviceScaleFactor: 1 }, + fps: 30, + speed: 1, + pace: 500, + preRoll: 0, + postRoll: 500, + hideSidebar: true, + ffmpegPath: 'ffmpeg', + }; + + beforeEach(() => { + vi.clearAllMocks(); + // An earlier describe's restoreAllMocks puts these back to their real + // implementations, so neuter them again. + vi.mocked(assertFfmpegAvailable).mockReset(); + vi.mocked(holdOpeningFrame).mockReset(); + vi.mocked(holdFinalFrame).mockReset(); + vi.mocked(fs.statSync).mockReset(); + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + function pacedPage(applied = 500) { + const page = createMockPage({ + handlers: [{ id: '1', name: 'test1', type: 'test' }], + testStatus: [{ id: '1', status: 'pass' }], + }); + // Enumeration, then the setPace evaluate, then the chunk. + page.evaluate = vi.fn() + .mockResolvedValueOnce([{ id: '1', name: 'test1', type: 'test' }]) + .mockResolvedValueOnce(applied) + .mockResolvedValue([{ id: '1', status: 'pass' }]); + return page; + } + + it("sets the pace in the page when recording with a pace", async () => { + vi.mocked(loadConfig).mockReturnValue({ ...defaultMockConfig, record: paceConfig }); + const page = pacedPage(); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + expect(page.evaluate).toHaveBeenCalledWith(expect.any(Function), 500); + }); + + it("does not set a pace when record.pace is 0", async () => { + vi.mocked(loadConfig).mockReturnValue({ + ...defaultMockConfig, + record: { ...paceConfig, pace: 0 }, + }); + const page = createMockPage({ + handlers: [{ id: '1', name: 'test1', type: 'test' }], + testStatus: [{ id: '1', status: 'pass' }], + }); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + expect(page.evaluate).not.toHaveBeenCalledWith(expect.any(Function), 0); + }); + + it("does not set a pace when recording is disabled", async () => { + vi.mocked(loadConfig).mockReturnValue({ ...defaultMockConfig }); + const page = createMockPage({ + handlers: [{ id: '1', name: 'test1', type: 'test' }], + testStatus: [{ id: '1', status: 'pass' }], + }); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + expect(page.evaluate).not.toHaveBeenCalledWith(expect.any(Function), 500); + }); + + it("warns when twd-js clamps the requested pace", async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.mocked(loadConfig).mockReturnValue({ + ...defaultMockConfig, + record: { ...paceConfig, pace: 99999 }, + }); + const page = pacedPage(5000); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + await runTests(); + + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('5000')); + }); + + it("degrades to an unpaced recording when twd-js has no pacing hook", async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.mocked(loadConfig).mockReturnValue({ ...defaultMockConfig, record: paceConfig }); + // An older twd-js: the in-page function returns null rather than a number. + const page = pacedPage(null); + puppeteer.launch.mockResolvedValue(createMockBrowser(page)); + + const hasFailures = await runTests(); + + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('newer twd-js')); + // The run still completes and still records; only the pacing is lost. + expect(hasFailures).toBe(false); + }); + +}); From 1f523f276fc60422a1b13d73cc09c4e60c935741 Mon Sep 17 00:00:00 2001 From: kevinccbsg Date: Tue, 28 Jul 2026 21:19:01 +0200 Subject: [PATCH 4/4] feat(record): pace recorded runs by default at 300ms Recording exists to produce something someone will watch, and an unpaced clip is about a second long, so the default should be the good outcome rather than one a second flag unlocks. --record alone is now watchable. 300 rather than 500: still clearly watchable and roughly 40% cheaper in wall clock on a broad run. --record-pace raises or lowers it, and 0 disables. The cost is run time, not timeouts. A 50 test suite averaging 10 actions per test gains about 2.5 minutes. Reaching protocolTimeout would take around 100 actions in a single test, which is why the docs point at wall clock instead. --- README.md | 29 +++++++++++++++++++++-------- bin/twd-cli.js | 2 +- src/config.js | 9 ++++++--- tests/config.test.js | 6 +++--- twd.config.example.json | 1 + 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 661c02f..d11fd3a 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ All keys live under `record` in `twd.config.json`. | `viewport` | object | `{ "width": 1280, "height": 720, "deviceScaleFactor": 1 }` | Applied only when recording. `width` and `height` set the video dimensions. `deviceScaleFactor` does **not** change the output resolution (Puppeteer measures the recording in CSS pixels), it only changes the page environment under test: raising it makes `srcset` and `image-set` pick 2x assets and sends dpr-branching code down a different path | | `fps` | number | `30` | Capture frame rate | | `speed` | number | `1` | Playback speed, e.g. `0.5` for half speed. This is a **uniform stretch of the whole timeline**, not per-command pacing: it slows the fast parts and the already-slow parts equally and cannot hold on a just-clicked element | -| `pace` | number | `0` | Milliseconds twd-js holds after each command, so the run itself is slower. Unlike `speed`, this does not cost frame rate, because the execution is paced rather than the video stretched. See [Pace versus speed](#pace-versus-speed). `0` disables it | +| `pace` | number | `300` | Milliseconds twd-js holds after each command, so the run itself is slower. **On by default**, because an unpaced recording is about a second long and unwatchable. Unlike `speed` this costs no frame rate, since the execution is paced rather than the video stretched. See [Pace versus speed](#pace-versus-speed). Set `0` to disable | | `preRoll` | number | `0` | Milliseconds to hold the opening state before the first test runs. Purely cosmetic | | `postRoll` | number | `500` | Milliseconds to hold the final state after the last test. **Not cosmetic:** without it the last thing your test did never appears in the video at all. See [Why the ending needs a hold](#why-the-ending-needs-a-hold). Set `0` only if you do not care about the ending | | `hideSidebar` | boolean | `true` | Hide the TWD sidebar during capture so the frame is just your app | @@ -173,20 +173,33 @@ at `0.25`. It also slows the dead air exactly as much as the interesting moments are captured at full rate and the pauses land where something just happened. Typing is spaced out per keystroke too, so text appears character by character. -Prefer `pace`. Reach for `speed` only when you cannot afford a slower run. +Pacing is on by default at 300ms, so `--record` alone gives you something +watchable. Reach for `speed` only when you cannot afford a slower run. ```bash +# Paced at 300ms, no extra flags +npx twd-cli run --record --test "checkout flow" + +# Slower, for a more deliberate demo npx twd-cli run --record --record-pace 500 --test "checkout flow" + +# Off, for the fastest possible recorded run +npx twd-cli run --record --record-pace 0 --test "checkout flow" ``` Values between 200 and 500 tend to read well. -Two things to know. A paced run takes substantially longer, so pace with a -`--test` filter rather than across a whole suite: a chunk is `chunkSize` tests -inside a single browser call bounded by `protocolTimeout`, and enough pacing -will exceed it. And pacing inserts real delays between actions, which can hide -race conditions, so a paced run is even less representative of CI than a -recorded run already is. +**The cost is wall clock.** Roughly, a 50 test suite averaging 10 actions per +test gains about 2.5 minutes at 300ms and 4 minutes at 500ms. That is the reason +to scope a recorded run with `--test` rather than record everything. + +Hitting `protocolTimeout` is unlikely: a chunk is `chunkSize` tests inside a +single browser call bounded by that timeout, so at 300ms you would need around +100 actions in a single test to reach it. If you do somehow get there, lower +`chunkSize` or raise `protocolTimeout`. + +Pacing also inserts real delays between actions, which can hide race conditions, +so a paced run is even less representative of CI than a recorded run already is. Pacing needs a `twd-js` version that provides the hook. On an older version the run still completes and still records, but unpaced, with a warning saying so. diff --git a/bin/twd-cli.js b/bin/twd-cli.js index decd045..a5b0d84 100755 --- a/bin/twd-cli.js +++ b/bin/twd-cli.js @@ -36,7 +36,7 @@ Options: --record Record the run to a video file (requires ffmpeg) --record-dir Output directory (default ./twd-artifacts) --record-speed Playback speed, e.g. 0.5 for half speed - --record-pace Slow the run itself, e.g. 500. Keeps full frame rate + --record-pace Slow the run itself (default 300). 0 disables pacing These three only set values. Recording still has to be turned on with --record or "record": { "enabled": true } in twd.config.json. diff --git a/src/config.js b/src/config.js index b9c8ab0..f278367 100644 --- a/src/config.js +++ b/src/config.js @@ -15,9 +15,12 @@ export const DEFAULT_RECORD = { speed: 1, // Milliseconds twd-js holds after each command, driven through // window.__twdSetPace. Unlike `speed`, this slows the run itself rather than - // stretching the video, so it does not cost frame rate. 0 disables pacing. - // No non-zero default: the right value depends on the app. - pace: 0, + // stretching the video, so it does not cost frame rate. + // + // On by default, because an unpaced recording is roughly a second long and + // nobody can watch it. 300 rather than 500: still clearly watchable, and it + // costs about 40% less wall clock on a broad run. Set 0 to disable. + pace: 300, // A beat on the opening state before the first test runs. Cosmetic, off by // default. preRoll: 0, diff --git a/tests/config.test.js b/tests/config.test.js index 1d2a9f6..d8387f8 100644 --- a/tests/config.test.js +++ b/tests/config.test.js @@ -190,7 +190,7 @@ describe('loadConfig', () => { viewport: { width: 1280, height: 720, deviceScaleFactor: 1 }, fps: 30, speed: 1, - pace: 0, + pace: 300, preRoll: 0, postRoll: 500, hideSidebar: true, @@ -255,10 +255,10 @@ describe('loadConfig', () => { expect(config.record.format).toBe('mp4'); }); - it('defaults record.pace to 0 so recording never paces unless asked', () => { + it('defaults record.pace to 300 so a plain --record run is watchable', () => { vi.mocked(fs.existsSync).mockReturnValue(false); - expect(loadConfig().record.pace).toBe(0); + expect(loadConfig().record.pace).toBe(300); }); it('merges a partial record.pace without dropping the other record defaults', () => { diff --git a/twd.config.example.json b/twd.config.example.json index 4b21e07..3a74912 100644 --- a/twd.config.example.json +++ b/twd.config.example.json @@ -15,6 +15,7 @@ "viewport": { "width": 1280, "height": 720, "deviceScaleFactor": 1 }, "fps": 30, "speed": 1, + "pace": 300, "preRoll": 0, "postRoll": 500, "hideSidebar": true,