Skip to content

Commit a7e62f9

Browse files
authored
fix: Branch coverage calculation (#11)
Summary Branch coverage data was read incorrectly. Rewriting the internal calculation and using the chance to clean up some of the parser/generator code Changes - Fixed the branch coverage calculation
1 parent 5e28190 commit a7e62f9

51 files changed

Lines changed: 762 additions & 736 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/core/process-coverage.ts

Lines changed: 14 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,10 @@ import * as fs from 'node:fs/promises'
22
import * as path from 'node:path'
33
import {
44
CoberturaCoverageParser,
5-
type CoverageMetrics,
65
type CoverageReport,
76
type FileCoverage,
8-
type LineCoverage,
9-
type LineCoverageState,
107
type PackageCoverage,
118
} from '../coverage/index.js'
12-
import type { PercentageCoverageMetrics } from '../coverage/model.js'
139
import { type ChangedLinesMap, filterByChangedLines, filterByGlob, getChangedLinesFromGit } from '../filter/index.js'
1410
import { generateMarkdown } from '../markdown/index.js'
1511

@@ -119,7 +115,7 @@ export async function processCoverage(
119115
}
120116

121117
const totalFiles = reports.reduce((s1, r) => s1 + r.packages.reduce((s2, p) => s2 + p.files.length, 0), 0)
122-
logger.info(`Found coverage data in ${reports.length} for ${totalFiles} files total`)
118+
logger.info(`Found coverage data in ${reports.length} reports for ${totalFiles} files total`)
123119
// Merge all reports into one (including sources)
124120
const mergedPackages = await mergeReportAndResolveSources(reports, inputs.sourceDir, logger)
125121

@@ -154,8 +150,8 @@ export async function processCoverage(
154150
}
155151
}
156152

157-
const overallMetrics = calculateOverallMetrics(mergedPackages)
158-
const prMetrics = calculateOverallMetrics(filteredPackages)
153+
const overallMetrics = CoberturaCoverageParser.calculatePackageCoverage(mergedPackages)
154+
const prMetrics = CoberturaCoverageParser.calculatePackageCoverage(filteredPackages)
159155
logger.info(
160156
`Calculated overall metrics (LineCoverage: ${overallMetrics.lineCoverage}, BranchCoverage: ${overallMetrics.branchCoverage})`,
161157
)
@@ -189,46 +185,6 @@ async function firstExistingDirectory(paths: readonly string[]): Promise<string
189185
return undefined
190186
}
191187

192-
/** Priority order for restrictive merging: lower = worse (takes precedence) */
193-
const statePriority: Record<LineCoverageState, number> = {
194-
'not-covered': 0,
195-
partial: 1,
196-
covered: 2,
197-
}
198-
199-
/**
200-
* Merge two FileCoverage objects using restrictive line state merging.
201-
* For lines present in both: take the "worst" state (not-covered > partial > covered).
202-
* For lines present in only one: use that state.
203-
*/
204-
function mergeFileCoverage(existing: FileCoverage, incoming: FileCoverage): FileCoverage {
205-
const worse = (a: LineCoverage, b: LineCoverage) => (statePriority[a.state] <= statePriority[b.state] ? a : b)
206-
207-
const linesByNumber = new Map<number, LineCoverage>()
208-
209-
// seed with existing
210-
for (const l of existing.lines) linesByNumber.set(l.lineNumber, l)
211-
212-
// merge incoming
213-
for (const l of incoming.lines) {
214-
const prev = linesByNumber.get(l.lineNumber)
215-
linesByNumber.set(l.lineNumber, prev ? worse(prev, l) : l)
216-
}
217-
218-
const lines = [...linesByNumber.values()].sort((a, b) => a.lineNumber - b.lineNumber)
219-
const lineMetrics = { covered: lines.reduce((n, l) => n + (l.state === 'covered' ? 1 : 0), 0), total: lines.length }
220-
221-
const sumMetrics = (a?: CoverageMetrics, b?: CoverageMetrics): CoverageMetrics | undefined =>
222-
a && b ? ({ covered: a.covered + b.covered, total: a.total + b.total } as CoverageMetrics) : (a ?? b)
223-
224-
return {
225-
...existing,
226-
lines,
227-
lineMetrics,
228-
branchMetrics: sumMetrics(existing.branchMetrics, incoming.branchMetrics),
229-
}
230-
}
231-
232188
/**
233189
* Merge multiple coverage reports into one.
234190
* Also merges sources from all reports.
@@ -261,9 +217,9 @@ async function mergeReportAndResolveSources(
261217
// Add files with resolved paths (merge duplicates)
262218
for (const file of pkg.files) {
263219
const resolvedPath = path.resolve(source, file.filename)
264-
if (fileMap.has(resolvedPath)) {
265-
const existing = fileMap.get(resolvedPath)!
266-
const merged = mergeFileCoverage(existing, { resolvedPath, ...file })
220+
const existing = fileMap.get(resolvedPath)
221+
if (existing) {
222+
const merged = CoberturaCoverageParser.merge(existing, file.lines)
267223
fileMap.set(resolvedPath, merged)
268224
logger.debug?.(`Merged duplicate file: ${file.filename}`)
269225
} else {
@@ -274,43 +230,20 @@ async function mergeReportAndResolveSources(
274230
}
275231

276232
// Convert file maps back to arrays for the final report
277-
const packages: PackageCoverage[] = Array.from(packageMap.values()).map(({ name, fileMap }) => ({
278-
name,
279-
files: Array.from(fileMap.values()),
280-
}))
233+
const packages: PackageCoverage[] = Array.from(packageMap.values()).map(({ name, fileMap }) => {
234+
const files = Array.from(fileMap.values())
235+
return {
236+
name,
237+
files,
238+
coverage: CoberturaCoverageParser.calculateFileCoverage(files),
239+
}
240+
})
281241

282242
const totalFiles = packages.reduce((sum, pkg) => sum + pkg.files.length, 0)
283243
logger.info(`Merged coverage data for ${totalFiles} distinct files`)
284244
return packages
285245
}
286246

287-
/**
288-
* Calculate overall coverage metrics from a merged packages.
289-
*/
290-
function calculateOverallMetrics(packages: PackageCoverage[]): PercentageCoverageMetrics {
291-
let lineCovered = 0
292-
let lineTotal = 0
293-
let branchCovered = 0
294-
let branchTotal = 0
295-
296-
for (const pkg of packages) {
297-
for (const file of pkg.files) {
298-
lineCovered += file.lineMetrics.covered
299-
lineTotal += file.lineMetrics.total
300-
301-
if (file.branchMetrics) {
302-
branchCovered += file.branchMetrics.covered
303-
branchTotal += file.branchMetrics.total
304-
}
305-
}
306-
}
307-
308-
return {
309-
lineCoverage: lineTotal > 0 ? (lineCovered / lineTotal) * 100 : 0,
310-
branchCoverage: branchTotal > 0 ? (branchCovered / branchTotal) * 100 : 0,
311-
}
312-
}
313-
314247
/**
315248
* Read file contents from disk for all files in the coverage packages.
316249
* Returns a map of resolved path -> lines array.

src/coverage/index.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
export type {
2-
CoverageMetrics,
32
CoverageReport,
43
FileCoverage,
54
LineCoverage,
6-
LineCoverageState,
75
PackageCoverage,
86
} from './model.js'
97

src/coverage/model.ts

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,52 @@
1-
/** Coverage state for a single line */
2-
export type LineCoverageState =
3-
| 'covered' // Line is fully covered (green)
4-
| 'partial' // Line executed but missing branch coverage (yellow)
5-
| 'not-covered' // Line not executed at all (red)
6-
71
export type LineCoverage = {
8-
lineNumber: number
9-
state: LineCoverageState
10-
}
11-
12-
export type CoverageMetrics = {
13-
covered: number
14-
total: number
2+
/** The number of the line with coverage information*/
3+
readonly lineNumber: number
4+
/** True, if the line was hit at least once */
5+
readonly covered: boolean
6+
/** The number of branches covered */
7+
readonly branchesCovered: number
8+
/** The total number of branches available in this line */
9+
readonly totalBranches: number
1510
}
1611

1712
export type PercentageCoverageMetrics = {
18-
lineCoverage: number
19-
branchCoverage?: number | undefined
13+
/** The number of lines covered */
14+
readonly linesCovered: number
15+
/** The total number of lines */
16+
readonly totalLines: number
17+
/** The number of branches covered */
18+
readonly branchesCovered: number
19+
/** The total number of branches */
20+
readonly totalBranches: number
21+
/** The line coverage */
22+
readonly lineCoverage: number
23+
/** The branch coverage */
24+
readonly branchCoverage: number | undefined
2025
}
2126

2227
export type FileCoverage = {
2328
/** Display path (relative to source root, for markdown output) */
24-
filename: string
29+
readonly filename: string
2530
/** Absolute path for reading file contents (set after path resolution) */
26-
resolvedPath?: string
27-
lines: LineCoverage[]
28-
lineMetrics: CoverageMetrics
29-
branchMetrics?: CoverageMetrics | undefined
31+
resolvedPath?: string | undefined
32+
/** Coverage information per line */
33+
readonly lines: LineCoverage[]
34+
/** The coverage information */
35+
readonly coverage: PercentageCoverageMetrics
3036
}
3137

3238
export type PackageCoverage = {
33-
name: string
34-
files: FileCoverage[]
39+
readonly name: string
40+
readonly files: FileCoverage[],
41+
/** The coverage information */
42+
readonly coverage: PercentageCoverageMetrics
3543
}
3644

3745
export type CoverageReport = {
3846
/** Hint name for logging (e.g., the file path this report was parsed from) */
39-
hintName?: string
47+
readonly hintName?: string | undefined
4048
/** The packages that are listed in this coverage report */
41-
packages: PackageCoverage[]
49+
readonly packages: PackageCoverage[]
4250
/** Source paths from Cobertura XML <sources> element */
43-
sources?: string[]
51+
readonly sources?: string[] | undefined
4452
}

0 commit comments

Comments
 (0)