Skip to content

Commit ba8055c

Browse files
rochdevrochdev
committed
ci(all-green): merge lcov reports per-file instead of concatenating
Each matrix cell in a sibling workflow (Node.js version, plugin partition) writes its own complete lcov report, so a shared source file gets an `SF:` block from every cell. Concatenating those blocks, as group-coverage.mjs did, produced a report with duplicate `SF:` sections per file; Codecov keeps only the last block for a file rather than summing across duplicates, which silently discarded almost all branch/function coverage once per-cell uploads were merged into one per-workflow upload (PR #9197's branch dropped from 10802 to 65 branches as a result). mergeLcov now sums DA:/FNDA:/BRDA: hit counts per file across cells instead, the way `lcov --add-tracefile` does. Generated by Claude Code.
1 parent 7a3eabe commit ba8055c

2 files changed

Lines changed: 180 additions & 16 deletions

File tree

‎scripts/group-coverage.mjs‎

Lines changed: 127 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,16 @@ import { join } from 'node:path'
77
// merging and uploading anything — the goal is for each workflow's coverage to reach Datadog and
88
// Codecov shortly after that workflow finishes, in parallel with the rest still running.
99
//
10-
// lcov is a plain-text, per-source-file record format, so concatenating reports is a valid merge on
11-
// its own (`lcov`/`genhtml` do the same to combine reports) — no per-file hit-count summing is
12-
// needed, unlike istanbul's JSON, which is why only lcov is uploaded: this repo's
13-
// `patch-istanbul-lib-coverage.js` already folds branch/function hit data into lcov's `DA:` records,
14-
// and `.codecov.yml` only gates line-level `patch` coverage, so istanbul's JSON added merge cost
15-
// (summing hit counts across cells for shared files) without affecting the gate.
10+
// Only lcov is uploaded, not istanbul's JSON: this repo's `patch-istanbul-lib-coverage.js` already
11+
// folds branch/function hit data into lcov's `DA:` records, and `.codecov.yml` only gates line-level
12+
// `patch` coverage, so the JSON report added merge cost without affecting the gate. lcov itself still
13+
// needs a real per-file merge, not concatenation: every matrix cell in a workflow run (each Node.js
14+
// version, each plugin partition) writes its own complete report, so a shared source file gets an
15+
// `SF:` block from every cell. Concatenating those blocks produces a report with duplicate `SF:`
16+
// sections per file, which downstream lcov consumers (including Codecov) resolve by keeping only the
17+
// last block for that file rather than summing across blocks — silently discarding most of the
18+
// branch/function data every earlier cell had recorded. `mergeLcov` sums `DA:`/`FNDA:`/`BRDA:` hit
19+
// counts per file across cells instead, the way `lcov --add-tracefile` does.
1620
//
1721
// Per-integration/per-area flags were dropped: `.codecov.yml` only gates the separate
1822
// `master-coverage` flag (attached to every upload regardless of grouping), so a finer-grained flag
@@ -95,17 +99,129 @@ function planCoverageGroups (files) {
9599
}
96100

97101
/**
98-
* Concatenate every cell's lcov report into a single lcov file. lcov's format is a sequence of
99-
* independent per-source-file records, so concatenation alone is a valid merge.
102+
* @typedef {object} LcovFileRecord
103+
* @property {Map<string, number>} lines line number -> summed hit count
104+
* @property {Map<string, { line: string, count: number }>} functions function name -> declaration
105+
* line and summed hit count
106+
* @property {Map<string, { hits: number, reached: boolean }>} branches `line,block,branch` -> summed
107+
* hit count and whether any cell reported the enclosing block as reached (lcov uses `-` for a
108+
* branch whose block was never reached, distinct from a reached block whose branch was never taken)
109+
*/
110+
111+
/**
112+
* Split an lcov record line into its tag and payload, e.g. `DA:1,1` -> `['DA', '1,1']`.
113+
*
114+
* @param {string} line
115+
* @returns {[string, string]}
116+
*/
117+
function splitLcovLine (line) {
118+
const index = line.indexOf(':')
119+
return index === -1 ? [line, ''] : [line.slice(0, index), line.slice(index + 1)]
120+
}
121+
122+
/**
123+
* Fold one `SF:`-delimited record's `DA:`/`FN:`/`FNDA:`/`BRDA:` lines into the running per-file
124+
* merge state, summing hit counts for lines/functions/branches already seen from earlier cells.
125+
*
126+
* @param {string[]} recordLines
127+
* @param {Map<string, LcovFileRecord>} files source file path -> merge state
128+
* @param {string[]} order source file paths in first-seen order
129+
* @returns {void}
130+
*/
131+
function mergeLcovRecord (recordLines, files, order) {
132+
const sourceFileLine = recordLines.find(line => line.startsWith('SF:'))
133+
if (!sourceFileLine) return
134+
135+
const path = splitLcovLine(sourceFileLine)[1]
136+
if (!files.has(path)) {
137+
files.set(path, { lines: new Map(), functions: new Map(), branches: new Map() })
138+
order.push(path)
139+
}
140+
const record = files.get(path)
141+
142+
for (const line of recordLines) {
143+
const [tag, rest] = splitLcovLine(line)
144+
if (tag === 'DA') {
145+
const [lineNumber, count] = rest.split(',')
146+
record.lines.set(lineNumber, (record.lines.get(lineNumber) ?? 0) + Number(count))
147+
} else if (tag === 'FN') {
148+
const [lineNumber, name] = rest.split(',')
149+
if (!record.functions.has(name)) record.functions.set(name, { line: lineNumber, count: 0 })
150+
} else if (tag === 'FNDA') {
151+
const [count, name] = rest.split(',')
152+
const fn = record.functions.get(name) ?? { line: '0', count: 0 }
153+
fn.count += Number(count)
154+
record.functions.set(name, fn)
155+
} else if (tag === 'BRDA') {
156+
const [lineNumber, block, branch, count] = rest.split(',')
157+
const key = `${lineNumber},${block},${branch}`
158+
const branchRecord = record.branches.get(key) ?? { hits: 0, reached: false }
159+
if (count !== '-') {
160+
branchRecord.hits += Number(count)
161+
branchRecord.reached = true
162+
}
163+
record.branches.set(key, branchRecord)
164+
}
165+
}
166+
}
167+
168+
/**
169+
* Serialize one file's merged coverage state back into lcov record lines, recomputing the
170+
* `LF`/`LH`/`FNF`/`FNH`/`BRF`/`BRH` summary lines from the merged data.
171+
*
172+
* @param {string} path
173+
* @param {LcovFileRecord} record
174+
* @returns {string}
175+
*/
176+
function serializeLcovRecord (path, record) {
177+
const lines = [`SF:${path}`]
178+
179+
for (const [name, fn] of record.functions) lines.push(`FN:${fn.line},${name}`)
180+
for (const [name, fn] of record.functions) lines.push(`FNDA:${fn.count},${name}`)
181+
if (record.functions.size > 0) {
182+
const hit = [...record.functions.values()].filter(fn => fn.count > 0).length
183+
lines.push(`FNF:${record.functions.size}`, `FNH:${hit}`)
184+
}
185+
186+
for (const [key, branch] of record.branches) {
187+
lines.push(`BRDA:${key},${branch.reached ? branch.hits : '-'}`)
188+
}
189+
if (record.branches.size > 0) {
190+
const hit = [...record.branches.values()].filter(branch => branch.hits > 0).length
191+
lines.push(`BRF:${record.branches.size}`, `BRH:${hit}`)
192+
}
193+
194+
for (const [lineNumber, count] of record.lines) lines.push(`DA:${lineNumber},${count}`)
195+
if (record.lines.size > 0) {
196+
const hit = [...record.lines.values()].filter(count => count > 0).length
197+
lines.push(`LF:${record.lines.size}`, `LH:${hit}`)
198+
}
199+
200+
lines.push('end_of_record')
201+
return `${lines.join('\n')}\n`
202+
}
203+
204+
/**
205+
* Merge every cell's lcov report into a single lcov file, summing hit counts per source file
206+
* instead of concatenating records — see the module comment for why concatenation silently drops
207+
* coverage when the same file appears in more than one cell's report.
100208
*
101209
* @param {string[]} reportPaths
102210
* @returns {string}
103211
*/
104212
function mergeLcov (reportPaths) {
105-
return reportPaths.map(reportPath => {
213+
const files = new Map()
214+
const order = []
215+
216+
for (const reportPath of reportPaths) {
106217
const contents = readFileSync(reportPath, 'utf8')
107-
return contents.endsWith('\n') ? contents : `${contents}\n`
108-
}).join('')
218+
for (const record of contents.split('end_of_record')) {
219+
const recordLines = record.split('\n').map(line => line.trim()).filter(Boolean)
220+
if (recordLines.length > 0) mergeLcovRecord(recordLines, files, order)
221+
}
222+
}
223+
224+
return order.map(path => serializeLcovRecord(path, files.get(path))).join('')
109225
}
110226

111227
/**

‎scripts/group-coverage.spec.mjs‎

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,62 @@ describe('group-coverage', () => {
7878
rmSync(dir, { force: true, recursive: true })
7979
})
8080

81-
it('concatenates every report, adding a trailing newline when one is missing', () => {
81+
it('keeps unrelated files as separate records, in first-seen order', () => {
8282
const a = join(dir, 'a.info')
8383
const b = join(dir, 'b.info')
84-
writeFileSync(a, 'SF:a.js\nDA:1,1\nend_of_record\n')
85-
writeFileSync(b, 'SF:b.js\nDA:1,1\nend_of_record') // no trailing newline
84+
writeFileSync(a, 'SF:a.js\nDA:1,1\nLF:1\nLH:1\nend_of_record\n')
85+
writeFileSync(b, 'SF:b.js\nDA:1,1\nLF:1\nLH:1\nend_of_record') // no trailing newline
8686
assert.equal(
8787
mergeLcov([a, b]),
88-
'SF:a.js\nDA:1,1\nend_of_record\nSF:b.js\nDA:1,1\nend_of_record\n'
88+
'SF:a.js\nDA:1,1\nLF:1\nLH:1\nend_of_record\n' +
89+
'SF:b.js\nDA:1,1\nLF:1\nLH:1\nend_of_record\n'
90+
)
91+
})
92+
93+
it('sums DA hit counts for the same file and line across reports', () => {
94+
const a = join(dir, 'a.info')
95+
const b = join(dir, 'b.info')
96+
writeFileSync(a, 'SF:shared.js\nDA:1,1\nDA:2,0\nLF:2\nLH:1\nend_of_record\n')
97+
writeFileSync(b, 'SF:shared.js\nDA:1,2\nDA:2,3\nLF:2\nLH:2\nend_of_record\n')
98+
assert.equal(
99+
mergeLcov([a, b]),
100+
'SF:shared.js\nDA:1,3\nDA:2,3\nLF:2\nLH:2\nend_of_record\n'
101+
)
102+
})
103+
104+
it('sums FNDA hit counts for the same function across reports', () => {
105+
const a = join(dir, 'a.info')
106+
const b = join(dir, 'b.info')
107+
writeFileSync(a, 'SF:shared.js\nFN:1,foo\nFNDA:1,foo\nFNF:1\nFNH:1\nend_of_record\n')
108+
writeFileSync(b, 'SF:shared.js\nFN:1,foo\nFNDA:0,foo\nFNF:1\nFNH:0\nend_of_record\n')
109+
assert.equal(
110+
mergeLcov([a, b]),
111+
'SF:shared.js\nFN:1,foo\nFNDA:1,foo\nFNF:1\nFNH:1\nend_of_record\n'
112+
)
113+
})
114+
115+
it('sums BRDA hit counts for the same branch, treating "-" as an unreached block', () => {
116+
const a = join(dir, 'a.info')
117+
const b = join(dir, 'b.info')
118+
// First cell never reaches the block (`-`); second cell reaches it but doesn't take branch 1.
119+
writeFileSync(a, 'SF:shared.js\nBRDA:1,0,0,-\nBRDA:1,0,1,-\nBRF:2\nBRH:0\nend_of_record\n')
120+
writeFileSync(b, 'SF:shared.js\nBRDA:1,0,0,2\nBRDA:1,0,1,0\nBRF:2\nBRH:1\nend_of_record\n')
121+
assert.equal(
122+
mergeLcov([a, b]),
123+
'SF:shared.js\nBRDA:1,0,0,2\nBRDA:1,0,1,0\nBRF:2\nBRH:1\nend_of_record\n'
124+
)
125+
})
126+
127+
it('merges duplicate SF blocks for the same file into one record instead of two', () => {
128+
const a = join(dir, 'a.info')
129+
writeFileSync(
130+
a,
131+
'SF:shared.js\nDA:1,1\nLF:1\nLH:1\nend_of_record\n' +
132+
'SF:shared.js\nDA:1,4\nLF:1\nLH:1\nend_of_record\n'
133+
)
134+
assert.equal(
135+
mergeLcov([a]),
136+
'SF:shared.js\nDA:1,5\nLF:1\nLH:1\nend_of_record\n'
89137
)
90138
})
91139
})
@@ -113,7 +161,7 @@ describe('group-coverage', () => {
113161
assert.equal(outputDir, join(output, '42', 'lcov'))
114162
assert.equal(
115163
readFileSync(join(outputDir, 'lcov.info'), 'utf8'),
116-
'SF:a.js\nDA:1,1\nend_of_record\n'
164+
'SF:a.js\nDA:1,1\nLF:1\nLH:1\nend_of_record\n'
117165
)
118166
})
119167

0 commit comments

Comments
 (0)