From 03a37c7b80da4e7e6d5348a02c5975040e1a8ef7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 12 Oct 2026 10:22:29 +0200 Subject: [PATCH] feat(coverage): decide the touched-function CRAP verdict `check:coverage-crap --base ` scores the functions this change touched against the Istanbul map `pnpm test:coverage` already writes, and says when the report cannot answer that question at all rather than reporting no findings. This commit is the model and the run: the verdict, its reasons, and the renderer. CI keeps reporting only until the layer above flips it. The model owns every reason a run is untrustworthy, including a missing lcov enumeration, so a caller cannot decide that fact by passing an empty set. One relativizer turns a coverage map key, an lcov path, and a diff path into the same key; three that disagreed meant a map from another checkout matched nothing. --- scripts/coverage-changed/model.ts | 15 +- scripts/coverage-crap/model.test.ts | 297 ++++++++++++++++++- scripts/coverage-crap/model.ts | 427 +++++++++++++++++++++++++--- scripts/coverage-crap/run.ts | 238 ++++++++++++++-- scripts/lib/repo-path.ts | 22 ++ 5 files changed, 914 insertions(+), 85 deletions(-) create mode 100644 scripts/lib/repo-path.ts diff --git a/scripts/coverage-changed/model.ts b/scripts/coverage-changed/model.ts index 2efe6f7d9e..286a39865d 100644 --- a/scripts/coverage-changed/model.ts +++ b/scripts/coverage-changed/model.ts @@ -10,6 +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'; + // Single source of truth for the gate. Changed-line coverage below this // percentage fails the Coverage CI job (unless waived). export const CHANGED_LINE_COVERAGE_THRESHOLD = 70; @@ -33,16 +35,11 @@ export type ChangedFileDiff = { readonly deleted: boolean; }; -// Normalize an lcov/diff path to a repo-relative posix path. `rootDir` strips an -// absolute-path prefix some coverage providers emit; `./` and `b/` prefixes are -// dropped so lcov and diff paths join on the same key. +// 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 = raw.trim().replaceAll('\\', '/'); - if (rootDir) { - const root = rootDir.replaceAll('\\', '/').replace(/\/$/, ''); - if (p === root) return ''; - if (p.startsWith(`${root}/`)) p = p.slice(root.length + 1); - } + let p = repoRelativePath(raw, rootDir); if (p.startsWith('b/')) p = p.slice(2); if (p.startsWith('./')) p = p.slice(2); return p; diff --git a/scripts/coverage-crap/model.test.ts b/scripts/coverage-crap/model.test.ts index a53afb4b24..ceacd0004f 100644 --- a/scripts/coverage-crap/model.test.ts +++ b/scripts/coverage-crap/model.test.ts @@ -2,11 +2,20 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import { test } from 'node:test'; import { + coverageMapPaths, + decideCrapVerdict, parseFallowJsonReport, + parseIstanbulCoverageMap, readCrapFindings, + readCrapExemptions, renderCrapAnnotations, renderCrapReport, sanitizeIstanbulCoverage, + type CrapFinding, + type CrapProblem, + type CrapScope, + type CrapVerdict, + type EstimatedFinding, } from './model.ts'; const valid = { s: { 0: 3, 1: 0 }, f: { 0: 1 }, b: { 0: [2, 0] } }; @@ -71,13 +80,42 @@ test('a report without a findings array is an error, not an empty verdict', () = assert.throws(() => readCrapFindings({ error: true }, 30), /no findings array/); }); +/** A verdict that owes nothing: what `decideCrapVerdict` returns when the reports answer cleanly. */ +const clean = (scope: CrapScope, findings: readonly CrapFinding[] = []): CrapVerdict => ({ + blocking: false, + problems: [], + unscoped: [], + offenders: scope === 'touched' ? findings : [], +}); + +/** A report in the shape a case does not care about, so each case states only its difference. */ +const render = (input: Partial[0]> = {}) => + renderCrapReport({ + findings: [], + dropped: [], + threshold: 30, + scope: 'touched', + verdict: clean('touched'), + ...input, + }); + +/** A verdict that blocks or lists problems, for the renderer cases that build one by hand. */ +const verdictWith = ( + problems: readonly CrapProblem[], + unscoped: readonly EstimatedFinding[] = [], +) => ({ + blocking: true, + problems, + unscoped, + offenders: [], +}); + test('the report names touched functions and unscored files', () => { const findings = readCrapFindings({ findings: [finding({})] }, 30); - const report = renderCrapReport({ + const report = render({ findings, dropped: ['src/odd.ts'], - threshold: 30, - scope: 'touched', + verdict: { blocking: true, problems: [], unscoped: [], offenders: findings }, }); assert.match(report, /1 touched function\(s\) at or above 30/); assert.match(report, /\| `src\/a\.ts:10` run \| 42\.0 \| 6 \| 20% \|/); @@ -88,17 +126,266 @@ test('the report names touched functions and unscored files', () => { }); test('an empty report says none and lists no table', () => { - const report = renderCrapReport({ findings: [], dropped: [], threshold: 30, scope: 'touched' }); + const report = render(); assert.match(report, /: no touched function/); assert.doesNotMatch(report, /\| Function/); + // Each section owns its own emptiness guard, so a clean run must not announce empty ones: an + // "Unscored: 0 file(s)" or "exemptions (0)" line reads as a finding to someone scanning the log. + assert.doesNotMatch(report, /Unscored:/); + assert.doesNotMatch(report, /exemptions/i); }); test('a whole-repository report does not claim the functions were touched', () => { const findings = readCrapFindings({ findings: [finding({})] }, 30); - const report = renderCrapReport({ findings, dropped: [], threshold: 30, scope: 'all' }); + const report = render({ + findings, + scope: 'all', + verdict: clean('all'), + }); assert.match(report, /: 1 function\(s\) at or above 30/); }); +test('a touched report whose reports cannot answer names the reason and stays ahead of the table', () => { + const report = render({ + verdict: verdictWith([ + { kind: 'unscored_touched', files: [{ path: 'src/new.ts', addedLines: 4 }] }, + { kind: 'corrupt_touched', files: ['src/odd.ts'] }, + ]), + }); + assert.match(report, /no trustworthy verdict/); + assert.match(report, /no record in the coverage map\*\*: `src\/new\.ts` \(4 changed line\(s\)\)/); + assert.match(report, /impossible counts\*\*: `src\/odd\.ts`/); + // Every problem carries the command that clears it, since the reader has no other way to act. + assert.match(report, /Run `pnpm test:coverage` again/); +}); + +test('an unanswerable touched report never reads as the empty verdict it was computed from', () => { + const report = render({ + verdict: verdictWith([{ kind: 'enumeration_missing' }]), + }); + assert.doesNotMatch(report, /no touched function\(s\)/); + assert.match(report, /\*\*no usable lcov report lists the files the coverage run measured/); +}); + +test('the same gap in the whole-repo report is a note, not a verdict', () => { + const report = render({ + scope: 'all', + verdict: { + ...verdictWith([{ kind: 'map_partial', mapFiles: 9, enumeratedFiles: 12 }]), + blocking: false, + }, + }); + assert.match(report, /report-only\): no function\(s\) at or above 30/); + assert.match(report, /Not gating, but the report is incomplete/); + assert.match(report, /\(9 recorded, 12 enumerated\)/); +}); + +// Only an override that raises `maxCrap` above the default exempts a file: the others are caps, and +// reading a cap as an exemption would hide debt the report is supposed to show. +const CONFIGURED = JSON.stringify({ + health: { + thresholdOverrides: [ + { files: ['src/worker.ts'], maxCrap: 1e12, reason: 'Worker thread.' }, + { files: ['src/cheaper.ts'], maxCrap: 12, reason: 'A cap, not an exemption.' }, + { files: ['src/no-crap.ts'], reason: 'Raises nothing.' }, + ], + }, +}); + +test('the exemptions are read from the resolved config and stated with their file-wide scope', () => { + const exemptions = readCrapExemptions(CONFIGURED); + assert.deepEqual(exemptions.exemptions, [ + { files: ['src/worker.ts'], maxCrap: 1e12, reason: 'Worker thread.' }, + ]); + const report = render({ + scope: 'all', + verdict: clean('all'), + exemptions, + }); + assert.match(report, /exemptions in `fallow-crap\.json` \(1\).*every function in its files/s); + assert.match(report, /- `src\/worker\.ts` — `maxCrap: 1e\+12`: Worker thread\./); +}); + +test('the exemption listing distinguishes none, empty, and unreadable', () => { + // None and unknown are different claims, and an unreadable one must not be a crash: fallow can + // exit 0 and print a warning with no report, and which files are exempt is reporting, not gating. + // Throwing there would turn a clean advisory `--all` run into exit 1. + assert.deepEqual(readCrapExemptions('{"health":{}}'), { available: true, exemptions: [] }); + assert.deepEqual(readCrapExemptions('{"health":{"thresholdOverrides":[]}}'), { + available: true, + exemptions: [], + }); + assert.deepEqual(readCrapExemptions('Error: no config'), { + available: false, + detail: 'fallow printed no JSON report:', + }); +}); + +test('exemptions that could not be read are reported as unknown rather than as none', () => { + const report = render({ + scope: 'all', + verdict: clean('all'), + exemptions: { available: false, detail: 'fallow config exited with code 1' }, + }); + assert.match(report, /could not be read from the config \(fallow config exited with code 1\)/); + assert.doesNotMatch(report, /exemptions in `fallow-crap\.json` \(0\)/); +}); + +/** + * A verdict over one touched file in one measured file, with whatever a case cares about overridden. + * The defaults are the shape of a clean answer, so each case states only its difference. + */ +const decide = (input: Partial[0]> = {}) => + decideCrapVerdict({ + scope: 'touched', + findings: [], + estimated: [], + touched: [{ path: 'src/a.ts', addedLines: 1 }], + dropped: new Set(), + mapPaths: new Set(['src/a.ts']), + enumeratedPaths: new Set(['src/a.ts']), + ...input, + }); + +const kinds = (verdict: CrapVerdict) => verdict.problems.map((problem) => problem.kind); + +test('the verdict blocks a touched function at or above the threshold', () => { + const findings = readCrapFindings({ findings: [finding({})] }, 30); + const verdict = decide({ findings }); + assert.equal(verdict.blocking, true); + assert.deepEqual(verdict.problems, []); + assert.deepEqual(verdict.offenders, findings); +}); + +test('the whole-repo verdict reports the backlog without blocking', () => { + const verdict = decide({ + scope: 'all', + findings: readCrapFindings({ findings: [finding({})] }, 30), + }); + assert.equal(verdict.blocking, false); + assert.deepEqual(verdict.offenders, []); +}); + +test('a touched file the sanitizer dropped is named once, as corrupt', () => { + assert.deepEqual( + kinds( + decide({ + touched: [{ path: 'src/odd.ts', addedLines: 1 }], + dropped: new Set(['src/odd.ts']), + }), + ), + ['corrupt_touched'], + ); +}); + +test('an unmeasured touched function is a problem only in a file the map records', () => { + const estimated = [{ path: 'src/a.ts', name: 'added', line: 40, crap: 42 }]; + // The map has the file but not the function: it predates the code and the score is a guess. + assert.deepEqual(kinds(decide({ estimated })), ['unmeasured_touched']); + // A file entirely outside the map is already reported as unscored, not also as unmeasured — and + // an empty map is partial on its own, so both names appear and neither is a duplicate. + assert.deepEqual(kinds(decide({ estimated, mapPaths: new Set() })), [ + 'map_partial', + 'unscored_touched', + ]); +}); + +test('an estimate in a file coverage never enumerated is listed, not blocked on', () => { + // `scripts/**` and other out-of-scope files are the shape of a normal tooling PR: coverage does not + // instrument them at all, so re-running cannot measure them and the estimate is the only score + // available. Blocking would refuse every such PR; staying silent would hide a number ≥ 30. + const verdict = decide({ + estimated: [{ path: 'scripts/tool.ts', name: 'parse', line: 12, crap: 31.6 }], + touched: [{ path: 'scripts/tool.ts', addedLines: 3 }], + }); + assert.deepEqual(verdict.problems, []); + assert.equal(verdict.blocking, false); + assert.deepEqual(verdict.unscoped, [ + { path: 'scripts/tool.ts', name: 'parse', line: 12, crap: 31.6 }, + ]); + const report = render({ + verdict, + }); + assert.match(report, /no touched function\(s\) at or above 30/); + assert.match( + report, + /Not measured, and not blocking.*`scripts\/tool\.ts:12` parse \(CRAP 31\.6\)/s, + ); +}); + +test('an estimate in a file the map predates stays a blocking problem, not a listing', () => { + const verdict = decide({ estimated: [{ path: 'src/a.ts', name: 'added', line: 40, crap: 42 }] }); + assert.deepEqual(kinds(verdict), ['unmeasured_touched']); + assert.deepEqual(verdict.unscoped, []); + assert.equal(verdict.blocking, true); +}); + +test('an estimate is never called out-of-scope without an enumeration to check it against', () => { + // Without lcov the run cannot tell "coverage does not instrument this" from "the run should have + // measured this and did not" — the latter is the gap it already reports. Listing the file as + // out-of-scope beside `no trustworthy verdict` would answer a question this run cannot ask. + const verdict = decide({ + estimated: [{ path: 'scripts/tool.ts', name: 'parse', line: 12, crap: 31.6 }], + touched: [{ path: 'scripts/tool.ts', addedLines: 3 }], + mapPaths: new Set(), + enumeratedPaths: null, + }); + assert.deepEqual(verdict.unscoped, []); + assert.equal(verdict.blocking, true); + const report = render({ + verdict, + }); + assert.doesNotMatch(report, /does not instrument/); +}); + +test('a map that records every enumerated file is trusted, and none is a partial map', () => { + const both = ['src/a.ts', 'src/b.ts']; + assert.deepEqual(kinds(decide({ mapPaths: new Set(both), enumeratedPaths: new Set(both) })), []); + // One enumerated file the map does not record is enough to call the map incomplete. + assert.deepEqual( + kinds(decide({ mapPaths: new Set(['src/a.ts']), enumeratedPaths: new Set(both) })), + ['map_partial'], + ); +}); + +test('a same-sized map holding the wrong paths is still a partial map', () => { + // Two coverage runs mixed into one directory can produce equal counts over different files, and a + // count comparison would call that complete. The check is per path, so one unrecorded file is + // enough — and it names the touched file rather than only the discrepancy. + const verdict = decide({ + mapPaths: new Set(['src/other.ts', 'src/b.ts']), + enumeratedPaths: new Set(['src/a.ts', 'src/b.ts']), + }); + assert.deepEqual(kinds(verdict), ['map_partial', 'unscored_touched']); + assert.equal(verdict.blocking, true); +}); + +test('a run that recorded no files has no record of what it ran, and no verdict', () => { + // A truncated or malformed lcov parses to nothing rather than failing, and every completeness + // check reads this list: an empty one would silence them all and leave a clean-looking verdict. + assert.deepEqual(kinds(decide({ enumeratedPaths: null })), ['enumeration_missing']); + // The whole-repo backlog is scored from the map alone, so an absent lcov says nothing about it. + assert.deepEqual(kinds(decide({ scope: 'all', enumeratedPaths: null })), []); +}); + +test('a coverage map is only readable as files to counter objects', () => { + assert.deepEqual(parseIstanbulCoverageMap('{"src/a.ts":{"s":{"0":0}}}'), { + 'src/a.ts': { s: { 0: 0 } }, + }); + assert.throws(() => parseIstanbulCoverageMap('not json'), /not valid JSON/); + assert.throws(() => parseIstanbulCoverageMap('[]'), /an array, not a file -> counters object/); + assert.throws(() => parseIstanbulCoverageMap('{"src/a.ts":[]}'), /for src\/a\.ts is object/); + assert.throws(() => parseIstanbulCoverageMap('{"src/a.ts":{"s":[]}}'), /has s as object/); +}); + +test("coverage keys from another checkout do not match this root's files", () => { + const paths = coverageMapPaths( + { '/work/other/src/a.ts': {}, 'src/relative.ts': {} }, + '/work/this', + ); + assert.deepEqual([...paths], ['../other/src/a.ts', 'src/relative.ts']); +}); + test('annotations put each finding on its file and line', () => { const findings = readCrapFindings({ findings: [finding({})] }, 30); assert.equal( diff --git a/scripts/coverage-crap/model.ts b/scripts/coverage-crap/model.ts index a77b0a794e..fdf79452d6 100644 --- a/scripts/coverage-crap/model.ts +++ b/scripts/coverage-crap/model.ts @@ -1,5 +1,8 @@ -// Pure pieces of the coverage-based CRAP report: which Istanbul file records fallow can trust, -// and how the touched functions it scores are rendered. run.ts owns the git, fallow and file I/O. +// 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 { repoRelativePath } from '../lib/repo-path.ts'; type IstanbulCounters = Readonly<{ s?: Readonly>; @@ -9,6 +12,55 @@ type IstanbulCounters = Readonly<{ export type IstanbulCoverageMap = Readonly>; +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * The Istanbul map `pnpm test:coverage` writes, or a failure naming what is wrong with it. + * + * A PR gate decides from this file, so an unreadable one has to be its own outcome rather than an + * empty findings list: `[]` and `{}` both read as "nothing here is complex enough to block". + */ +export function parseIstanbulCoverageMap(text: string): IstanbulCoverageMap { + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch (error: unknown) { + throw new Error( + `coverage map is not valid JSON: ${error instanceof Error ? error.message : String(error)}`, + ); + } + if (!isRecord(parsed)) { + throw new Error( + `coverage map is ${Array.isArray(parsed) ? 'an array' : typeof parsed}, not a file -> counters object`, + ); + } + for (const [file, record] of Object.entries(parsed)) { + if (!isRecord(record)) { + throw new Error(`coverage map record for ${file} is ${typeof record}, not a counters object`); + } + for (const counter of ['s', 'f', 'b'] as const) { + const counts = record[counter]; + if (counts !== undefined && !isRecord(counts)) { + throw new Error( + `coverage map record for ${file} has ${counter} as ${typeof counts}, not a count object`, + ); + } + } + } + return parsed as IstanbulCoverageMap; +} + +/** + * A coverage map's keys as repo-relative paths. Vitest writes absolute ones, so the record's home + * is part of reading the map: a report from another checkout relativizes outside this root and + * matches none of its files, which must not read as "that file was never measured". + */ +export function coverageMapPaths(map: IstanbulCoverageMap, root: string): Set { + return new Set(Object.keys(map).map((key) => repoRelativePath(key, root))); +} + export type SanitizedCoverage = Readonly<{ map: IstanbulCoverageMap; /** Records dropped because a hit count is not a non-negative integer. */ @@ -60,16 +112,15 @@ export type CrapFinding = Readonly<{ }>; function readFinding(value: unknown): CrapFinding | null { - if (typeof value !== 'object' || value === null) return null; - const record = value as Record; - const { path, name, line, crap, cyclomatic } = record; + if (!isRecord(value)) return null; + const { path, name, line, crap, cyclomatic } = value; // Outside the coverage map (tests, scripts, dropped records) fallow estimates from imports. - if (record.coverage_source !== 'istanbul') return null; + if (value.coverage_source !== 'istanbul') return null; if (typeof path !== 'string' || typeof name !== 'string') return null; if (typeof line !== 'number' || typeof crap !== 'number' || typeof cyclomatic !== 'number') { return null; } - const coveragePct = typeof record.coverage_pct === 'number' ? record.coverage_pct : null; + const coveragePct = typeof value.coverage_pct === 'number' ? value.coverage_pct : null; return { path, name, line, crap, cyclomatic, coveragePct }; } @@ -80,21 +131,48 @@ export function parseFallowJsonReport(stdout: string): unknown { return JSON.parse(stdout.slice(start)); } +function readFindings(report: unknown): readonly Record[] { + const findings = isRecord(report) ? report.findings : undefined; + if (!Array.isArray(findings)) throw new Error('fallow health report has no findings array'); + return findings.filter((entry): entry is Record => isRecord(entry)); +} + /** The measured CRAP findings of one `fallow health --format json` report, highest score first. */ export function readCrapFindings(report: unknown, threshold: number): CrapFinding[] { - const findings = - typeof report === 'object' && report !== null - ? (report as { findings?: unknown }).findings - : undefined; - if (!Array.isArray(findings)) { - throw new Error('fallow health report has no findings array'); - } - return findings + return readFindings(report) .map(readFinding) .filter((finding): finding is CrapFinding => finding !== null && finding.crap >= threshold) .sort((left, right) => right.crap - left.crap || left.path.localeCompare(right.path)); } +/** A function at or above the threshold that fallow scored without an Istanbul record. */ +export type EstimatedFinding = Readonly<{ + path: string; + name: string; + line: number; + crap: number; +}>; + +/** + * The functions reported at or above the threshold that fallow scored from complexity alone: it + * emits `coverage_source: 'estimated'` when a function's definition is not in the coverage data, so + * in a file the map *does* record the map predates the function. Such a score is not a measurement, + * and cannot settle whether a touched function is affordable. + */ +export function readEstimatedFindings(report: unknown, threshold: number): EstimatedFinding[] { + return readFindings(report) + .filter((entry) => entry.coverage_source !== 'istanbul') + .filter((entry) => typeof entry.crap === 'number' && entry.crap >= threshold) + .filter((entry) => typeof entry.path === 'string' && typeof entry.name === 'string') + .map((entry) => ({ + path: entry.path as string, + name: entry.name as string, + line: typeof entry.line === 'number' ? entry.line : 0, + crap: entry.crap as number, + })) + .sort((left, right) => left.path.localeCompare(right.path) || left.line - right.line); +} + function formatCoverage(pct: number | null): string { return pct === null ? 'n/a' : `${pct.toFixed(0)}%`; } @@ -102,39 +180,308 @@ function formatCoverage(pct: number | null): string { /** `touched` scores the functions a diff changes; `all` scores every function in the map. */ export type CrapScope = 'touched' | 'all'; +/** A source file the diff adds lines to, and how many. */ +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 + * charge a finding to, since that filter indexes the same added lines. + */ +export function readTouchedFiles(diffs: readonly ChangedFileDiff[]): TouchedFile[] { + return diffs + .filter((diff) => !diff.deleted && diff.added.length > 0) + .map((diff) => ({ path: diff.path, addedLines: diff.added.length })); +} + +/** A touched file the coverage run enumerated but recorded no counters for. */ +export type UnscoredTouchedFile = Readonly<{ path: string; addedLines: number }>; + +/** + * Why this run cannot trust what it scored. Keyed, never inferred from error text: the report and + * the remediation both branch on `kind`, and prose is the one place a reason could be reworded + * away. A missing or unreadable map is not one of these — run.ts fails before computing a verdict, + * because there is no report left to reason about. + */ +export type CrapProblem = + | Readonly<{ kind: 'enumeration_missing' }> + | Readonly<{ kind: 'map_partial'; mapFiles: number; enumeratedFiles: number }> + | Readonly<{ kind: 'corrupt_touched'; files: readonly string[] }> + | Readonly<{ kind: 'unscored_touched'; files: readonly UnscoredTouchedFile[] }> + | Readonly<{ kind: 'unmeasured_touched'; files: readonly EstimatedFinding[] }>; + +const PROBLEM_MESSAGES: Readonly> = { + enumeration_missing: + 'no usable lcov report lists the files the coverage run measured (coverage/lcov.info)', + map_partial: 'the Istanbul coverage map records fewer files than the same run enumerated', + corrupt_touched: 'touched file(s) were dropped from the map for impossible counts', + unscored_touched: 'touched file(s) with changed lines have no record in the coverage map', + unmeasured_touched: + 'touched function(s) are absent from the coverage map, so their score is estimated from complexity alone', +}; + +const PROBLEM_FIXES: Readonly> = { + enumeration_missing: + 'Run `pnpm test:coverage` again: it writes coverage/lcov.info beside the map.', + map_partial: 'Run `pnpm test:coverage` again on this commit; coverage/ mixes reports.', + corrupt_touched: + 'Run `pnpm test:coverage` again on this commit. If the counts stay impossible, that file is unscored, not clean.', + unscored_touched: + 'Run `pnpm test:coverage` again on this commit. A file the run never loaded has no measured score.', + unmeasured_touched: + 'Run `pnpm test:coverage` again: the map predates these functions, so only their complexity is known.', +}; + +function describeProblemFile(file: string | UnscoredTouchedFile | EstimatedFinding): string { + if (typeof file === 'string') return `\`${file}\``; + return 'addedLines' in file + ? `\`${file.path}\` (${file.addedLines} changed line(s))` + : `\`${file.path}:${file.line}\` ${file.name}`; +} + +function renderProblems(problems: readonly CrapProblem[]): string[] { + return problems.map((problem) => { + const counts = + 'mapFiles' in problem + ? ` (${problem.mapFiles} recorded, ${problem.enumeratedFiles} enumerated)` + : ''; + const files = + 'files' in problem ? `: ${problem.files.map(describeProblemFile).join(', ')}` : ''; + return `- **${PROBLEM_MESSAGES[problem.kind]}**${counts}${files}\n ${PROBLEM_FIXES[problem.kind]}`; + }); +} + +export type CrapVerdict = Readonly<{ + blocking: boolean; + problems: readonly CrapProblem[]; + /** Estimates the report cannot measure, listed for the reader without blocking on them. */ + unscoped: readonly EstimatedFinding[]; + offenders: readonly CrapFinding[]; +}>; + +/** + * The verdict for one report. `touched` scope blocks on a function at or above the threshold and on + * any reason the report cannot be trusted to answer, because `[]` from a partial map looks exactly + * like a clean branch. `all` scope is the backlog report on `main`, which carries debt on purpose: it + * lists what it could not verify without blocking on it. `enumeratedPaths` is null when the run wrote + * no usable lcov report, which from here means nothing was recorded as measured. + */ +export function decideCrapVerdict(input: { + scope: CrapScope; + findings: readonly CrapFinding[]; + estimated: readonly EstimatedFinding[]; + touched: readonly TouchedFile[]; + dropped: ReadonlySet; + mapPaths: ReadonlySet; + enumeratedPaths: ReadonlySet | null; +}): CrapVerdict { + const { enumeratedPaths } = input; + const problems: CrapProblem[] = []; + const corrupt = input.touched + .filter((file) => input.dropped.has(file.path)) + .map((file) => file.path); + if (corrupt.length > 0) problems.push({ kind: 'corrupt_touched', files: corrupt }); + // The enumeration is what answers "what did this run measure", the question the per-file verdict + // cannot do without. The whole-repo backlog is scored from the map alone, so an absent lcov says + // nothing about it and must not be called incomplete. + if (enumeratedPaths === null && input.scope === 'touched') { + problems.push({ kind: 'enumeration_missing' }); + } + + // Both checks below read the enumeration, and without it enumeration_missing above is the whole + // story. lcov and the Istanbul map are two reports of one coverage run, so every file the run + // enumerated has to be recorded in the map. The test is per path rather than a count: a map of the + // right size holding the wrong paths — one file measured, another dropped, both reported as 1,724 + // entries — is exactly what a report mixed from two coverage runs looks like, and counting would + // trust it. + if (enumeratedPaths !== null) { + const unrecorded = new Set([...enumeratedPaths].filter((file) => !input.mapPaths.has(file))); + if (unrecorded.size > 0) { + problems.push({ + kind: 'map_partial', + mapFiles: input.mapPaths.size, + enumeratedFiles: enumeratedPaths.size, + }); + } + + // Of those, the ones this diff touches are what the verdict was going to score. Records the + // sanitizer dropped are named as corrupt above and stay in `mapPaths`, so they are not also here. + const unscored = input.touched + .filter((file) => unrecorded.has(file.path)) + .map((file) => ({ path: file.path, addedLines: file.addedLines })); + if (unscored.length > 0) problems.push({ kind: 'unscored_touched', files: unscored }); + } + + // A touched file the map does record, with a function in it that the map does not: the map is + // older than the code and fallow is estimating that function from its complexity. + const touchedPaths = new Set(input.touched.map((file) => file.path)); + const unmeasured = input.estimated.filter( + (finding) => touchedPaths.has(finding.path) && input.mapPaths.has(finding.path), + ); + if (unmeasured.length > 0) problems.push({ kind: 'unmeasured_touched', files: unmeasured }); + + // The same estimate for a file neither report knows is a different fact: coverage does not measure + // that file at all (scripts, a package the coverage config excludes), so no re-run would score it and + // it is not debt this PR owes. It is still not a measurement, so the report says so rather than let + // the headline imply the function was checked and cleared. Files the run enumerated but did not + // record are named as unscored above, and stay out of this list. Only while the enumeration exists: + // without it, "coverage does not instrument this" is a claim this run cannot make. + const unscoped = enumeratedPaths + ? input.estimated.filter( + (finding) => + touchedPaths.has(finding.path) && + !input.mapPaths.has(finding.path) && + !enumeratedPaths.has(finding.path), + ) + : []; + + // In `touched` scope fallow has already narrowed the findings to functions whose lines the diff + // adds, so nothing is left to re-filter here. + return { + blocking: input.scope === 'touched' && (input.findings.length > 0 || problems.length > 0), + problems, + unscoped, + offenders: input.scope === 'touched' ? input.findings : [], + }; +} + +/** A file-wide `maxCrap` override of `fallow-crap.json`, read from the resolved config. */ +export type CrapExemption = Readonly<{ files: readonly string[]; maxCrap: number; reason: string }>; + +/** + * Resolved-config stdout as the file-wide CRAP exemptions: the overrides that raise `maxCrap` for + * named files, so every function inside them is exempt — not only the out-of-process ones that + * motivated the override. Which files those are is something the report states or admits it cannot + * state, so failure is admitted too rather than thrown: a fallow that exits 0 and prints nothing + * parseable must not turn the advisory `--all` run red. + */ +export function readCrapExemptions(stdout: string): CrapExemptions { + try { + const report = parseFallowJsonReport(stdout); + const overrides = + isRecord(report) && isRecord(report.health) ? report.health.thresholdOverrides : undefined; + if (!Array.isArray(overrides)) return { available: true, exemptions: [] }; + return { + available: true, + exemptions: overrides + .filter((override): override is Record => isRecord(override)) + .filter((override) => typeof override.maxCrap === 'number' && override.maxCrap > 30) + .map((override) => ({ + files: Array.isArray(override.files) + ? override.files.filter((file): file is string => typeof file === 'string') + : [], + maxCrap: override.maxCrap as number, + reason: typeof override.reason === 'string' ? override.reason : '', + })), + }; + } catch (error: unknown) { + return { + available: false, + detail: error instanceof Error ? error.message.split('\n')[0]! : String(error), + }; + } +} + +/** The exemptions, or how they failed to load — "none" and "unknown" are different claims. */ +export type CrapExemptions = + | Readonly<{ available: true; exemptions: readonly CrapExemption[] }> + | Readonly<{ available: false; detail: string }>; + export function renderCrapReport(input: { findings: readonly CrapFinding[]; dropped: readonly string[]; threshold: number; scope: CrapScope; + verdict: CrapVerdict; + exemptions?: CrapExemptions; }): string { - const { findings, dropped, threshold, scope } = input; - const subject = scope === 'touched' ? 'touched function(s)' : 'function(s)'; - const lines = [ - `## Coverage-based CRAP (report-only): ${findings.length === 0 ? 'no' : findings.length} ${subject} at or above ${threshold}`, - '', - ]; - if (findings.length > 0) { - lines.push('| Function | CRAP | Cyclomatic | Coverage |', '| --- | ---: | ---: | ---: |'); - for (const finding of findings) { - lines.push( + const { findings, dropped, threshold, scope, verdict } = input; + // A PR fails for two different reasons and the reader must not confuse them: debt a touched + // function owes, and a report that cannot answer. Only the second hides the count. + const unanswerable = scope === 'touched' && verdict.problems.length > 0; + const mode = scope === 'touched' ? `blocking at ${threshold}` : 'report-only'; + const headline = unanswerable + ? 'no trustworthy verdict' + : `${findings.length === 0 ? 'no' : findings.length} ${scope === 'touched' ? 'touched function(s)' : 'function(s)'} at or above ${threshold}`; + return `${[ + `## Coverage-based CRAP (${mode}): ${headline}`, + ...renderProblemSection(verdict.problems, unanswerable), + ...renderFindingSection(findings), + ...renderUnscopedSection(verdict.unscoped), + ...renderDroppedSection(dropped), + ...renderExemptionSection(input.exemptions), + ].join('\n')}\n`; +} + +/** + * Estimates the coverage run could not measure because the file is outside what it instruments. Not a + * problem — a re-run changes nothing — but not a measurement either, and a bare "no touched + * function(s)" headline would imply the reader was checked. + */ +function renderUnscopedSection(unscoped: readonly EstimatedFinding[]): readonly string[] { + if (unscoped.length === 0) return []; + return section([ + `Not measured, and not blocking: ${unscoped.length} touched function(s) in file(s) coverage does not instrument, scored from complexity alone: ${unscoped + .map( + (finding) => + `\`${finding.path}:${finding.line}\` ${finding.name} (CRAP ${finding.crap.toFixed(1)})`, + ) + .join(', ')}`, + ]); +} + +/** A section of the report, or nothing. Each owns the blank line that separates it above. */ +function section(content: readonly string[]): readonly string[] { + return content.length === 0 ? [] : ['', ...content]; +} + +function renderProblemSection(problems: readonly CrapProblem[], unanswerable: boolean) { + if (problems.length === 0) return []; + return section([ + ...(unanswerable + ? ['The reports this verdict would rest on do not answer the question:', ''] + : ['Not gating, but the report is incomplete:', '']), + ...renderProblems(problems), + ]); +} + +function renderFindingSection(findings: readonly CrapFinding[]): readonly string[] { + if (findings.length === 0) return []; + return section([ + '| Function | CRAP | Cyclomatic | Coverage |', + '| --- | ---: | ---: | ---: |', + ...findings.map( + (finding) => `| \`${finding.path}:${finding.line}\` ${finding.name} | ${finding.crap.toFixed(1)} | ${finding.cyclomatic} | ${formatCoverage(finding.coveragePct)} |`, - ); - } - lines.push( - '', - 'Add tests for the uncovered branches, or split the function. Code that only runs out of process (workers, browser-evaluated scripts, child processes) measures 0%.', - ); - } - if (dropped.length > 0) { - lines.push( - '', - `Unscored: ${dropped.length} file(s) with counts no execution can produce ` + - `(for example, an unresolved script wrapper offset or a negative branch path count, ` + - `both fixed under patches/): ${dropped.map((file) => `\`${file}\``).join(', ')}`, - ); + ), + '', + 'Add tests for the uncovered branches, or split the function. Code that only runs out of process (workers, browser-evaluated scripts, child processes) measures 0%.', + ]); +} + +function renderDroppedSection(dropped: readonly string[]): readonly string[] { + if (dropped.length === 0) return []; + return section([ + `Unscored: ${dropped.length} file(s) with counts no execution can produce ` + + `(for example, an unresolved script wrapper offset or a negative branch path count, ` + + `both fixed under patches/): ${dropped.map((file) => `\`${file}\``).join(', ')}`, + ]); +} + +function renderExemptionSection(exemptions: CrapExemptions | undefined): readonly string[] { + if (!exemptions) return []; + if (!exemptions.available) { + return section([ + `File-wide CRAP exemptions could not be read from the config (${exemptions.detail}), so this report cannot say which files are exempt.`, + ]); } - return `${lines.join('\n')}\n`; + return section([ + `File-wide CRAP exemptions in \`fallow-crap.json\` (${exemptions.exemptions.length}). Each exempts every function in its files, including helpers Node could test:`, + ...exemptions.exemptions.map( + (exemption) => + `- ${exemption.files.map((file) => `\`${file}\``).join(', ')} — \`maxCrap: ${exemption.maxCrap.toExponential(0)}\`${exemption.reason ? `: ${exemption.reason}` : ''}`, + ), + ]); } // Workflow-command escaping (actions/toolkit `command.ts`): data may not carry `%`, CR, or LF, and diff --git a/scripts/coverage-crap/run.ts b/scripts/coverage-crap/run.ts index ca7291c603..d5a1330caf 100644 --- a/scripts/coverage-crap/run.ts +++ b/scripts/coverage-crap/run.ts @@ -1,63 +1,178 @@ -// Entry point for the coverage-based CRAP report: `pnpm check:coverage-crap [--base | --all]`. +// Entry point for the coverage-based CRAP verdict: `pnpm check:coverage-crap [--base | --all]`. // // Reuses the Istanbul map `pnpm test:coverage` already wrote (never runs coverage a second time), // drops records with impossible counts, and asks fallow to score the functions the diff against -// touches, or every function with --all. Report-only: it prints the functions at or above -// the threshold to stdout and the GitHub job summary (touched functions also as warning -// annotations), and exits 0 once it has a verdict. +// touches, or every function with --all. +// +// `--base` (a PR) blocks: it exits 1 when a touched function scores at or above the threshold, and +// also when the reports cannot be trusted to answer — a missing or malformed map, a map with fewer +// files than the same run enumerated, or a touched file the map dropped or never recorded. `--all` +// (the `main` report) stays report-only, because `main` carries debt on purpose: it exits 0 once it +// has a report to show. Attribution is rename-aware — `readChangedLinesDiff` pairs a pure move at +// 90%, so relocating a file owes nothing for the functions that moved with it. import fs from 'node:fs'; 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 { repoRelativePath } from '../lib/repo-path.ts'; import { + coverageMapPaths, + decideCrapVerdict, parseFallowJsonReport, + parseIstanbulCoverageMap, + readCrapExemptions, readCrapFindings, + readEstimatedFindings, + readTouchedFiles, renderCrapAnnotations, renderCrapReport, sanitizeIstanbulCoverage, + type CrapExemptions, + type CrapFinding, type CrapScope, type IstanbulCoverageMap, + type TouchedFile, } from './model.ts'; 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; -export function run(argv: readonly string[], cwd?: string): number { - const values = parseScriptArgs(argv, USAGE, { - base: { type: 'string', default: 'origin/main' }, - all: { type: 'boolean', default: false }, - }); - const base = values.base ?? 'origin/main'; - const scope: CrapScope = values.all ? 'all' : 'touched'; - const root = cwd ?? runCmdSync('git', ['rev-parse', '--show-toplevel']).stdout.trim(); +// This repository's own pinned fallow, beside the checkout this script was loaded from. Resolving +// it from the analyzed root instead would let a second checkout supply a different version. +const PINNED_FALLOW_BIN = path.resolve( + import.meta.dirname, + '..', + '..', + 'node_modules', + 'fallow', + 'bin', + 'fallow', +); + +function firstLine(text: string): string { + return text.trim().split('\n')[0] ?? 'no output'; +} + +/** Drops an `AppError` prefix so the report names the cause rather than the wrapper. */ +function describe(error: unknown): string { + return error instanceof Error ? firstLine(error.message) : String(error); +} + +/** + * Says out loud that no verdict was reached. These cases exit before a report exists, so they go to + * stderr rather than heading a table of nothing, which would read as a clean result in the logs. + */ +function refuse(reason: string): number { + process.stderr.write(`check:coverage-crap: no trustworthy verdict — ${reason}\n`); + 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[] { + const diff = readChangedLinesDiff(root, base); + fs.writeFileSync(path.resolve(root, DIFF_PATH), diff); + return readTouchedFiles(parseUnifiedDiff(diff)); +} + +/** + * The files the coverage run enumerated, per lcov, or null when it wrote no record of that. A run + * that lists no files cannot be that record either, and reaching the verdict as an empty set would + * satisfy every completeness check at once. + */ +function readEnumeratedPaths(root: string): Set | null { + const lcovPath = path.resolve(root, LCOV_PATH); + if (!fs.existsSync(lcovPath)) return null; + const enumerated = new Set(parseLcov(fs.readFileSync(lcovPath, 'utf8'), root).keys()); + return enumerated.size > 0 ? enumerated : null; +} + +/** What one coverage run and one diff say, as far as they can be read at all. */ +type CrapInputs = Readonly<{ + /** Every file the run recorded counters for, including records dropped as corrupt below. */ + mapPaths: ReadonlySet; + /** Every file the run listed in lcov, or null when it wrote nothing usable. */ + enumeratedPaths: ReadonlySet | null; + touched: readonly TouchedFile[]; + /** Records the sanitizer refused, as repo-relative paths. */ + dropped: readonly string[]; +}>; + +/** + * Reads the two reports of one coverage run plus this change's diff, or names the reason there is + * nothing to decide from. A verdict computed from an unreadable map would read as "no findings", + * which is indistinguishable from a clean branch, so those cases stop here. + */ +function gatherCrapInputs( + root: string, + base: string, + scope: CrapScope, +): CrapInputs | { refusal: string } { const coveragePath = path.resolve(root, COVERAGE_PATH); if (!fs.existsSync(coveragePath)) { - process.stderr.write( - `check:coverage-crap: no Istanbul coverage map at ${coveragePath}. Run \`pnpm test:coverage\` first.\n`, - ); - return 1; + return { + refusal: `no Istanbul coverage map at ${coveragePath}. Run \`pnpm test:coverage\` first.`, + }; + } + let map: IstanbulCoverageMap; + try { + map = parseIstanbulCoverageMap(fs.readFileSync(coveragePath, 'utf8')); + } catch (error: unknown) { + return { refusal: `${coveragePath}: ${describe(error)}` }; } - const { map, dropped } = sanitizeIstanbulCoverage( - JSON.parse(fs.readFileSync(coveragePath, 'utf8')) as IstanbulCoverageMap, - ); - fs.writeFileSync(path.resolve(root, SANITIZED_PATH), JSON.stringify(map)); + let touched: readonly TouchedFile[] = []; if (scope === 'touched') { - fs.writeFileSync(path.resolve(root, DIFF_PATH), readChangedLinesDiff(root, base)); + try { + touched = readScopedDiff(root, base); + } catch (error: unknown) { + return { refusal: `the diff against ${base} failed: ${describe(error)}` }; + } } + const { map: sanitized, dropped } = sanitizeIstanbulCoverage(map); + fs.writeFileSync(path.resolve(root, SANITIZED_PATH), JSON.stringify(sanitized)); + + // lcov is the same coverage run's own list of the files it enumerated, which is what makes a + // touched file absent from the map detectable. Only the per-file verdict needs it: the whole-repo + // backlog is scored from the map alone. + // lcov is the same coverage run's own list of the files it enumerated, which is what makes a + // touched file absent from the map detectable. Whether a missing one is worth reporting is the + // verdict's call, not this reader's. + return { + mapPaths: coverageMapPaths(map, root), + enumeratedPaths: readEnumeratedPaths(root), + touched, + dropped: dropped.map((file) => repoRelativePath(file, root)), + }; +} + +/** + * Scores the sanitized map, scoped to this diff in `touched` scope. `fallowBin` is a parameter rather + * than the constant so a test can assert these flags against a stand-in that reports a chosen finding. + */ +function runFallowHealth( + root: string, + scope: CrapScope, + fallowBin: string, +): { stdout: string } | { failure: string } { const fallow = runCmdSync( process.execPath, [ - path.join(root, 'node_modules', 'fallow', 'bin', 'fallow'), + fallowBin, 'health', '--config', FALLOW_CONFIG, @@ -74,24 +189,85 @@ export function run(argv: readonly string[], cwd?: string): number { ], { cwd: root, allowFailure: true, maxBuffer: FALLOW_MAX_BUFFER_BYTES }, ); - if (fallow.exitCode !== 0) { - process.stderr.write(`check:coverage-crap: fallow health failed:\n${fallow.stderr}\n`); + return fallow.exitCode === 0 + ? { stdout: fallow.stdout } + : { failure: `fallow health failed:\n${fallow.stderr}` }; +} + +/** + * The exemptions come from the resolved config, not from a copy in this diff: the report then states + * the scope fallow actually applied. Reading them is reporting, not gating, so a failure says so. + */ +function readExemptions(root: string, fallowBin: string): CrapExemptions { + const config = runCmdSync( + process.execPath, + [fallowBin, 'config', '--config', FALLOW_CONFIG, '--format', 'json'], + { cwd: root, allowFailure: true, maxBuffer: FALLOW_MAX_BUFFER_BYTES }, + ); + return config.exitCode === 0 + ? readCrapExemptions(config.stdout) + : { available: false, detail: firstLine(config.stderr) }; +} + +/** Scores what was gathered, prints the verdict, and returns the process exit code. */ +function reportCrapVerdict( + root: string, + scope: CrapScope, + inputs: CrapInputs, + fallowBin: string, +): number { + const health = runFallowHealth(root, scope, fallowBin); + if ('failure' in health) { + process.stderr.write(`check:coverage-crap: ${health.failure}\n`); return 1; } - const findings = readCrapFindings(parseFallowJsonReport(fallow.stdout), CRAP_THRESHOLD); - const report = renderCrapReport({ + const report = parseFallowJsonReport(health.stdout); + const findings = readCrapFindings(report, CRAP_THRESHOLD); + const verdict = decideCrapVerdict({ + scope, + findings, + estimated: readEstimatedFindings(report, CRAP_THRESHOLD), + touched: inputs.touched, + dropped: new Set(inputs.dropped), + // Pre-sanitizer on purpose: a record the sanitizer dropped is reported as corrupt, and a file + // the run measured is not also missing from the map. + mapPaths: inputs.mapPaths, + enumeratedPaths: inputs.enumeratedPaths, + }); + const rendered = renderCrapReport({ findings, - dropped: dropped.map((file) => path.relative(root, file)), + dropped: inputs.dropped, threshold: CRAP_THRESHOLD, scope, + verdict, + exemptions: readExemptions(root, fallowBin), }); - process.stdout.write(report); + publish(rendered, scope, verdict.offenders); + return verdict.blocking ? 1 : 0; +} + +/** The report goes to stdout, to the job summary, and — the debt only — onto the PR's diff. */ +function publish(rendered: string, scope: CrapScope, offenders: readonly CrapFinding[]): void { + process.stdout.write(rendered); const summaryPath = process.env.GITHUB_STEP_SUMMARY; - if (summaryPath) fs.appendFileSync(summaryPath, report); + if (summaryPath) fs.appendFileSync(summaryPath, rendered); if (scope === 'touched' && process.env.GITHUB_ACTIONS === 'true') { - process.stdout.write(renderCrapAnnotations(findings)); + process.stdout.write(renderCrapAnnotations(offenders)); } - return 0; +} + +export function run(argv: readonly string[], cwd?: string, fallowBin = PINNED_FALLOW_BIN): number { + const values = parseScriptArgs(argv, USAGE, { + base: { type: 'string', default: 'origin/main' }, + all: { type: 'boolean', default: false }, + }); + const base = values.base ?? 'origin/main'; + const scope: CrapScope = values.all ? 'all' : 'touched'; + const root = cwd ?? runCmdSync('git', ['rev-parse', '--show-toplevel']).stdout.trim(); + + const inputs = gatherCrapInputs(root, base, scope); + if ('refusal' in inputs) return refuse(inputs.refusal); + return reportCrapVerdict(root, scope, inputs, fallowBin); } // exitCode, not exit(): a large report written to a pipe must drain before the process ends. diff --git a/scripts/lib/repo-path.ts b/scripts/lib/repo-path.ts new file mode 100644 index 0000000000..32dd4408b6 --- /dev/null +++ b/scripts/lib/repo-path.ts @@ -0,0 +1,22 @@ +// The one rule for turning a path a tool handed us into a repo-relative posix path. Coverage maps, +// lcov reports and unified diffs each arrive with their own idea of a path — Vitest writes absolute +// keys, lcov writes paths relative to where the run started, git prefixes both sides — and they only +// join if relativization has exactly one implementation. Two that disagree silently, and a map from +// another checkout reads as "no file was measured". + +import path from 'node:path'; + +/** + * `raw` as seen from `root`, in posix form. + * + * An absolute path outside the root comes back with `../` segments rather than staying absolute: it + * matches no file in this checkout, and it does so identically for every caller. + */ +export function repoRelativePath(raw: string, root?: string): string { + const posix = raw.trim().split('\\').join('/'); + if (!root || !path.isAbsolute(posix)) return posix; + return path + .relative(root.replace(/[\\/]$/, ''), posix) + .split(path.sep) + .join('/'); +}