From a70d6dbce32081389fc7b7738c77a3a01e8d2e3a Mon Sep 17 00:00:00 2001 From: Vittorio Distefano Date: Wed, 13 May 2026 20:32:37 +0200 Subject: [PATCH] fix(test): keep browser screenshots out of tracked baselines Browser smoke tests wrote timeline acceptance screenshots directly into the tracked screenshots directory, so ordinary validation could dirty the worktree and push contributors toward cleanup hooks. Route default browser captures to the ignored target/browser-smoke/screenshots artifact directory instead, and reserve tracked baseline refreshes for the explicit --update-screenshots runner flag and make screenshots-update target. Update the README and demo docs to document the split between non-mutating smoke validation and intentional baseline refreshes. --- Makefile | 9 +++++++- README.md | 3 ++- demos/README.md | 7 ++++++ tests/demo-browser-check.js | 44 ++++++++++++++++++++++++++++++++++--- 4 files changed, 58 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index de343b2..e8491c6 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ VERSIONED_JS := static/sf/sf.$(VERSION).js # ============== Phony Targets ============== .PHONY: banner help assets build build-release test test-quick test-doc test-unit test-frontend test-browser test-one \ - lint lint-frontend fmt fmt-check clippy ci-local pre-release version package-verify browser-setup \ + lint lint-frontend fmt fmt-check clippy ci-local pre-release version package-verify browser-setup screenshots-update \ bump-version bump-patch bump-minor bump-major bump-dry release-tag demo-serve \ publish-dry publish clean watch @@ -136,6 +136,12 @@ test-browser: printf "$(GREEN)$(CHECK) Browser smoke tests passed$(RESET)\n" || \ (printf "$(RED)$(CROSS) Browser smoke tests failed$(RESET)\n" && exit 1) +screenshots-update: + @printf "$(PROGRESS) Refreshing tracked browser screenshot baselines...\n" + @node tests/demo-browser-check.js --update-screenshots && \ + printf "$(GREEN)$(CHECK) Screenshot baselines refreshed$(RESET)\n" || \ + (printf "$(RED)$(CROSS) Screenshot baseline refresh failed$(RESET)\n" && exit 1) + lint-frontend: @printf "$(PROGRESS) Running frontend lint...\n" @npm run lint:frontend --silent && \ @@ -345,6 +351,7 @@ help: banner @/bin/echo -e " $(GREEN)make test-frontend$(RESET) - Run frontend Node tests" @/bin/echo -e " $(GREEN)make test-browser$(RESET) - Run browser demo smoke tests" @/bin/echo -e " $(GREEN)make test-one TEST=name$(RESET) - Run specific test with output" + @/bin/echo -e " $(GREEN)make screenshots-update$(RESET) - Refresh tracked browser screenshot baselines" @/bin/echo -e "" @/bin/echo -e "$(CYAN)$(BOLD)Lint & Format:$(RESET)" @/bin/echo -e " $(GREEN)make lint$(RESET) - Run fmt-check + clippy + frontend lint" diff --git a/README.md b/README.md index 65b3f32..e5fc2bc 100644 --- a/README.md +++ b/README.md @@ -799,7 +799,8 @@ Runnable demo fixtures live in `demos/`. - `demos/timeline-dense.html` is the repeatable 28-day, 100-lane, 1500-item dense validation fixture for one scrollable body viewport. - `demos/rail.html` focuses on the low-level rail primitives: resource cards, blocks, gauges, and changeovers. - `make demo-serve` serves the repository at `http://localhost:8000/demos/` for local validation. -- `make test-browser` runs browser-level smoke tests against the shipped demo fixtures and refreshes the timeline acceptance screenshots in `screenshots/`. +- `make test-browser` runs browser-level smoke tests against the shipped demo fixtures and writes transient screenshots to `target/browser-smoke/screenshots/`. +- `make screenshots-update` explicitly refreshes the tracked timeline acceptance screenshots in `screenshots/`. - Run `make browser-setup` once on a machine to install the Playwright test dependency and Chromium. ## Acknowledgments diff --git a/demos/README.md b/demos/README.md index 30d7f86..705da1f 100644 --- a/demos/README.md +++ b/demos/README.md @@ -33,6 +33,13 @@ make test-browser ``` The automated check serves the repository locally, opens the runnable demo fixtures in Chromium, fails on page or script errors, and verifies that the primary shipped UI surfaces mount successfully. +It writes transient screenshots to `target/browser-smoke/screenshots/` by default so ordinary validation does not modify tracked baselines. + +Refresh the tracked timeline screenshot baselines only when accepting an intentional visual change: + +```bash +make screenshots-update +``` ## Coverage diff --git a/tests/demo-browser-check.js b/tests/demo-browser-check.js index bffc95a..2221cce 100644 --- a/tests/demo-browser-check.js +++ b/tests/demo-browser-check.js @@ -4,7 +4,46 @@ const http = require('node:http'); const path = require('node:path'); const ROOT = path.resolve(__dirname, '..'); -const SCREENSHOT_DIR = path.join(ROOT, 'screenshots'); +const SCREENSHOT_BASELINE_DIR = path.join(ROOT, 'screenshots'); +const SCREENSHOT_ARTIFACT_DIR = path.join(ROOT, 'target', 'browser-smoke', 'screenshots'); + +function usage() { + return 'Usage: node tests/demo-browser-check.js [--update-screenshots]'; +} + +function parseRunnerConfig(args) { + if (args.length === 0) { + return { + screenshotDir: SCREENSHOT_ARTIFACT_DIR, + updateScreenshots: false, + }; + } + + if (args.length === 1 && args[0] === '--update-screenshots') { + return { + screenshotDir: SCREENSHOT_BASELINE_DIR, + updateScreenshots: true, + }; + } + + throw new Error('Unknown browser check option: ' + args.join(' ') + '\n' + usage()); +} + +function prepareScreenshotDirectory(config) { + if (!config.updateScreenshots) { + fs.rmSync(config.screenshotDir, { recursive: true, force: true }); + } + fs.mkdirSync(config.screenshotDir, { recursive: true }); +} + +let runnerConfig; +try { + runnerConfig = parseRunnerConfig(process.argv.slice(2)); + prepareScreenshotDirectory(runnerConfig); +} catch (error) { + process.stderr.write((error && error.message ? error.message : String(error)) + '\n'); + process.exit(1); +} function contentTypeFor(filePath) { switch (path.extname(filePath).toLowerCase()) { @@ -147,8 +186,7 @@ async function runCheck(name, fn) { } async function captureScreenshot(target, filename) { - fs.mkdirSync(SCREENSHOT_DIR, { recursive: true }); - const screenshotPath = path.join(SCREENSHOT_DIR, filename); + const screenshotPath = path.join(runnerConfig.screenshotDir, filename); await target.screenshot({ path: screenshotPath }); assert.equal(fs.existsSync(screenshotPath), true); assert.equal(fs.statSync(screenshotPath).size > 0, true);