From 5cd3128b6e4252f2745e1da0c5ff65b0042e6d50 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 21 Sep 2026 20:36:47 -0400 Subject: [PATCH 1/2] ci: require zero orphan tests without a baseline --- .github/workflows/ci.yml | 2 +- scripts/check-no-main-deletions.mjs | 4 ++ scripts/check-test-reachability.mjs | 54 ++++-------------------- scripts/check-test-reachability.test.mjs | 52 ++++++++++++++++++----- scripts/test-reachability-baseline.json | 1 - 5 files changed, 55 insertions(+), 58 deletions(-) delete mode 100644 scripts/test-reachability-baseline.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2634db1c34..a5df6c6d61 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,7 +48,7 @@ jobs: - name: Report head reachability run: node scripts/check-test-reachability.mjs --report - run: node --test scripts/check-test-reachability.test.mjs - - run: node scripts/check-test-reachability.mjs --base origin/main + - run: node scripts/check-test-reachability.mjs changes: name: Detect changes diff --git a/scripts/check-no-main-deletions.mjs b/scripts/check-no-main-deletions.mjs index ab62efce9c..cfef68febc 100644 --- a/scripts/check-no-main-deletions.mjs +++ b/scripts/check-no-main-deletions.mjs @@ -40,6 +40,10 @@ const STORYBOARD_VIEW_REASON = "owner-directed removal of the Studio storyboard view; its only readers were deleted with it"; export const ALLOWED_DELETIONS = new Map([ + [ + "scripts/test-reachability-baseline.json", + "Reachability now requires zero orphans and rejects baseline files.", + ], [ "packages/studio/src/player/hooks/useTimelineRowElements.ts", "D-834 removes the duplicate row-source hook; manifest elements are now the single timeline row owner", diff --git a/scripts/check-test-reachability.mjs b/scripts/check-test-reachability.mjs index 4028cd3867..562a864990 100644 --- a/scripts/check-test-reachability.mjs +++ b/scripts/check-test-reachability.mjs @@ -1,4 +1,3 @@ -import { execFileSync } from "node:child_process"; import { createHash } from "node:crypto"; import { readFileSync, existsSync } from "node:fs"; import { posix, resolve } from "node:path"; @@ -399,46 +398,13 @@ export function audit(files, read, manifest) { return issues; } -function baselineIssues(file, count, current, previous) { - const errors = []; - if (![Number.isInteger(count), count > 0, count <= previous].every(Boolean)) - errors.push(`${file}: baseline may only shrink`); - if (current < count) errors.push(`${file}: lower baseline to ${current}`); - return errors; -} - -export function ratchet(issues, baseline, previous = baseline) { - const errors = Object.entries(issues).flatMap(([file, failures]) => { - return failures.length > (baseline.files[file] ?? 0) ? [`${file}: ${failures.join("; ")}`] : []; - }); - const budgetErrors = Object.entries(baseline.files).flatMap(([file, count]) => - baselineIssues(file, count, issues[file]?.length ?? 0, previous.files[file] ?? 0), +export function verdict(issues, baselinePresent = false) { + const errors = Object.entries(issues).map( + ([file, failures]) => `${file}: ${failures.join("; ")}`, ); - if (baseline.total !== Object.values(baseline.files).reduce((sum, n) => sum + n, 0)) - errors.push("Incorrect baseline total"); - return [...errors, ...budgetErrors]; -} - -function baseArgument(index) { - const base = process.argv[index + 1]; - if (!base || base.startsWith("-")) throw new Error("--base needs a Git ref"); - return base; -} - -function previousBaseline(baseline) { - const index = process.argv.indexOf("--base"); - let previous = baseline; - if (index !== -1) { - const base = baseArgument(index); - const paths = execFileSync("git", ["ls-tree", "--name-only", base, "--", BASELINE], { - encoding: "utf8", - }); - if (paths.trim()) - previous = JSON.parse( - execFileSync("git", ["show", `${base}:${BASELINE}`], { encoding: "utf8" }), - ); - } - return previous; + if (baselinePresent) + errors.push("Test reachability baseline is forbidden; every test must be reachable."); + return errors; } function main() { @@ -449,15 +415,11 @@ function main() { const issues = audit(files, read, JSON.parse(read(MANIFEST))); if (process.argv.includes("--report")) { console.log(JSON.stringify(issues, null, 2)); - return; } - const baseline = JSON.parse(read(BASELINE)); - const previous = previousBaseline(baseline); - const errors = ratchet(issues, baseline, previous); + const errors = verdict(issues, existsSync(resolve(root, BASELINE))); if (errors.length) { console.error(errors.join("\n")); process.exitCode = 1; - } else - console.log(`Test reachability verified: ${Object.keys(issues).length} baselined test files.`); + } else console.log("Test reachability verified: zero orphan tests."); } if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) main(); diff --git a/scripts/check-test-reachability.test.mjs b/scripts/check-test-reachability.test.mjs index 23c6919b09..0be5e9a030 100644 --- a/scripts/check-test-reachability.test.mjs +++ b/scripts/check-test-reachability.test.mjs @@ -1,6 +1,11 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { audit, digest, pinnedSource, ratchet } from "./check-test-reachability.mjs"; +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { audit, digest, pinnedSource, verdict } from "./check-test-reachability.mjs"; function fixture(command = "bun run test:scripts", filter = '"scripts/**"') { return { @@ -28,11 +33,10 @@ jobs: } const check = (tree, manifest = { guards: {}, runners: [] }) => audit(Object.keys(tree), (path) => tree[path], manifest); -const empty = { total: 0, files: {} }; test("planted guard-filter hole fails and adding its directory passes", () => { const tree = fixture(); - assert.match(ratchet(check(tree), empty)[0], /CI filters exclude skills\/lib/); + assert.match(verdict(check(tree))[0], /CI filters exclude skills\/lib/); tree[".github/workflows/ci.yml"] = tree[".github/workflows/ci.yml"].replace( '"scripts/**"', '"scripts/**"\n - "skills/**"', @@ -115,13 +119,13 @@ test("custom runner mappings require matching commands and unchanged producer so assert.throws(() => check(tree, manifest), /mapping needs review/); }); -test("baselines accept existing debt, reject new debt and cannot be raised", () => { - const issues = { "a.test.ts": ["orphan"] }; - const baseline = { total: 1, files: { "a.test.ts": 1 } }; - assert.deepEqual(ratchet(issues, baseline), []); - assert.match(ratchet({ ...issues, "new.test.ts": ["orphan"] }, baseline)[0], /new.test.ts/); - assert.match(ratchet(issues, baseline, empty)[0], /only shrink/); - assert.match(ratchet({}, baseline)[0], /lower baseline/); +test("zero orphans passes and any orphan fails", () => { + assert.deepEqual(verdict({}), []); + assert.deepEqual(verdict({ "a.test.ts": ["orphan"] }), ["a.test.ts: orphan"]); +}); + +test("a baseline file is forbidden even with zero orphans", () => { + assert.match(verdict({}, true)[0], /baseline is forbidden/); }); test("conditions in the first step key are never credited", () => { @@ -226,3 +230,31 @@ test("a missing pinned package gives the runner mapping diagnostic", () => { /Runner mapping needs review: missing\/package.json#script#test/, ); }); + +test("report CLI rejects a restored baseline and a planted orphan", (t) => { + const cwd = mkdtempSync(join(tmpdir(), "reachability-")); + t.after(() => rmSync(cwd, { recursive: true, force: true })); + const tree = fixture("node --test scripts/parity.test.mjs", '"**"'); + tree["scripts/test-reachability.json"] = JSON.stringify({ guards: {}, runners: [] }); + for (const [path, text] of Object.entries(tree)) { + mkdirSync(dirname(join(cwd, path)), { recursive: true }); + writeFileSync(join(cwd, path), text); + } + execFileSync("git", ["init", "--quiet"], { cwd }); + execFileSync("git", ["add", "."], { cwd }); + const script = fileURLToPath(new URL("./check-test-reachability.mjs", import.meta.url)); + const run = () => spawnSync(process.execPath, [script, "--report"], { cwd, encoding: "utf8" }); + const clean = run(); + assert.equal(clean.status, 0, clean.stderr); + const baseline = join(cwd, "scripts/test-reachability-baseline.json"); + writeFileSync(baseline, '{"total":0,"files":{}}'); + const restored = run(); + assert.equal(restored.status, 1); + assert.match(restored.stderr, /baseline is forbidden/); + rmSync(baseline); + writeFileSync(join(cwd, "scripts/orphan.test.mjs"), ""); + execFileSync("git", ["add", "."], { cwd }); + const orphan = run(); + assert.equal(orphan.status, 1); + assert.match(orphan.stderr, /orphan.test.mjs: no CI runner selects this test/); +}); diff --git a/scripts/test-reachability-baseline.json b/scripts/test-reachability-baseline.json deleted file mode 100644 index 612672504a..0000000000 --- a/scripts/test-reachability-baseline.json +++ /dev/null @@ -1 +0,0 @@ -{ "total": 0, "files": {} } From 317425394842aee31c4f8b2b1b9fcca628d18888 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 22 Sep 2026 01:10:07 -0400 Subject: [PATCH 2/2] fix: keep reachability reports diagnostic-only --- scripts/check-test-reachability.mjs | 1 + scripts/check-test-reachability.test.mjs | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/scripts/check-test-reachability.mjs b/scripts/check-test-reachability.mjs index 562a864990..95dcd2d788 100644 --- a/scripts/check-test-reachability.mjs +++ b/scripts/check-test-reachability.mjs @@ -415,6 +415,7 @@ function main() { const issues = audit(files, read, JSON.parse(read(MANIFEST))); if (process.argv.includes("--report")) { console.log(JSON.stringify(issues, null, 2)); + return; } const errors = verdict(issues, existsSync(resolve(root, BASELINE))); if (errors.length) { diff --git a/scripts/check-test-reachability.test.mjs b/scripts/check-test-reachability.test.mjs index 0be5e9a030..9d78a6dc7f 100644 --- a/scripts/check-test-reachability.test.mjs +++ b/scripts/check-test-reachability.test.mjs @@ -231,7 +231,7 @@ test("a missing pinned package gives the runner mapping diagnostic", () => { ); }); -test("report CLI rejects a restored baseline and a planted orphan", (t) => { +test("report CLI prints diagnostics without applying the gate verdict", (t) => { const cwd = mkdtempSync(join(tmpdir(), "reachability-")); t.after(() => rmSync(cwd, { recursive: true, force: true })); const tree = fixture("node --test scripts/parity.test.mjs", '"**"'); @@ -249,12 +249,12 @@ test("report CLI rejects a restored baseline and a planted orphan", (t) => { const baseline = join(cwd, "scripts/test-reachability-baseline.json"); writeFileSync(baseline, '{"total":0,"files":{}}'); const restored = run(); - assert.equal(restored.status, 1); - assert.match(restored.stderr, /baseline is forbidden/); + assert.equal(restored.status, 0, restored.stderr); + assert.deepEqual(JSON.parse(restored.stdout), {}); rmSync(baseline); writeFileSync(join(cwd, "scripts/orphan.test.mjs"), ""); execFileSync("git", ["add", "."], { cwd }); const orphan = run(); - assert.equal(orphan.status, 1); - assert.match(orphan.stderr, /orphan.test.mjs: no CI runner selects this test/); + assert.equal(orphan.status, 0, orphan.stderr); + assert.match(orphan.stdout, /orphan.test.mjs/); });