Skip to content

Commit 1c9bd19

Browse files
PradSharma554PradSharma554
committed
fix/coverage-ignore-brda-61586
1 parent ae228c1 commit 1c9bd19

File tree

4 files changed

+127
-40
lines changed

4 files changed

+127
-40
lines changed

‎lib/internal/test_runner/coverage.js‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ class TestCoverage {
114114

115115
if (match !== null) {
116116
ignoreCount = NumberParseInt(match.groups?.count ?? 1, 10);
117+
coverageLine.ignore = true;
117118
}
118119
}
119120

@@ -193,14 +194,33 @@ class TestCoverage {
193194
ObjectAssign(range, mapRangeToLines(range, lines));
194195

195196
if (isBlockCoverage) {
197+
// Skip branches where all lines are ignored from coverage.
198+
if (range.ignoredLines === range.lines.length) {
199+
continue;
200+
}
201+
202+
// For uncovered branches, skip if all non-ignored lines are
203+
// already covered by other ranges. This handles the case where
204+
if (range.count === 0) {
205+
let hasUncoveredNonIgnoredLine = false;
206+
for (let l = 0; l < range.lines.length; ++l) {
207+
if (!range.lines[l].ignore && range.lines[l].count === 0) {
208+
hasUncoveredNonIgnoredLine = true;
209+
break;
210+
}
211+
}
212+
if (!hasUncoveredNonIgnoredLine) {
213+
continue;
214+
}
215+
}
216+
196217
ArrayPrototypePush(branchReports, {
197218
__proto__: null,
198219
line: range.lines[0]?.line,
199220
count: range.count,
200221
});
201222

202-
if (range.count !== 0 ||
203-
range.ignoredLines === range.lines.length) {
223+
if (range.count !== 0) {
204224
branchesCovered++;
205225
}
206226

‎test/fixtures/test-runner/coverage-ignore-branch/source.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
'use strict';
2+
3+
function getValue(condition) {
4+
if (condition) {
5+
return 'truthy';
6+
}
7+
/* node:coverage ignore next */
8+
return 'falsy';
9+
}
10+
11+
module.exports = { getValue };

‎test/fixtures/test-runner/coverage-ignore-branch/test.js‎

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: 85 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,19 @@ function getTapCoverageFixtureReport() {
2525

2626
const report = [
2727
'# start of coverage report',
28-
'# --------------------------------------------------------------------------------------------',
28+
'# -----------------------------------------------------------------------------------------',
2929
'# file | line % | branch % | funcs % | uncovered lines',
30-
'# --------------------------------------------------------------------------------------------',
30+
'# -----------------------------------------------------------------------------------------',
3131
'# test | | | | ',
3232
'# fixtures | | | | ',
3333
'# test-runner | | | | ',
34-
'# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
34+
'# coverage.js | 79.78 | 41.67 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72',
3535
'# invalid-tap.js | 100.00 | 100.00 | 100.00 | ',
3636
'# v8-coverage | | | | ',
3737
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
38-
'# --------------------------------------------------------------------------------------------',
39-
'# all files | 78.35 | 43.75 | 60.00 | ',
40-
'# --------------------------------------------------------------------------------------------',
38+
'# -----------------------------------------------------------------------------------------',
39+
'# all files | 79.38 | 46.67 | 60.00 | ',
40+
'# -----------------------------------------------------------------------------------------',
4141
'# end of coverage report',
4242
].join('\n');
4343

@@ -53,19 +53,19 @@ function getSpecCoverageFixtureReport() {
5353

5454
const report = [
5555
'\u2139 start of coverage report',
56-
'\u2139 --------------------------------------------------------------------------------------------',
56+
'\u2139 -----------------------------------------------------------------------------------------',
5757
'\u2139 file | line % | branch % | funcs % | uncovered lines',
58-
'\u2139 --------------------------------------------------------------------------------------------',
58+
'\u2139 -----------------------------------------------------------------------------------------',
5959
'\u2139 test | | | | ',
6060
'\u2139 fixtures | | | | ',
6161
'\u2139 test-runner | | | | ',
62-
'\u2139 coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
62+
'\u2139 coverage.js | 79.78 | 41.67 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72',
6363
'\u2139 invalid-tap.js | 100.00 | 100.00 | 100.00 | ',
6464
'\u2139 v8-coverage | | | | ',
6565
'\u2139 throw.js | 71.43 | 50.00 | 100.00 | 5-6',
66-
'\u2139 --------------------------------------------------------------------------------------------',
67-
'\u2139 all files | 78.35 | 43.75 | 60.00 | ',
68-
'\u2139 --------------------------------------------------------------------------------------------',
66+
'\u2139 -----------------------------------------------------------------------------------------',
67+
'\u2139 all files | 79.38 | 46.67 | 60.00 | ',
68+
'\u2139 -----------------------------------------------------------------------------------------',
6969
'\u2139 end of coverage report',
7070
].join('\n');
7171

@@ -198,12 +198,12 @@ test('coverage is combined for multiple processes', skipIfNoInspector, () => {
198198
'# -------------------------------------------------------------------',
199199
'# file | line % | branch % | funcs % | uncovered lines',
200200
'# -------------------------------------------------------------------',
201-
'# common.js | 89.86 | 62.50 | 100.00 | 8 13-14 18 34-35 53',
201+
'# common.js | 89.86 | 66.67 | 100.00 | 8 13-14 18 34-35 53',
202202
'# first.test.js | 83.33 | 100.00 | 50.00 | 5-6',
203203
'# second.test.js | 100.00 | 100.00 | 100.00 | ',
204204
'# third.test.js | 100.00 | 100.00 | 100.00 | ',
205205
'# -------------------------------------------------------------------',
206-
'# all files | 92.11 | 72.73 | 88.89 | ',
206+
'# all files | 92.11 | 76.19 | 88.89 | ',
207207
'# -------------------------------------------------------------------',
208208
'# end of coverage report',
209209
].join('\n');
@@ -415,18 +415,18 @@ test('coverage with excluded files', skipIfNoInspector, () => {
415415
const result = spawnSync(process.execPath, args);
416416
const report = [
417417
'# start of coverage report',
418-
'# -----------------------------------------------------------------------------------------',
418+
'# --------------------------------------------------------------------------------------',
419419
'# file | line % | branch % | funcs % | uncovered lines',
420-
'# -----------------------------------------------------------------------------------------',
420+
'# --------------------------------------------------------------------------------------',
421421
'# test | | | | ',
422422
'# fixtures | | | | ',
423423
'# test-runner | | | | ',
424-
'# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
424+
'# coverage.js | 79.78 | 41.67 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72',
425425
'# v8-coverage | | | | ',
426426
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
427-
'# -----------------------------------------------------------------------------------------',
428-
'# all files | 78.13 | 40.00 | 60.00 | ',
429-
'# -----------------------------------------------------------------------------------------',
427+
'# --------------------------------------------------------------------------------------',
428+
'# all files | 79.17 | 42.86 | 60.00 | ',
429+
'# --------------------------------------------------------------------------------------',
430430
'# end of coverage report',
431431
].join('\n');
432432

@@ -452,18 +452,18 @@ test('coverage with included files', skipIfNoInspector, () => {
452452
const result = spawnSync(process.execPath, args);
453453
const report = [
454454
'# start of coverage report',
455-
'# -----------------------------------------------------------------------------------------',
455+
'# --------------------------------------------------------------------------------------',
456456
'# file | line % | branch % | funcs % | uncovered lines',
457-
'# -----------------------------------------------------------------------------------------',
457+
'# --------------------------------------------------------------------------------------',
458458
'# test | | | | ',
459459
'# fixtures | | | | ',
460460
'# test-runner | | | | ',
461-
'# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
461+
'# coverage.js | 79.78 | 41.67 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72',
462462
'# v8-coverage | | | | ',
463463
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
464-
'# -----------------------------------------------------------------------------------------',
465-
'# all files | 78.13 | 40.00 | 60.00 | ',
466-
'# -----------------------------------------------------------------------------------------',
464+
'# --------------------------------------------------------------------------------------',
465+
'# all files | 79.17 | 42.86 | 60.00 | ',
466+
'# --------------------------------------------------------------------------------------',
467467
'# end of coverage report',
468468
].join('\n');
469469

@@ -488,16 +488,16 @@ test('coverage with included and excluded files', skipIfNoInspector, () => {
488488
const result = spawnSync(process.execPath, args);
489489
const report = [
490490
'# start of coverage report',
491-
'# -----------------------------------------------------------------------------------------',
491+
'# --------------------------------------------------------------------------------------',
492492
'# file | line % | branch % | funcs % | uncovered lines',
493-
'# -----------------------------------------------------------------------------------------',
493+
'# --------------------------------------------------------------------------------------',
494494
'# test | | | | ',
495495
'# fixtures | | | | ',
496496
'# test-runner | | | | ',
497-
'# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
498-
'# -----------------------------------------------------------------------------------------',
499-
'# all files | 78.65 | 38.46 | 60.00 | ',
500-
'# -----------------------------------------------------------------------------------------',
497+
'# coverage.js | 79.78 | 41.67 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72',
498+
'# --------------------------------------------------------------------------------------',
499+
'# all files | 79.78 | 41.67 | 60.00 | ',
500+
'# --------------------------------------------------------------------------------------',
501501
'# end of coverage report',
502502
].join('\n');
503503

@@ -514,18 +514,18 @@ test('coverage with included and excluded files', skipIfNoInspector, () => {
514514
test('correctly prints the coverage report of files contained in parent directories', skipIfNoInspector, () => {
515515
let report = [
516516
'# start of coverage report',
517-
'# --------------------------------------------------------------------------------------------',
517+
'# -----------------------------------------------------------------------------------------',
518518
'# file | line % | branch % | funcs % | uncovered lines',
519-
'# --------------------------------------------------------------------------------------------',
519+
'# -----------------------------------------------------------------------------------------',
520520
'# .. | | | | ',
521-
'# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
521+
'# coverage.js | 79.78 | 41.67 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72',
522522
'# invalid-tap.js | 100.00 | 100.00 | 100.00 | ',
523523
'# .. | | | | ',
524524
'# v8-coverage | | | | ',
525525
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
526-
'# --------------------------------------------------------------------------------------------',
527-
'# all files | 78.35 | 43.75 | 60.00 | ',
528-
'# --------------------------------------------------------------------------------------------',
526+
'# -----------------------------------------------------------------------------------------',
527+
'# all files | 79.38 | 46.67 | 60.00 | ',
528+
'# -----------------------------------------------------------------------------------------',
529529
'# end of coverage report',
530530
].join('\n');
531531

@@ -551,6 +551,53 @@ test('correctly prints the coverage report of files contained in parent director
551551
assert.strictEqual(result.status, 0);
552552
});
553553

554+
// Regression test for https://github.com/nodejs/node/issues/61586
555+
test('coverage ignore comments exclude branches in lcov output', skipIfNoInspector, () => {
556+
const fixture = fixtures.path(
557+
'test-runner', 'coverage-ignore-branch', 'test.js'
558+
);
559+
const args = [
560+
'--experimental-test-coverage',
561+
'--test-reporter', 'lcov',
562+
'--test-coverage-exclude=!test/fixtures/test-runner/coverage-ignore-branch/**',
563+
fixture,
564+
];
565+
const result = spawnSync(process.execPath, args);
566+
const lcov = result.stdout.toString();
567+
568+
assert.strictEqual(result.stderr.toString(), '');
569+
assert.strictEqual(result.status, 0);
570+
571+
// Extract the source.js section from the lcov output.
572+
const sourceSection = lcov.split('end_of_record')
573+
.find((s) => s.includes('source.js'));
574+
assert(sourceSection, 'lcov output should contain source.js coverage');
575+
576+
// Verify that all branches are reported as covered (BRH should equal BRF).
577+
// The ignored branch should not penalize coverage.
578+
const brfMatch = sourceSection.match(/BRF:(\d+)/);
579+
const brhMatch = sourceSection.match(/BRH:(\d+)/);
580+
assert.match(sourceSection, /BRF:\d+/, 'lcov should contain BRF');
581+
assert.match(sourceSection, /BRH:\d+/, 'lcov should contain BRH');
582+
assert.strictEqual(
583+
brfMatch[1],
584+
brhMatch[1],
585+
`All branches should be covered when ignored code is excluded. ` +
586+
`BRF=${brfMatch[1]}, BRH=${brhMatch[1]}`,
587+
);
588+
589+
// Verify no BRDA entries show 0 for the ignored branch.
590+
const brdaEntries = sourceSection.match(/BRDA:\d+,\d+,\d+,(\d+)/g) || [];
591+
for (const entry of brdaEntries) {
592+
const count = entry.match(/BRDA:\d+,\d+,\d+,(\d+)/)[1];
593+
assert.notStrictEqual(
594+
count,
595+
'0',
596+
`No branch should show 0 coverage when the path is ignored: ${entry}`,
597+
);
598+
}
599+
});
600+
554601
// Regression test for https://github.com/nodejs/node/issues/61080
555602
test('coverage with directory and file named "file"', skipIfNoInspector, () => {
556603
const fixture = fixtures.path('test-runner', 'coverage-file-name', 'test.js');

0 commit comments

Comments
 (0)