From 2d45b2330680bcb119f7860faefb7975e11284e9 Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 15 Sep 2026 03:02:20 +0000 Subject: [PATCH 1/2] fix(ci): validate coverage lanes dynamically --- .../__tests__/coverage-contract.spec.mjs | 93 +++++++++++++++++++ src/scripts/coverage-contract.mjs | 64 +++++++++++++ src/scripts/verify-coverage-contract.mjs | 21 ++--- 3 files changed, 165 insertions(+), 13 deletions(-) create mode 100644 src/scripts/__tests__/coverage-contract.spec.mjs create mode 100644 src/scripts/coverage-contract.mjs diff --git a/src/scripts/__tests__/coverage-contract.spec.mjs b/src/scripts/__tests__/coverage-contract.spec.mjs new file mode 100644 index 0000000000..3ff2791fd9 --- /dev/null +++ b/src/scripts/__tests__/coverage-contract.spec.mjs @@ -0,0 +1,93 @@ +import { describe, expect, it } from "vitest" + +import { mergeCoverageSources, parseCoverageSourceLines } from "../coverage-contract.mjs" + +const coverage = (records) => + records + .map( + ([source, lines]) => + `SF:${source}\n${lines.map((line) => `DA:${line},1`).join("\n")}\nLF:${lines.length}\nend_of_record`, + ) + .join("\n") + +const parse = (records, lane) => parseCoverageSourceLines(coverage(records), lane) + +describe("coverage source equivalence", () => { + it("accepts legitimate changes to the instrumented source population", () => { + const before = [ + ["src/a.ts", [1]], + ["src/b.ts", [1]], + ] + const after = [ + ["src/a.ts", [1, 2]], + ["src/b.ts", [1]], + ] + + expect(() => + mergeCoverageSources( + ["api", "core"], + [ + ["api", parse(after, "api")], + ["core", parse([["src/a.ts", [1, 2]]], "core")], + ], + ), + ).not.toThrow() + expect([...parse(after, "api").values()].reduce((sum, lines) => sum + lines.size, 0)).toBe( + [...parse(before, "api").values()].reduce((sum, lines) => sum + lines.size, 0) + 1, + ) + }) + + it("rejects omitted lane coverage", () => { + expect(() => mergeCoverageSources(["api", "core"], [["api", parse([["src/a.ts", [1]]], "api")]])).toThrow( + "Coverage lane is missing: core", + ) + }) + + it("rejects duplicated lane coverage", () => { + expect(() => + mergeCoverageSources( + ["api"], + [ + ["api", parse([["src/a.ts", [1]]], "api")], + ["api", parse([["src/a.ts", [1]]], "api")], + ], + ), + ).toThrow("Coverage lane is duplicated: api") + }) + + it("rejects duplicate source records within a lane", () => { + expect(() => + parse( + [ + ["src/a.ts", [1]], + ["src/a.ts", [1]], + ], + "api", + ), + ).toThrow("api coverage contains duplicate source record: src/a.ts") + }) + + it("rejects unfinished source records", () => { + expect(() => parseCoverageSourceLines("SF:src/a.ts\nDA:1,1\nSF:src/b.ts\nLF:1", "api")).toThrow( + "api coverage contains an unfinished source record: src/a.ts", + ) + expect(() => parseCoverageSourceLines("SF:src/a.ts\nDA:1,1\n", "api")).toThrow( + "api coverage contains an unfinished source record: src/a.ts", + ) + expect(() => parseCoverageSourceLines("SF:src/a.ts\nDA:1,1\nLF:1\n", "api")).toThrow( + "api coverage contains an unfinished source record: src/a.ts", + ) + }) + + it("rejects conflicting instrumented line counts", () => { + expect(() => + mergeCoverageSources( + ["api", "core"], + [ + ["api", parse([["src/a.ts", [1, 3]]], "api")], + ["core", parse([["src/a.ts", [1, 2]]], "core")], + ], + ), + ).toThrow("core coverage has conflicting instrumented lines for src/a.ts") + }) +}) diff --git a/src/scripts/coverage-contract.mjs b/src/scripts/coverage-contract.mjs new file mode 100644 index 0000000000..a0b50a71b6 --- /dev/null +++ b/src/scripts/coverage-contract.mjs @@ -0,0 +1,64 @@ +export const parseCoverageSourceLines = (lcov, lane) => { + const sources = new Map() + let source + let instrumentedLines = new Set() + let hasSummary = false + + for (const line of lcov.split(/\r?\n/)) { + if (line.startsWith("SF:")) { + if (source) throw new Error(`${lane} coverage contains an unfinished source record: ${source}`) + source = line.slice(3) + if (!source) throw new Error(`${lane} coverage contains an empty source path`) + instrumentedLines = new Set() + hasSummary = false + } else if (line.startsWith("DA:")) { + if (!source) throw new Error(`${lane} coverage contains DA outside a source record`) + if (hasSummary) throw new Error(`${lane} coverage contains DA after LF for ${source}`) + const lineNumber = Number(line.slice(3).split(",", 1)[0]) + if (!Number.isSafeInteger(lineNumber) || lineNumber < 1) + throw new Error(`${lane} coverage contains invalid DA for ${source}`) + if (instrumentedLines.has(lineNumber)) + throw new Error(`${lane} coverage contains duplicate DA for ${source}:${lineNumber}`) + instrumentedLines.add(lineNumber) + } else if (line.startsWith("LF:")) { + if (!source) throw new Error(`${lane} coverage contains LF outside a source record`) + if (sources.has(source)) throw new Error(`${lane} coverage contains duplicate source record: ${source}`) + + const linesFound = Number(line.slice(3)) + if (!Number.isSafeInteger(linesFound) || linesFound < 0 || linesFound !== instrumentedLines.size) + throw new Error(`${lane} coverage contains invalid LF for ${source}`) + sources.set(source, instrumentedLines) + hasSummary = true + } else if (line === "end_of_record") { + if (!source || !hasSummary) throw new Error(`${lane} coverage contains an invalid record terminator`) + source = undefined + } + } + if (source) throw new Error(`${lane} coverage contains an unfinished source record: ${source}`) + + return sources +} + +export const mergeCoverageSources = (expectedLanes, coverageByLane) => { + const lanes = new Set() + const combinedSources = new Map() + for (const [lane, sources] of coverageByLane) { + if (lanes.has(lane)) throw new Error(`Coverage lane is duplicated: ${lane}`) + lanes.add(lane) + if (sources.size === 0) throw new Error(`Coverage lane has no source records: ${lane}`) + for (const [source, instrumentedLines] of sources) { + const existingLines = combinedSources.get(source) + if ( + existingLines && + (existingLines.size !== instrumentedLines.size || + [...existingLines].some((line) => !instrumentedLines.has(line))) + ) + throw new Error(`${lane} coverage has conflicting instrumented lines for ${source}`) + combinedSources.set(source, instrumentedLines) + } + } + + for (const lane of expectedLanes) if (!lanes.has(lane)) throw new Error(`Coverage lane is missing: ${lane}`) + for (const lane of lanes) if (!expectedLanes.includes(lane)) throw new Error(`Unexpected coverage lane: ${lane}`) + return combinedSources +} diff --git a/src/scripts/verify-coverage-contract.mjs b/src/scripts/verify-coverage-contract.mjs index 14b19c47ea..c8698575a5 100644 --- a/src/scripts/verify-coverage-contract.mjs +++ b/src/scripts/verify-coverage-contract.mjs @@ -5,6 +5,8 @@ import { relative, resolve } from "node:path" import process from "node:process" import { promisify } from "node:util" +import { mergeCoverageSources, parseCoverageSourceLines } from "./coverage-contract.mjs" + const pnpm = process.platform === "win32" ? process.env.npm_execpath : "pnpm" if (!pnpm) throw new Error("pnpm executable path is unavailable") const command = process.platform === "win32" ? process.execPath : pnpm @@ -151,16 +153,9 @@ try { rmSync(collectionDirectory, { recursive: true, force: true }) } -const coverageSources = new Map() -for (const lane of [...ownershipLanes, "tree-sitter"]) { - let source - for (const line of readFileSync(resolve(root, "coverage", lane, "lcov.info"), "utf8").split(/\r?\n/)) { - if (line.startsWith("SF:")) source = line.slice(3) - if (line.startsWith("LF:")) coverageSources.set(source, Number(line.slice(3))) - } -} -const instrumentedLines = [...coverageSources.values()].reduce((sum, lines) => sum + lines, 0) -if (coverageSources.size !== 469 || instrumentedLines !== 30_229) - throw new Error( - `Coverage source population changed: ${coverageSources.size} records and ${instrumentedLines} lines; verify equivalence and update the baseline deliberately`, - ) +const coverageLanes = [...ownershipLanes, "tree-sitter"] +const coverageByLane = coverageLanes.map((lane) => [ + lane, + parseCoverageSourceLines(readFileSync(resolve(root, "coverage", lane, "lcov.info"), "utf8"), lane), +]) +mergeCoverageSources(coverageLanes, coverageByLane) From 619c50a58a73fe34f13f80b91393ae03d040afd0 Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 15 Sep 2026 03:48:13 +0000 Subject: [PATCH 2/2] test(ci): harden LCOV contract parsing --- .../__tests__/coverage-contract.spec.mjs | 45 +++++++++++++++++++ src/scripts/coverage-contract.mjs | 12 +++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/scripts/__tests__/coverage-contract.spec.mjs b/src/scripts/__tests__/coverage-contract.spec.mjs index 3ff2791fd9..589f6b7d71 100644 --- a/src/scripts/__tests__/coverage-contract.spec.mjs +++ b/src/scripts/__tests__/coverage-contract.spec.mjs @@ -79,6 +79,51 @@ describe("coverage source equivalence", () => { ) }) + it.each([ + ["empty source paths", "SF:\nLF:0\nend_of_record", "empty source path"], + ["DA outside a record", "DA:1,1", "DA outside a source record"], + ["LF outside a record", "LF:0", "LF outside a source record"], + ["DA after LF", "SF:src/a.ts\nDA:1,1\nLF:1\nDA:2,1\nend_of_record", "DA after LF"], + ["missing DA counts", "SF:src/a.ts\nDA:1\nLF:1\nend_of_record", "invalid DA"], + ["nonnumeric DA counts", "SF:src/a.ts\nDA:1,nope\nLF:1\nend_of_record", "invalid DA"], + [ + "unsafe DA line numbers", + `SF:src/a.ts\nDA:${Number.MAX_SAFE_INTEGER + 1},0\nLF:1\nend_of_record`, + "invalid DA", + ], + ["unsafe DA counts", `SF:src/a.ts\nDA:1,${Number.MAX_SAFE_INTEGER + 1}\nLF:1\nend_of_record`, "invalid DA"], + ["duplicate DA lines", "SF:src/a.ts\nDA:1,0\nDA:1,1\nLF:2\nend_of_record", "duplicate DA"], + ["empty LF values", "SF:src/a.ts\nLF:\nend_of_record", "invalid LF"], + ["nonnumeric LF values", "SF:src/a.ts\nLF:nope\nend_of_record", "invalid LF"], + ["invalid terminators", "end_of_record", "invalid record terminator"], + ])("rejects %s", (_name, lcov, error) => { + expect(() => parseCoverageSourceLines(lcov, "api")).toThrow(error) + }) + + it("accepts DA and LF numeric boundaries", () => { + expect(() => + parseCoverageSourceLines( + `SF:src/a.ts\nDA:1,0\nDA:${Number.MAX_SAFE_INTEGER},0,checksum\nLF:2\nend_of_record`, + "api", + ), + ).not.toThrow() + }) + + it("rejects empty and unexpected lane coverage", () => { + expect(() => mergeCoverageSources(["api"], [["api", new Map()]])).toThrow( + "Coverage lane has no instrumented lines: api", + ) + expect(() => + mergeCoverageSources( + ["api"], + [ + ["api", parse([["src/a.ts", [1]]], "api")], + ["core", parse([["src/a.ts", [1]]], "core")], + ], + ), + ).toThrow("Unexpected coverage lane: core") + }) + it("rejects conflicting instrumented line counts", () => { expect(() => mergeCoverageSources( diff --git a/src/scripts/coverage-contract.mjs b/src/scripts/coverage-contract.mjs index a0b50a71b6..2c56d4259d 100644 --- a/src/scripts/coverage-contract.mjs +++ b/src/scripts/coverage-contract.mjs @@ -14,9 +14,13 @@ export const parseCoverageSourceLines = (lcov, lane) => { } else if (line.startsWith("DA:")) { if (!source) throw new Error(`${lane} coverage contains DA outside a source record`) if (hasSummary) throw new Error(`${lane} coverage contains DA after LF for ${source}`) - const lineNumber = Number(line.slice(3).split(",", 1)[0]) + const match = /^DA:(\d+),(\d+)(?:,[^,\r\n]+)?$/.exec(line) + const lineNumber = match ? Number(match[1]) : Number.NaN + const executionCount = match ? Number(match[2]) : Number.NaN if (!Number.isSafeInteger(lineNumber) || lineNumber < 1) throw new Error(`${lane} coverage contains invalid DA for ${source}`) + if (!Number.isSafeInteger(executionCount)) + throw new Error(`${lane} coverage contains invalid DA for ${source}`) if (instrumentedLines.has(lineNumber)) throw new Error(`${lane} coverage contains duplicate DA for ${source}:${lineNumber}`) instrumentedLines.add(lineNumber) @@ -24,7 +28,8 @@ export const parseCoverageSourceLines = (lcov, lane) => { if (!source) throw new Error(`${lane} coverage contains LF outside a source record`) if (sources.has(source)) throw new Error(`${lane} coverage contains duplicate source record: ${source}`) - const linesFound = Number(line.slice(3)) + const match = /^LF:(\d+)$/.exec(line) + const linesFound = match ? Number(match[1]) : Number.NaN if (!Number.isSafeInteger(linesFound) || linesFound < 0 || linesFound !== instrumentedLines.size) throw new Error(`${lane} coverage contains invalid LF for ${source}`) sources.set(source, instrumentedLines) @@ -45,7 +50,8 @@ export const mergeCoverageSources = (expectedLanes, coverageByLane) => { for (const [lane, sources] of coverageByLane) { if (lanes.has(lane)) throw new Error(`Coverage lane is duplicated: ${lane}`) lanes.add(lane) - if (sources.size === 0) throw new Error(`Coverage lane has no source records: ${lane}`) + if (sources.size === 0 || [...sources.values()].every((lines) => lines.size === 0)) + throw new Error(`Coverage lane has no instrumented lines: ${lane}`) for (const [source, instrumentedLines] of sources) { const existingLines = combinedSources.get(source) if (