-
Notifications
You must be signed in to change notification settings - Fork 280
[Fix] Merge queue rejects legitimate source changes #1644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| 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.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"], | ||
| ["zero DA line numbers", "SF:src/a.ts\nDA:0,1\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"], | ||
| ["mismatched LF values", "SF:src/a.ts\nDA:1,1\nLF:2\nend_of_record", "invalid LF"], | ||
| ["records without LF", "SF:src/a.ts\nend_of_record", "invalid record terminator"], | ||
| ["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", () => { | ||
| const sources = parseCoverageSourceLines( | ||
| `SF:src/a.ts\nDA:1,0\nDA:${Number.MAX_SAFE_INTEGER},0,checksum\nLF:2\nend_of_record`, | ||
| "api", | ||
| ) | ||
|
|
||
| expect(sources).toEqual(new Map([["src/a.ts", new Set([1, Number.MAX_SAFE_INTEGER])]])) | ||
| }) | ||
|
|
||
| it("rejects empty and unexpected lane coverage", () => { | ||
| expect(() => mergeCoverageSources(["api"], [["api", new Map()]])).toThrow( | ||
| "Coverage lane has no instrumented lines: api", | ||
| ) | ||
|
zoomote[bot] marked this conversation as resolved.
|
||
| expect(() => mergeCoverageSources(["api"], [["api", new Map([["src/a.ts", new Set()]])]])).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( | ||
| ["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") | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| export const parseCoverageSourceLines = (lcov, lane) => { | ||
| const sources = new Map() | ||
| let source | ||
| let instrumentedLines = new Set() | ||
| let hasSummary = false | ||
|
Check warning on line 5 in src/scripts/coverage-contract.mjs
|
||
|
|
||
| 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 match = /^DA:(\d+),(\d+)(?:,[^,\r\n]+)?$/.exec(line) | ||
|
Check warning on line 17 in src/scripts/coverage-contract.mjs
|
||
| 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) | ||
| } 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 match = /^LF:(\d+)$/.exec(line) | ||
|
Check warning on line 31 in src/scripts/coverage-contract.mjs
|
||
| const linesFound = match ? Number(match[1]) : Number.NaN | ||
| if (!Number.isSafeInteger(linesFound) || linesFound < 0 || linesFound !== instrumentedLines.size) | ||
|
Check warning on line 33 in src/scripts/coverage-contract.mjs
|
||
| 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 || [...sources.values()].every((lines) => lines.size === 0)) | ||
|
Check warning on line 53 in src/scripts/coverage-contract.mjs
|
||
| throw new Error(`Coverage lane has no instrumented lines: ${lane}`) | ||
| for (const [source, instrumentedLines] of sources) { | ||
| const existingLines = combinedSources.get(source) | ||
| if ( | ||
| existingLines && | ||
| (existingLines.size !== instrumentedLines.size || | ||
|
Check warning on line 59 in src/scripts/coverage-contract.mjs
|
||
| [...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 | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.