Skip to content

Commit 5caffef

Browse files
committed
test_runner: exclude ignored lines from BRDA in lcov output
When using `/* node:coverage ignore next */` comments, the coverage system correctly excluded lines from DA (line coverage) entries in LCOV output but still reported branches as uncovered (BRDA count=0). This caused branch coverage to show incorrect percentages (e.g., 66% instead of 100%) when the uncovered branch path contained ignored lines, impacting CI/CD pipelines that enforce branch coverage thresholds. The fix treats branches as covered when they have `count === 0` but contain ignored lines (`ignoredLines > 0`). This matches the behavior of c8 and ensures ignored code doesn't penalize branch coverage. Fixes: #61586
1 parent 784ca7b commit 5caffef

File tree

4 files changed

+77
-3
lines changed

4 files changed

+77
-3
lines changed

lib/internal/test_runner/coverage.js

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,14 +193,23 @@ class TestCoverage {
193193
ObjectAssign(range, mapRangeToLines(range, lines));
194194

195195
if (isBlockCoverage) {
196+
// Skip branches where all lines are ignored (similar to line handling)
197+
if (range.ignoredLines === range.lines.length) {
198+
continue;
199+
}
200+
201+
// If the branch is uncovered but contains ignored lines, treat it as
202+
// covered. This matches the behavior of tools like c8 and ensures that
203+
// ignored code doesn't penalize branch coverage.
204+
const branchCount = (range.count === 0 && range.ignoredLines > 0) ? 1 : range.count;
205+
196206
ArrayPrototypePush(branchReports, {
197207
__proto__: null,
198208
line: range.lines[0]?.line,
199-
count: range.count,
209+
count: branchCount,
200210
});
201211

202-
if (range.count !== 0 ||
203-
range.ignoredLines === range.lines.length) {
212+
if (branchCount !== 0) {
204213
branchesCovered++;
205214
}
206215

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
// Source file for testing that branch coverage respects ignore comments
3+
4+
function getValue(condition) {
5+
if (condition) {
6+
return 'truthy';
7+
}
8+
/* node:coverage ignore next */
9+
return 'falsy';
10+
}
11+
12+
module.exports = { getValue };
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
'use strict';
2+
const { test } = require('node:test');
3+
const assert = require('node:assert');
4+
const { getValue } = require('./source.js');
5+
6+
// Only call with true, so the false branch is "uncovered" but ignored
7+
test('getValue returns truthy for true', () => {
8+
assert.strictEqual(getValue(true), 'truthy');
9+
});

test/parallel/test-runner-coverage.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,3 +566,47 @@ test('coverage with directory and file named "file"', skipIfNoInspector, () => {
566566
assert.strictEqual(result.status, 0);
567567
assert(result.stdout.toString().includes('start of coverage report'));
568568
});
569+
570+
// Regression test for https://github.com/nodejs/node/issues/61586
571+
test('coverage ignore comments exclude branches in LCOV output', skipIfNoInspector, () => {
572+
const fixture = fixtures.path('test-runner', 'coverage-ignore-branch', 'test.js');
573+
const args = [
574+
'--experimental-test-coverage',
575+
'--test-reporter', 'lcov',
576+
'--test-coverage-exclude=!test/fixtures/test-runner/coverage-ignore-branch/**',
577+
fixture,
578+
];
579+
const result = spawnSync(process.execPath, args);
580+
const lcov = result.stdout.toString();
581+
582+
assert.strictEqual(result.stderr.toString(), '');
583+
assert.strictEqual(result.status, 0);
584+
585+
// Extract the source.js section from LCOV output
586+
const sourceSection = lcov.split('end_of_record').find((s) => s.includes('source.js'));
587+
assert(sourceSection, 'LCOV should contain source.js coverage');
588+
589+
// Verify that all branches are reported as covered (BRH should equal BRF)
590+
// The ignored branch should not penalize coverage
591+
const brfMatch = sourceSection.match(/BRF:(\d+)/);
592+
const brhMatch = sourceSection.match(/BRH:(\d+)/);
593+
assert.match(sourceSection, /BRF:(\d+)/, 'LCOV should contain BRF');
594+
assert.match(sourceSection, /BRH:(\d+)/, 'LCOV should contain BRH');
595+
assert.strictEqual(
596+
brfMatch[1],
597+
brhMatch[1],
598+
`All branches should be covered when ignored code is not executed. BRF=${brfMatch[1]}, BRH=${brhMatch[1]}`,
599+
);
600+
601+
// Verify no BRDA entries show 0 (uncovered) for the ignored branch
602+
// The branch at the if statement should be covered, not penalized by the ignored return
603+
const brdaEntries = sourceSection.match(/BRDA:\d+,\d+,\d+,(\d+)/g) || [];
604+
for (const entry of brdaEntries) {
605+
const count = entry.match(/BRDA:\d+,\d+,\d+,(\d+)/)[1];
606+
assert.notStrictEqual(
607+
count,
608+
'0',
609+
`No branch should show 0 coverage when the uncovered path is ignored: ${entry}`,
610+
);
611+
}
612+
});

0 commit comments

Comments
 (0)