From 462826f94720e5ffc0efb389f1a2660ed462d3b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 12 Oct 2026 13:56:58 +0200 Subject: [PATCH] test(coverage): pin what a deletion-only edit is charged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The question was whether an edit that only deletes lines inside a surviving function scoring >= 30 goes uncharged. Reproduced against real fallow with `--diff-file`: it goes uncharged, and it is not a gap in this gate's own attribution. Fallow's diff index holds added lines only — it reports so itself when handed a deletion-only diff — so no line exists to intersect a function's span with, and `readTouchedFiles` agrees with the filter it is priced against rather than overriding it. So the fix is a pin, not a rule change: charging deletions would need fallow's own scoping loosened, which would block every function in a file any line of which changed. Three cases hold the boundary down, each proving with `--all` that the debt really is in the map rather than absent from it: a deletion inside the function, lines added elsewhere in the same file, and deleting the function outright. The first and third pass because removing code retires debt — the same reason the whole-file and pure-rename exemptions exist. A deletion that leaves an untested complex function standing is the residual, and `--all` still lists it. Part of #3414. --- scripts/coverage-crap/run.test.ts | 74 +++++++++++++++++++++++++++---- 1 file changed, 65 insertions(+), 9 deletions(-) diff --git a/scripts/coverage-crap/run.test.ts b/scripts/coverage-crap/run.test.ts index ed0bb0807a..9dc41a4704 100644 --- a/scripts/coverage-crap/run.test.ts +++ b/scripts/coverage-crap/run.test.ts @@ -48,6 +48,8 @@ type CoverageOptions = Readonly<{ hits?: number; /** Emit a count no execution can produce, which the sanitizer drops with the whole record. */ corrupt?: boolean; + /** Record the file with no functions in it, as a run over a deleted function's file reports. */ + noFunctions?: boolean; }>; function istanbulRecord(file: string, options: CoverageOptions): Record { @@ -56,17 +58,19 @@ function istanbulRecord(file: string, options: CoverageOptions): Record { assert.match(out, /\| `src\/moved\.ts:1` score \| 56\.0 \| 7 \| 0% \|/); }); +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 + // nothing but removals contributes no line to intersect a function's span with, and this run + // agrees rather than overriding it. Removing code retires debt instead of adding it, which is the + // rule the whole-file and pure-rename exemptions already follow — the residual is deliberate: a + // deletion that leaves an untested complex function standing is not charged here, and `--all` + // still lists it on main. Pinned so a change to either layer's scoping has to say so. + writeCoverage({ 'src/debt.ts': { span: [1, 13] }, 'src/clean.ts': { hits: 4 } }); + write('src/debt.ts', DEBT.replace(' total += 1;\n', '')); + commit('delete a line inside the debt function'); + + const { code, out } = check(['--base', HEADS[0]!]); + + assert.equal(code, 0); + assert.match(out, /no touched function\(s\) at or above 30/); + // Not a vacuous pass: the same map, scored without the diff, does hold this function at 42. + const backlog = check(['--all']); + assert.equal(backlog.code, 0); + assert.match(backlog.out, /\| `src\/debt\.ts:1` score \| 42\.0 \| 6 \| 0% \|/); +}); + +test('lines added elsewhere in a file do not charge a function whose own lines did not change', () => { + // Scoping is per function span, not per file, so a one-line edit in a large module does not + // inherit every debt score inside it. Here the file has added lines — `helper` is rewritten and + // another export appears below the function — and the function lost one line, yet none of the + // added lines fall in its span, so it is not this change's verdict. + writeCoverage({ 'src/debt.ts': { span: [1, 13] }, 'src/clean.ts': { hits: 4 } }); + write( + 'src/debt.ts', + `${DEBT.replace(' total += 1;\n', '')}\nexport const helper = 2;\nexport const extra = 3;\n`, + ); + commit('drop a line inside the function, add two below it'); + + const { code, out } = check(['--base', HEADS[0]!]); + + assert.equal(code, 0); + assert.doesNotMatch(out, /`src\/debt\.ts:1` score/); + const backlog = check(['--all']); + assert.match(backlog.out, /\| `src\/debt\.ts:1` score \| 42\.0 \| 6 \| 0% \|/); +}); + +test('deleting the debt function altogether is retiring it, not a change to it', () => { + writeCoverage({ 'src/debt.ts': { noFunctions: true }, 'src/clean.ts': { hits: 4 } }); + write('src/debt.ts', TRIVIAL); + commit('replace the debt function with a constant'); + + const { code, out } = check(['--base', HEADS[0]!]); + + assert.equal(code, 0); + assert.match(out, /no touched function\(s\) at or above 30/); +}); + test('a touched file the sanitizer dropped as corrupt yields no verdict', () => { writeCoverage({ 'src/debt.ts': {}, 'src/clean.ts': { corrupt: true } }); write('src/clean.ts', `${TRIVIAL}export const also_trivial = 2;\n`);