From 8122399ab4cb954d750fce2aca259b2377acc889 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 13 Oct 2026 03:20:29 +0200 Subject: [PATCH 1/3] refactor(coverage): move unified-diff parsing beside its producer parseUnifiedDiff, ChangedFileDiff, and normalizePath now live in scripts/lib beside readChangedLinesDiff, the diff they parse, instead of inside the changed-line gate's model where the CRAP gate had to import across families. Both gates import from scripts/lib. Hand the diff to fallow health on stdin (--diff-stdin) instead of writing and reading coverage/fallow-crap.diff, so no artifact of an earlier run can supply a scope. --- scripts/coverage-changed/model.test.ts | 51 +---------------- scripts/coverage-changed/model.ts | 78 +------------------------- scripts/coverage-changed/run.ts | 9 +-- scripts/coverage-crap/model.ts | 4 +- scripts/coverage-crap/run.test.ts | 10 ++-- scripts/coverage-crap/run.ts | 42 ++++++++------ scripts/lib/repo-git.test.ts | 43 ++++++++++++++ scripts/lib/repo-git.ts | 71 +++++++++++++++++++++++ scripts/lib/repo-path.test.ts | 10 ++++ scripts/lib/repo-path.ts | 12 ++++ 10 files changed, 173 insertions(+), 157 deletions(-) create mode 100644 scripts/lib/repo-git.test.ts create mode 100644 scripts/lib/repo-path.test.ts diff --git a/scripts/coverage-changed/model.test.ts b/scripts/coverage-changed/model.test.ts index 058e81d8e2..b58742dc08 100644 --- a/scripts/coverage-changed/model.test.ts +++ b/scripts/coverage-changed/model.test.ts @@ -1,24 +1,15 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; +import { type ChangedFileDiff, parseUnifiedDiff } from '../lib/repo-git.ts'; import { CHANGED_LINE_COVERAGE_THRESHOLD, computeChangedCoverage, ignoredLineSet, isIncludableSource, isTestFile, - normalizePath, parseLcov, - parseUnifiedDiff, - type ChangedFileDiff, } from './model.ts'; -test('normalizePath strips root, b/ and ./ prefixes and posixifies', () => { - assert.equal(normalizePath('/repo/src/a.ts', '/repo'), 'src/a.ts'); - assert.equal(normalizePath('b/src/a.ts'), 'src/a.ts'); - assert.equal(normalizePath('./src/a.ts'), 'src/a.ts'); - assert.equal(normalizePath(String.raw`src\a.ts`), 'src/a.ts'); -}); - test('parseLcov reads DA hits and BRDA taken, treating "-" as not taken', () => { const lcov = [ 'SF:/repo/src/a.ts', @@ -43,46 +34,6 @@ test('parseLcov reads DA hits and BRDA taken, treating "-" as not taken', () => assert.equal(index.get('src/b.ts')?.lineHits.get(10), 1); }); -test('parseUnifiedDiff numbers added lines per hunk and ignores deleted lines', () => { - const diff = [ - 'diff --git a/src/a.ts b/src/a.ts', - '--- a/src/a.ts', - '+++ b/src/a.ts', - '@@ -1,0 +1,2 @@', - '+const a = 1;', - '+const b = 2;', - '@@ -10,1 +12,1 @@', - '-old line', - '+new line', - ].join('\n'); - const [file] = parseUnifiedDiff(diff); - assert.equal(file.path, 'src/a.ts'); - assert.equal(file.deleted, false); - assert.deepEqual(file.added, [1, 2, 12]); -}); - -test('parseUnifiedDiff marks deleted files and keeps rename destinations', () => { - const diff = [ - 'diff --git a/src/gone.ts b/src/gone.ts', - '--- a/src/gone.ts', - '+++ /dev/null', - '@@ -1,1 +0,0 @@', - '-was here', - 'diff --git a/src/old.ts b/src/new.ts', - 'rename from src/old.ts', - 'rename to src/new.ts', - '--- a/src/old.ts', - '+++ b/src/new.ts', - '@@ -3,0 +4,1 @@', - '+added on new path', - ].join('\n'); - const files = parseUnifiedDiff(diff); - const gone = files.find((f) => f.path === 'src/gone.ts'); - assert.equal(gone?.deleted, true); - const renamed = files.find((f) => f.path === 'src/new.ts'); - assert.deepEqual(renamed?.added, [4]); -}); - test('source/test classification matches the coverage include glob', () => { assert.equal(isIncludableSource('src/core/x.ts'), true); assert.equal(isIncludableSource('src/core/x.test.ts'), false); diff --git a/scripts/coverage-changed/model.ts b/scripts/coverage-changed/model.ts index 286a39865d..5ca1110301 100644 --- a/scripts/coverage-changed/model.ts +++ b/scripts/coverage-changed/model.ts @@ -10,7 +10,8 @@ // lcov was dropped by an exclude glob — reported (non-gating) so exclusions // cannot silently absorb new logic. -import { repoRelativePath } from '../lib/repo-path.ts'; +import type { ChangedFileDiff } from '../lib/repo-git.ts'; +import { normalizePath } from '../lib/repo-path.ts'; // Single source of truth for the gate. Changed-line coverage below this // percentage fails the Coverage CI job (unless waived). @@ -27,24 +28,6 @@ export type LcovFile = { export type CoverageIndex = ReadonlyMap; -export type ChangedFileDiff = { - readonly path: string; - // New-side line numbers of '+' lines, ascending and de-duplicated. - readonly added: readonly number[]; - // `+++ /dev/null`: the file was deleted, so it contributes no changed lines. - readonly deleted: boolean; -}; - -// Normalize an lcov/diff path to a repo-relative posix path, then drop the `a/`-style prefixes git -// writes. Relativization itself is shared with the coverage-map keys (scripts/lib/repo-path.ts): a -// key from another checkout must relativize identically here and there, or the two never join. -export function normalizePath(raw: string, rootDir?: string): string { - let p = repoRelativePath(raw, rootDir); - if (p.startsWith('b/')) p = p.slice(2); - if (p.startsWith('./')) p = p.slice(2); - return p; -} - function parseDaRecord(body: string): readonly [number, number] | null { const [lineNo, hits] = body.split(','); const n = Number(lineNo); @@ -90,63 +73,6 @@ export function parseLcov(text: string, rootDir?: string): CoverageIndex { return index; } -type DiffSection = { path: string; added: Set; deleted: boolean; cursor: number }; - -function newStartOfHunk(line: string): number | null { - const match = /@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/.exec(line); - return match ? Number(match[1]) : null; -} - -function pathFromDiffHeader(line: string): string { - const header = /^diff --git a\/.+ b\/(.+)$/.exec(line); - return header ? normalizePath(header[1]!) : ''; -} - -// Fold one line of a file's diff body into its section. Renames+edits report -// their added lines under the destination (`+++ b/...`); pure renames and -// deletions add nothing. `--unified=0` means no context lines in practice. -function foldDiffBodyLine(section: DiffSection, line: string): void { - if (line.startsWith('+++ ')) { - const target = line.slice(4).trim(); - if (target === '/dev/null') section.deleted = true; - else section.path = normalizePath(target); - } else if (line.startsWith('@@')) { - const start = newStartOfHunk(line); - if (start !== null) section.cursor = start; - } else if (line.startsWith('+') && !line.startsWith('+++')) { - section.added.add(section.cursor); - section.cursor += 1; - } else if (line.startsWith('-') || line.startsWith('\\')) { - // Removed line or "\ No newline": occupies no new-side number. - } else { - section.cursor += 1; - } -} - -export function parseUnifiedDiff(diff: string): ChangedFileDiff[] { - const files: ChangedFileDiff[] = []; - let section: DiffSection | null = null; - const flush = (): void => { - if (section && section.path) { - files.push({ - path: section.path, - added: [...section.added].sort((a, b) => a - b), - deleted: section.deleted, - }); - } - }; - for (const line of diff.split('\n')) { - if (line.startsWith('diff --git ')) { - flush(); - section = { path: pathFromDiffHeader(line), added: new Set(), deleted: false, cursor: 0 }; - } else if (section) { - foldDiffBodyLine(section, line); - } - } - flush(); - return files; -} - // Vitest coverage `include` spans two roots: `src/**/*.ts` and // `packages/*/src/**/*.ts`. Test files are excluded there and carry no product // logic, so they never count toward the gate or the excluded tally (which diff --git a/scripts/coverage-changed/run.ts b/scripts/coverage-changed/run.ts index a63f27e6f6..6c0bd3b94f 100644 --- a/scripts/coverage-changed/run.ts +++ b/scripts/coverage-changed/run.ts @@ -16,13 +16,8 @@ import path from 'node:path'; import { pathToFileURL } from 'node:url'; import { runCmdSync } from '@agent-device/host-kit/command'; import { parseScriptArgs } from '../lib/cli-args.ts'; -import { readChangedLinesDiff } from '../lib/repo-git.ts'; -import { - computeChangedCoverage, - parseLcov, - parseUnifiedDiff, - type ChangedCoverageResult, -} from './model.ts'; +import { parseUnifiedDiff, readChangedLinesDiff } from '../lib/repo-git.ts'; +import { computeChangedCoverage, parseLcov, type ChangedCoverageResult } from './model.ts'; const USAGE = 'Usage: pnpm check:coverage-changed [--base ]\n'; const LCOV_PATH = 'coverage/lcov.info'; diff --git a/scripts/coverage-crap/model.ts b/scripts/coverage-crap/model.ts index fdf79452d6..58fdf2765c 100644 --- a/scripts/coverage-crap/model.ts +++ b/scripts/coverage-crap/model.ts @@ -1,7 +1,7 @@ // Pure pieces of the coverage-based CRAP verdict: which Istanbul file records fallow can trust, // whether the two reports of one coverage run agree about what it measured, and how the scored // functions are rendered. run.ts owns the git, fallow, config and file I/O. -import type { ChangedFileDiff } from '../coverage-changed/model.ts'; +import type { ChangedFileDiff } from '../lib/repo-git.ts'; import { repoRelativePath } from '../lib/repo-path.ts'; type IstanbulCounters = Readonly<{ @@ -185,7 +185,7 @@ export type TouchedFile = Readonly<{ path: string; addedLines: number }>; /** * The files whose lines this change edits. A pure rename, delete, or mode change adds no lines, so - * it contributes none — which is also exactly the set of files fallow's `--diff-file` scoping can + * it contributes none — which is also exactly the set of files fallow's `--diff-stdin` scoping can * charge a finding to, since that filter indexes the same added lines. */ export function readTouchedFiles(diffs: readonly ChangedFileDiff[]): TouchedFile[] { diff --git a/scripts/coverage-crap/run.test.ts b/scripts/coverage-crap/run.test.ts index 0062d355c1..c6cde4ea92 100644 --- a/scripts/coverage-crap/run.test.ts +++ b/scripts/coverage-crap/run.test.ts @@ -218,8 +218,8 @@ test('editing a function that already scores too high blocks, even though the sc test("the score of an untouched file is not this change's verdict", () => { // Half of this is the exit code: the map holds `src/debt.ts` at CRAP 42 and the run is still green, // which only happens if fallow filtered its findings by this diff rather than scoring the repo. - // The other half is that diff itself, since a flag that silently went missing would score - // everything and a diff built from the wrong range would look just as clean. + // The other half is that the diff reaches fallow on stdin and never touches the checkout, so a + // `fallow-crap.diff` left by an earlier run can never silently supply a scope. writeCoverage({ 'src/debt.ts': {}, 'src/clean.ts': { hits: 4 } }); write('src/clean.ts', `${TRIVIAL}export const also_trivial = 2;\n`); commit('clean edit, debt left alone'); @@ -228,9 +228,7 @@ test("the score of an untouched file is not this change's verdict", () => { assert.equal(code, 0); assert.doesNotMatch(out, /src\/debt\.ts:1/); - const scoped = fs.readFileSync(path.join(repo, 'coverage/fallow-crap.diff'), 'utf8'); - assert.match(scoped, /^\+\+\+ b\/src\/clean\.ts$/m); - assert.doesNotMatch(scoped, /^\+\+\+ b\/src\/debt\.ts$/m); + assert.ok(!fs.existsSync(path.join(repo, 'coverage/fallow-crap.diff'))); }); test('a pure rename does not touch the functions that moved with the file', () => { @@ -261,7 +259,7 @@ test('a rename that also edits the body owes the edited lines', () => { }); test('deleting lines from a debt function does not charge the function that survives them', () => { - // Measured, not assumed: fallow's `--diff-file` index holds added lines only, so a hunk that is + // Measured, not assumed: fallow's diff-scoping index holds added lines only, so a hunk that is // nothing but removals contributes no line to intersect a function's span with, and this run // agrees rather than overriding it — the same reason the whole-file and pure-rename exemptions // exist. The residual is deliberate and not free: a deletion can even raise a surviving diff --git a/scripts/coverage-crap/run.ts b/scripts/coverage-crap/run.ts index 4105d0dfb2..ad0f494ef2 100644 --- a/scripts/coverage-crap/run.ts +++ b/scripts/coverage-crap/run.ts @@ -16,8 +16,8 @@ import path from 'node:path'; import { pathToFileURL } from 'node:url'; import { runCmdSync } from '@agent-device/host-kit/command'; import { parseScriptArgs } from '../lib/cli-args.ts'; -import { parseLcov, parseUnifiedDiff } from '../coverage-changed/model.ts'; -import { readChangedLinesDiff } from '../lib/repo-git.ts'; +import { parseLcov } from '../coverage-changed/model.ts'; +import { parseUnifiedDiff, readChangedLinesDiff } from '../lib/repo-git.ts'; import { repoRelativePath } from '../lib/repo-path.ts'; import { coverageMapPaths, @@ -42,7 +42,6 @@ const USAGE = 'Usage: pnpm check:coverage-crap [--base | --all]\n'; const COVERAGE_PATH = 'coverage/coverage-istanbul.json'; const LCOV_PATH = 'coverage/lcov.info'; const SANITIZED_PATH = 'coverage/fallow-crap-coverage.json'; -const DIFF_PATH = 'coverage/fallow-crap.diff'; const FALLOW_CONFIG = 'fallow-crap.json'; const CRAP_THRESHOLD = 30; const FALLOW_MAX_BUFFER_BYTES = 64 * 1024 * 1024; @@ -77,15 +76,13 @@ function refuse(reason: string): number { return 1; } -/** - * The diff this verdict is scoped to, written where fallow reads it. Without it there is nothing to - * scope to, and a `fallow-crap.diff` left by an earlier run would silently supply one, so a failure - * here is fatal rather than a scoring of the wrong lines. - */ -function readScopedDiff(root: string, base: string): readonly TouchedFile[] { +/** The diff this verdict is scoped to, as fallow receives it on stdin. */ +function readScopedDiff( + root: string, + base: string, +): { diff: string; touched: readonly TouchedFile[] } { const diff = readChangedLinesDiff(root, base); - fs.writeFileSync(path.resolve(root, DIFF_PATH), diff); - return readTouchedFiles(parseUnifiedDiff(diff)); + return { diff, touched: readTouchedFiles(parseUnifiedDiff(diff)) }; } /** @@ -106,6 +103,8 @@ type CrapInputs = Readonly<{ mapPaths: ReadonlySet; /** Every file the run listed in lcov, or null when it wrote nothing usable. */ enumeratedPaths: ReadonlySet | null; + /** The base...HEAD diff fallow scores against, on stdin; empty in `all` scope. */ + diff: string; touched: readonly TouchedFile[]; /** Records the sanitizer refused, as repo-relative paths. */ dropped: readonly string[]; @@ -134,10 +133,11 @@ function gatherCrapInputs( return { refusal: `${coveragePath}: ${describe(error)}` }; } + let diff = ''; let touched: readonly TouchedFile[] = []; if (scope === 'touched') { try { - touched = readScopedDiff(root, base); + ({ diff, touched } = readScopedDiff(root, base)); } catch (error: unknown) { return { refusal: `the diff against ${base} failed: ${describe(error)}` }; } @@ -152,13 +152,18 @@ function gatherCrapInputs( return { mapPaths: coverageMapPaths(map, root), enumeratedPaths: readEnumeratedPaths(root), + diff, touched, dropped: dropped.map((file) => repoRelativePath(file, root)), }; } /** Scores the sanitized map, scoped to this diff in `touched` scope. */ -function runFallowHealth(root: string, scope: CrapScope): { stdout: string } | { failure: string } { +function runFallowHealth( + root: string, + scope: CrapScope, + diff: string, +): { stdout: string } | { failure: string } { const fallow = runCmdSync( process.execPath, [ @@ -171,13 +176,18 @@ function runFallowHealth(root: string, scope: CrapScope): { stdout: string } | { String(CRAP_THRESHOLD), '--coverage', SANITIZED_PATH, - ...(scope === 'touched' ? ['--diff-file', DIFF_PATH] : []), + ...(scope === 'touched' ? ['--diff-stdin'] : []), '--report-only', '--format', 'json', '--quiet', ], - { cwd: root, allowFailure: true, maxBuffer: FALLOW_MAX_BUFFER_BYTES }, + { + cwd: root, + allowFailure: true, + maxBuffer: FALLOW_MAX_BUFFER_BYTES, + ...(scope === 'touched' ? { stdin: diff } : {}), + }, ); return fallow.exitCode === 0 ? { stdout: fallow.stdout } @@ -201,7 +211,7 @@ function readExemptions(root: string): CrapExemptions { /** Scores what was gathered, prints the verdict, and returns the process exit code. */ function reportCrapVerdict(root: string, scope: CrapScope, inputs: CrapInputs): number { - const health = runFallowHealth(root, scope); + const health = runFallowHealth(root, scope, inputs.diff); if ('failure' in health) { process.stderr.write(`check:coverage-crap: ${health.failure}\n`); return 1; diff --git a/scripts/lib/repo-git.test.ts b/scripts/lib/repo-git.test.ts new file mode 100644 index 0000000000..aa61f52437 --- /dev/null +++ b/scripts/lib/repo-git.test.ts @@ -0,0 +1,43 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { parseUnifiedDiff } from './repo-git.ts'; + +test('parseUnifiedDiff numbers added lines per hunk and ignores deleted lines', () => { + const diff = [ + 'diff --git a/src/a.ts b/src/a.ts', + '--- a/src/a.ts', + '+++ b/src/a.ts', + '@@ -1,0 +1,2 @@', + '+const a = 1;', + '+const b = 2;', + '@@ -10,1 +12,1 @@', + '-old line', + '+new line', + ].join('\n'); + const [file] = parseUnifiedDiff(diff); + assert.equal(file.path, 'src/a.ts'); + assert.equal(file.deleted, false); + assert.deepEqual(file.added, [1, 2, 12]); +}); + +test('parseUnifiedDiff marks deleted files and keeps rename destinations', () => { + const diff = [ + 'diff --git a/src/gone.ts b/src/gone.ts', + '--- a/src/gone.ts', + '+++ /dev/null', + '@@ -1,1 +0,0 @@', + '-was here', + 'diff --git a/src/old.ts b/src/new.ts', + 'rename from src/old.ts', + 'rename to src/new.ts', + '--- a/src/old.ts', + '+++ b/src/new.ts', + '@@ -3,0 +4,1 @@', + '+added on new path', + ].join('\n'); + const files = parseUnifiedDiff(diff); + const gone = files.find((f) => f.path === 'src/gone.ts'); + assert.equal(gone?.deleted, true); + const renamed = files.find((f) => f.path === 'src/new.ts'); + assert.deepEqual(renamed?.added, [4]); +}); diff --git a/scripts/lib/repo-git.ts b/scripts/lib/repo-git.ts index d4ca6e4234..d04a90b670 100644 --- a/scripts/lib/repo-git.ts +++ b/scripts/lib/repo-git.ts @@ -1,4 +1,5 @@ import { runCmdSync } from '@agent-device/host-kit/command'; +import { normalizePath } from './repo-path.ts'; export type RepoGit = (args: string[]) => string; @@ -38,3 +39,73 @@ export function readChangedLinesDiff(repoRoot: string, base: string): string { { cwd: repoRoot, maxBuffer: CHANGED_LINES_MAX_BUFFER_BYTES }, ).stdout; } + +export type ChangedFileDiff = { + readonly path: string; + // New-side line numbers of '+' lines, ascending and de-duplicated. + readonly added: readonly number[]; + // `+++ /dev/null`: the file was deleted, so it contributes no changed lines. + readonly deleted: boolean; +}; + +type DiffSection = { path: string; added: Set; deleted: boolean; cursor: number }; + +function newStartOfHunk(line: string): number | null { + const match = /@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/.exec(line); + return match ? Number(match[1]) : null; +} + +function pathFromDiffHeader(line: string): string { + const header = /^diff --git a\/.+ b\/(.+)$/.exec(line); + return header ? normalizePath(header[1]!) : ''; +} + +// Fold one line of a file's diff body into its section. Renames+edits report +// their added lines under the destination (`+++ b/...`); pure renames and +// deletions add nothing. `--unified=0` means no context lines in practice. +function foldDiffBodyLine(section: DiffSection, line: string): void { + if (line.startsWith('+++ ')) { + const target = line.slice(4).trim(); + if (target === '/dev/null') section.deleted = true; + else section.path = normalizePath(target); + } else if (line.startsWith('@@')) { + const start = newStartOfHunk(line); + if (start !== null) section.cursor = start; + } else if (line.startsWith('+') && !line.startsWith('+++')) { + section.added.add(section.cursor); + section.cursor += 1; + } else if (line.startsWith('-') || line.startsWith('\\')) { + // Removed line or "\ No newline": occupies no new-side number. + } else { + section.cursor += 1; + } +} + +/** + * `readChangedLinesDiff`'s report as per-file added-line sets — the one reading both changed-line + * gates score from. Keep it beside its producer: a hand-built diff must parse the same way the + * real one does. + */ +export function parseUnifiedDiff(diff: string): ChangedFileDiff[] { + const files: ChangedFileDiff[] = []; + let section: DiffSection | null = null; + const flush = (): void => { + if (section && section.path) { + files.push({ + path: section.path, + added: [...section.added].sort((a, b) => a - b), + deleted: section.deleted, + }); + } + }; + for (const line of diff.split('\n')) { + if (line.startsWith('diff --git ')) { + flush(); + section = { path: pathFromDiffHeader(line), added: new Set(), deleted: false, cursor: 0 }; + } else if (section) { + foldDiffBodyLine(section, line); + } + } + flush(); + return files; +} diff --git a/scripts/lib/repo-path.test.ts b/scripts/lib/repo-path.test.ts new file mode 100644 index 0000000000..9c3d312952 --- /dev/null +++ b/scripts/lib/repo-path.test.ts @@ -0,0 +1,10 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { normalizePath } from './repo-path.ts'; + +test('normalizePath strips root, b/ and ./ prefixes and posixifies', () => { + assert.equal(normalizePath('/repo/src/a.ts', '/repo'), 'src/a.ts'); + assert.equal(normalizePath('b/src/a.ts'), 'src/a.ts'); + assert.equal(normalizePath('./src/a.ts'), 'src/a.ts'); + assert.equal(normalizePath(String.raw`src\a.ts`), 'src/a.ts'); +}); diff --git a/scripts/lib/repo-path.ts b/scripts/lib/repo-path.ts index 77802eea9a..5f293aafb7 100644 --- a/scripts/lib/repo-path.ts +++ b/scripts/lib/repo-path.ts @@ -20,3 +20,15 @@ export function repoRelativePath(raw: string, root?: string): string { .split(path.sep) .join('/'); } + +/** + * An lcov record or diff path as a repo-relative posix path: relativize, then drop the `a/`-style + * prefixes git writes. A key from another checkout must normalize identically on both sides of + * every join, or the two never meet. + */ +export function normalizePath(raw: string, rootDir?: string): string { + let p = repoRelativePath(raw, rootDir); + if (p.startsWith('b/')) p = p.slice(2); + if (p.startsWith('./')) p = p.slice(2); + return p; +} From 89bac52f5af76be42ca8846bea9dd2082010b28e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 13 Oct 2026 03:20:29 +0200 Subject: [PATCH 2/3] chore(gates): run the moved diff-parser tests in the coverage-model gate --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b7f3ab1ade..fbc38cbf90 100644 --- a/package.json +++ b/package.json @@ -150,7 +150,7 @@ "check:gate-manifest": "node --experimental-strip-types scripts/gate/check.ts", "check:gate-manifest:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/gate/*.test.ts", "check:coverage-changed": "node --experimental-strip-types scripts/coverage-changed/run.ts", - "check:coverage-changed:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/coverage-changed/model.test.ts scripts/coverage-changed/run.test.ts", + "check:coverage-changed:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/lib/repo-git.test.ts scripts/lib/repo-path.test.ts scripts/coverage-changed/model.test.ts scripts/coverage-changed/run.test.ts", "check:coverage-crap": "node --experimental-strip-types scripts/coverage-crap/run.ts", "check:coverage-crap:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/coverage-crap/model.test.ts scripts/coverage-crap/patches.test.ts scripts/coverage-crap/run.test.ts", "check:layering": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/layering/*.test.ts && node --experimental-strip-types scripts/layering/check.ts", From 3b208bfb0fc51cc99826db07ce60d77880e8f9d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 13 Oct 2026 03:28:25 +0200 Subject: [PATCH 3/3] test(coverage): seed the stale diff the stdin scope must ignore --- scripts/coverage-crap/run.test.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/scripts/coverage-crap/run.test.ts b/scripts/coverage-crap/run.test.ts index c6cde4ea92..ded5fb20be 100644 --- a/scripts/coverage-crap/run.test.ts +++ b/scripts/coverage-crap/run.test.ts @@ -218,17 +218,27 @@ test('editing a function that already scores too high blocks, even though the sc test("the score of an untouched file is not this change's verdict", () => { // Half of this is the exit code: the map holds `src/debt.ts` at CRAP 42 and the run is still green, // which only happens if fallow filtered its findings by this diff rather than scoring the repo. - // The other half is that the diff reaches fallow on stdin and never touches the checkout, so a - // `fallow-crap.diff` left by an earlier run can never silently supply a scope. + // The diff reaches fallow on stdin and never through the checkout, so seed the artifact an + // earlier release wrote with a scope charging the untouched debt file: a gate that read that + // file again would report `src/debt.ts:1` at 42 and exit 1, and one that rewrote it would leave + // different content behind. Scoping on stdin does neither. writeCoverage({ 'src/debt.ts': {}, 'src/clean.ts': { hits: 4 } }); write('src/clean.ts', `${TRIVIAL}export const also_trivial = 2;\n`); commit('clean edit, debt left alone'); + const staleDiff = [ + 'diff --git a/src/debt.ts b/src/debt.ts', + '--- a/src/debt.ts', + '+++ b/src/debt.ts', + '@@ -1,0 +1,1 @@', + '+export const planted = 1;', + ].join('\n'); + write('coverage/fallow-crap.diff', staleDiff); const { code, out } = check(['--base', HEADS[0]!]); assert.equal(code, 0); assert.doesNotMatch(out, /src\/debt\.ts:1/); - assert.ok(!fs.existsSync(path.join(repo, 'coverage/fallow-crap.diff'))); + assert.equal(fs.readFileSync(path.join(repo, 'coverage/fallow-crap.diff'), 'utf8'), staleDiff); }); test('a pure rename does not touch the functions that moved with the file', () => {