From 903162f101ba5caa61c1427019f00f3301b5e94e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 3 Oct 2026 11:40:12 -0400 Subject: [PATCH 1/2] ci: a test-only change under studio or player needs no captures Tests are not behaviour a user sees, like the markdown the check already skips. A bench-harness PR (#4953) was asked for Before and After screenshots. --- scripts/check-pr-captures.mjs | 13 +++++++------ scripts/check-pr-captures.test.mjs | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/scripts/check-pr-captures.mjs b/scripts/check-pr-captures.mjs index 9e6ab31420..42308860ad 100644 --- a/scripts/check-pr-captures.mjs +++ b/scripts/check-pr-captures.mjs @@ -1,6 +1,6 @@ #!/usr/bin/env node // Fail a PR touching packages/studio or packages/player unless its body has Before and After sections with media. -// Markdown under them is docs, not behaviour, so it is not watched. +// Markdown and tests under them are not behaviour a user sees, so they are not watched. // usage: node scripts/check-pr-captures.mjs --base origin/main --head ; the body arrives in the env (see main). import { execFileSync } from "node:child_process"; @@ -35,8 +35,6 @@ function isMediaUrl(raw) { return url !== null && (isAttachmentUrl(url) || MEDIA_PATH.test(url.pathname)); } -const TEST_FILE = /\.(?:test|spec)\.[jt]sx?$/; - const CAPTURE_TITLES = { before: /^before(?:\s*[:(].*|\s+[-–—]\s.*)?$/i, after: /^after(?:\s*[:(].*|\s+[-–—]\s.*)?$/i, @@ -133,8 +131,12 @@ function parseRecord(record) { } const DOC_FILE = /\.mdx?$/i; +const TEST_PATH = /\/(?:tests|__tests__)\/|\.(?:test|spec)\./; const isWatched = (path) => - path !== "" && WATCHED_PREFIXES.some((prefix) => path.startsWith(prefix)) && !DOC_FILE.test(path); + path !== "" && + WATCHED_PREFIXES.some((prefix) => path.startsWith(prefix)) && + !DOC_FILE.test(path) && + !TEST_PATH.test(path); /** Parse `git diff --numstat -z --no-renames`, keeping watched paths. A binary file counts as a full budget. */ export const parseNumstat = (numstat) => @@ -143,8 +145,7 @@ export const parseNumstat = (numstat) => .map(parseRecord) .filter((file) => isWatched(file.path)); -const isVisualFile = (path) => - VISUAL_EXTENSIONS.some((ext) => path.endsWith(ext)) && !TEST_FILE.test(path); +const isVisualFile = (path) => VISUAL_EXTENSIONS.some((ext) => path.endsWith(ext)); /** Why a "No visible change" declaration does not hold for this diff; empty means it holds. */ export function noVisibleChangeFailures(files) { diff --git a/scripts/check-pr-captures.test.mjs b/scripts/check-pr-captures.test.mjs index 610c898c89..8d27adeda7 100644 --- a/scripts/check-pr-captures.test.mjs +++ b/scripts/check-pr-captures.test.mjs @@ -116,6 +116,17 @@ test("markdown under a watched package needs no capture", () => { assert.equal(evaluate({ body: "", files }).ok, true); }); +test("test files under a watched package need no capture", () => { + const files = parseNumstat( + "28\t0\tpackages/studio/tests/e2e/edit-accuracy/server.test.mjs\0" + + "12\t4\tpackages/studio/tests/e2e/edit-accuracy/case.mjs\0" + + "6\t1\tpackages/player/src/a.spec.tsx\0" + + "40\t0\tpackages/studio/src/b.test.ts\0", + ); + assert.deepEqual(files, []); + assert.equal(evaluate({ body: "", files }).ok, true); +}); + test("a binary file spends the whole no-visible-change budget", () => { const [file] = parseNumstat("-\t-\tpackages/studio/public/logo.png\0"); assert.equal(file.lines, 20); @@ -193,7 +204,10 @@ test("comment fragments cannot rebuild a comment or hide a section from the read test("No visible change accepts a test-only .tsx change but not a component change", () => { const body = "## No visible change\nx"; - assert.equal(evaluate({ body, files: [studio("packages/studio/src/A.test.tsx", 5)] }).ok, true); + assert.equal( + evaluate({ body, files: parseNumstat("5\t0\tpackages/studio/src/A.test.tsx\0") }).ok, + true, + ); assert.equal(evaluate({ body, files: [studio("packages/studio/src/A.tsx", 5)] }).ok, false); }); From 67a14da3aaee189e3b663f9ef1c03a689106f68a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 3 Oct 2026 11:53:31 -0400 Subject: [PATCH 2/2] ci: pin a mixed source and test change, and say tests are not counted --- packages/studio/AGENTS.md | 2 +- scripts/check-pr-captures.mjs | 2 +- scripts/check-pr-captures.test.mjs | 11 ++++++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/studio/AGENTS.md b/packages/studio/AGENTS.md index 35ee67faab..325b5c9aef 100644 --- a/packages/studio/AGENTS.md +++ b/packages/studio/AGENTS.md @@ -149,7 +149,7 @@ bun run --cwd packages/studio test:edit-accuracy -- --grid pr --filter '^resize- - **Before and After captures.** A PR that changes code under `packages/studio` or `packages/player` needs `## Before` and `## After` sections in its description, each with an image or video. The exemptions (a small change with - no visible effect, Markdown) are defined in `scripts/check-pr-captures.mjs`. + no visible effect, Markdown, tests) are defined in `scripts/check-pr-captures.mjs`. ## Traps worth knowing diff --git a/scripts/check-pr-captures.mjs b/scripts/check-pr-captures.mjs index 42308860ad..31c941cd46 100644 --- a/scripts/check-pr-captures.mjs +++ b/scripts/check-pr-captures.mjs @@ -469,7 +469,7 @@ function printFailure(problems, prNumber) { "Edit the body text first: gh pr edit --body-file replaces the body and drops attachments.", ); console.error( - `A change with no visible effect (under ${NO_VISIBLE_CHANGE_MAX_LINES} lines, no .tsx/.css/.html) may instead add a '## No visible change' section.`, + `A change with no visible effect (under ${NO_VISIBLE_CHANGE_MAX_LINES} lines, no .tsx/.css/.html; tests and Markdown are not counted) may instead add a '## No visible change' section.`, ); } diff --git a/scripts/check-pr-captures.test.mjs b/scripts/check-pr-captures.test.mjs index 8d27adeda7..4fcfd62968 100644 --- a/scripts/check-pr-captures.test.mjs +++ b/scripts/check-pr-captures.test.mjs @@ -125,6 +125,11 @@ test("test files under a watched package need no capture", () => { ); assert.deepEqual(files, []); assert.equal(evaluate({ body: "", files }).ok, true); + const mixed = parseNumstat( + "3\t0\tpackages/studio/src/a.ts\0" + "40\t0\tpackages/studio/src/a.test.ts\0", + ); + assert.deepEqual(mixed, [studio("packages/studio/src/a.ts", 3)]); + assert.equal(evaluate({ body: "", files: mixed }).ok, false); }); test("a binary file spends the whole no-visible-change budget", () => { @@ -202,12 +207,8 @@ test("comment fragments cannot rebuild a comment or hide a section from the read ); }); -test("No visible change accepts a test-only .tsx change but not a component change", () => { +test("No visible change does not cover a component change", () => { const body = "## No visible change\nx"; - assert.equal( - evaluate({ body, files: parseNumstat("5\t0\tpackages/studio/src/A.test.tsx\0") }).ok, - true, - ); assert.equal(evaluate({ body, files: [studio("packages/studio/src/A.tsx", 5)] }).ok, false); });