Skip to content

Commit 22507c1

Browse files
committed
test_runner: exclude ignored branches from coverage
When using /* node:coverage ignore next */ comments, branches where all lines are ignored were still being included in BRDA entries and counted in total branch count (BRF), even though lines were correctly excluded from DA entries. Skip branches entirely when all lines in the branch are ignored (range.ignoredLines === range.lines.length), matching the behavior of how ignored lines are excluded from line coverage. Before: BRDA:4,2,0,0 # ignored branch appears with count=0 BRF:3 # 3 total branches (including ignored) BRH:2 # 2 branches hit (66.67%) After: (no BRDA entry for ignored branch) BRF:2 # 2 total branches (ignored branch excluded) BRH:2 # 2 branches hit (100%) Refs: https://github.com/tobigumo/node-coverage-brda-bug
1 parent 9ee3566 commit 22507c1

File tree

1 file changed

+93
-42
lines changed

1 file changed

+93
-42
lines changed

lib/internal/test_runner/coverage.js

Lines changed: 93 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,12 @@ const { join, resolve, relative } = require('path');
3131
const { fileURLToPath, URL } = require('internal/url');
3232
const { kMappings, SourceMap } = require('internal/source_map/source_map');
3333
const {
34-
codes: {
35-
ERR_SOURCE_MAP_CORRUPT,
36-
ERR_SOURCE_MAP_MISSING_SOURCE,
37-
},
34+
codes: { ERR_SOURCE_MAP_CORRUPT, ERR_SOURCE_MAP_MISSING_SOURCE },
3835
} = require('internal/errors');
3936
const { matchGlobPattern } = require('internal/fs/glob');
40-
const { constants: { kMockSearchParam } } = require('internal/test_runner/mock/loader');
37+
const {
38+
constants: { kMockSearchParam },
39+
} = require('internal/test_runner/mock/loader');
4140

4241
const kCoverageFileRegex = /^coverage-(\d+)-(\d{13})-(\d+)\.json$/;
4342
const kIgnoreRegex = /\/\* node:coverage ignore next (?<count>\d+ )?\*\//;
@@ -47,8 +46,10 @@ const kStatusRegex = /\/\* node:coverage (?<status>enable|disable) \*\//;
4746

4847
class CoverageLine {
4948
constructor(line, startOffset, src, length = src?.length) {
50-
const newlineLength = src == null ? 0 :
51-
RegExpPrototypeExec(kLineEndingRegex, src)?.[0].length ?? 0;
49+
const newlineLength =
50+
src == null
51+
? 0
52+
: (RegExpPrototypeExec(kLineEndingRegex, src)?.[0].length ?? 0);
5253

5354
this.line = line;
5455
this.src = src;
@@ -60,9 +61,7 @@ class CoverageLine {
6061
}
6162

6263
class TestCoverage {
63-
constructor(coverageDirectory,
64-
originalCoverageDirectory,
65-
options) {
64+
constructor(coverageDirectory, originalCoverageDirectory, options) {
6665
this.coverageDirectory = coverageDirectory;
6766
this.originalCoverageDirectory = originalCoverageDirectory;
6867
this.options = options;
@@ -87,8 +86,7 @@ class TestCoverage {
8786
return;
8887
}
8988

90-
const linesWithBreaks =
91-
RegExpPrototypeSymbolSplit(kLineSplitRegex, source);
89+
const linesWithBreaks = RegExpPrototypeSymbolSplit(kLineSplitRegex, source);
9290
let ignoreCount = 0;
9391
let enabled = true;
9492
let offset = 0;
@@ -179,7 +177,6 @@ class TestCoverage {
179177
continue;
180178
}
181179

182-
183180
for (let j = 0; j < functions.length; ++j) {
184181
const { isBlockCoverage, ranges } = functions[j];
185182

@@ -193,14 +190,18 @@ class TestCoverage {
193190
ObjectAssign(range, mapRangeToLines(range, lines));
194191

195192
if (isBlockCoverage) {
193+
// Skip branches where all lines are ignored
194+
if (range.ignoredLines === range.lines.length) {
195+
continue;
196+
}
197+
196198
ArrayPrototypePush(branchReports, {
197199
__proto__: null,
198200
line: range.lines[0]?.line,
199201
count: range.count,
200202
});
201203

202-
if (range.count !== 0 ||
203-
range.ignoredLines === range.lines.length) {
204+
if (range.count !== 0) {
204205
branchesCovered++;
205206
}
206207

@@ -297,10 +298,13 @@ class TestCoverage {
297298
let dir;
298299

299300
try {
300-
mkdirSync(this.originalCoverageDirectory, { __proto__: null, recursive: true });
301+
mkdirSync(this.originalCoverageDirectory, {
302+
__proto__: null,
303+
recursive: true,
304+
});
301305
dir = opendirSync(this.coverageDirectory);
302306

303-
for (let entry; (entry = dir.readSync()) !== null;) {
307+
for (let entry; (entry = dir.readSync()) !== null; ) {
304308
const src = join(this.coverageDirectory, entry.name);
305309
const dst = join(this.originalCoverageDirectory, entry.name);
306310
copyFileSync(src, dst);
@@ -326,7 +330,7 @@ class TestCoverage {
326330
try {
327331
dir = opendirSync(this.coverageDirectory);
328332

329-
for (let entry; (entry = dir.readSync()) !== null;) {
333+
for (let entry; (entry = dir.readSync()) !== null; ) {
330334
if (RegExpPrototypeExec(kCoverageFileRegex, entry.name) === null) {
331335
continue;
332336
}
@@ -344,7 +348,6 @@ class TestCoverage {
344348
}
345349
}
346350

347-
348351
mapCoverageWithSourceMap(coverage) {
349352
const { result } = coverage;
350353
const sourceMapCache = coverage['source-map-cache'];
@@ -386,18 +389,39 @@ class TestCoverage {
386389
const { startOffset, endOffset, count } = ranges[k];
387390
const { lines } = mapRangeToLines(ranges[k], executedLines);
388391

389-
let startEntry = sourceMap
390-
.findEntry(lines[0].line - 1, MathMax(0, startOffset - lines[0].startOffset));
391-
const endEntry = sourceMap
392-
.findEntry(lines[lines.length - 1].line - 1, (endOffset - lines[lines.length - 1].startOffset) - 1);
393-
if (!startEntry.originalSource && endEntry.originalSource &&
394-
lines[0].line === 1 && startOffset === 0 && lines[0].startOffset === 0) {
392+
let startEntry = sourceMap.findEntry(
393+
lines[0].line - 1,
394+
MathMax(0, startOffset - lines[0].startOffset),
395+
);
396+
const endEntry = sourceMap.findEntry(
397+
lines[lines.length - 1].line - 1,
398+
endOffset - lines[lines.length - 1].startOffset - 1,
399+
);
400+
if (
401+
!startEntry.originalSource &&
402+
endEntry.originalSource &&
403+
lines[0].line === 1 &&
404+
startOffset === 0 &&
405+
lines[0].startOffset === 0
406+
) {
395407
// Edge case when the first line is not mappable
396-
const { 2: originalSource, 3: originalLine, 4: originalColumn } = sourceMap[kMappings][0];
397-
startEntry = { __proto__: null, originalSource, originalLine, originalColumn };
408+
const {
409+
2: originalSource,
410+
3: originalLine,
411+
4: originalColumn,
412+
} = sourceMap[kMappings][0];
413+
startEntry = {
414+
__proto__: null,
415+
originalSource,
416+
originalLine,
417+
originalColumn,
418+
};
398419
}
399420

400-
if (!startEntry.originalSource || startEntry.originalSource !== endEntry.originalSource) {
421+
if (
422+
!startEntry.originalSource ||
423+
startEntry.originalSource !== endEntry.originalSource
424+
) {
401425
// The range is not mappable. Skip it.
402426
continue;
403427
}
@@ -413,21 +437,37 @@ class TestCoverage {
413437
// The range is not mappable. Skip it.
414438
continue;
415439
}
416-
for (let l = startEntry.originalLine; l <= endEntry.originalLine; l++) {
440+
for (
441+
let l = startEntry.originalLine;
442+
l <= endEntry.originalLine;
443+
l++
444+
) {
417445
mappedLines[l].count = count;
418446
}
419447

420448
ArrayPrototypePush(newRanges, {
421-
__proto__: null, startOffset: mappedStartOffset, endOffset: mappedEndOffset, count,
449+
__proto__: null,
450+
startOffset: mappedStartOffset,
451+
endOffset: mappedEndOffset,
452+
count,
422453
});
423454
}
424455

425456
if (!newUrl) {
426457
// No mappable ranges. Skip the function.
427458
continue;
428459
}
429-
const newScript = newResult.get(newUrl) ?? { __proto__: null, url: newUrl, functions: [] };
430-
ArrayPrototypePush(newScript.functions, { __proto__: null, functionName, ranges: newRanges, isBlockCoverage });
460+
const newScript = newResult.get(newUrl) ?? {
461+
__proto__: null,
462+
url: newUrl,
463+
functions: [],
464+
};
465+
ArrayPrototypePush(newScript.functions, {
466+
__proto__: null,
467+
functionName,
468+
ranges: newRanges,
469+
isBlockCoverage,
470+
});
431471
newResult.set(newUrl, newScript);
432472
}
433473
}
@@ -442,7 +482,10 @@ class TestCoverage {
442482
// Return -1 if the line is not mappable.
443483
return -1;
444484
}
445-
return MathMin(mappedLine.startOffset + entry.originalColumn, mappedLine.endOffset);
485+
return MathMin(
486+
mappedLine.startOffset + entry.originalColumn,
487+
mappedLine.endOffset,
488+
);
446489
}
447490

448491
mergeCoverage(merged, coverage) {
@@ -483,7 +526,8 @@ class TestCoverage {
483526
if (
484527
matchGlobPattern(relativePath, excludeGlobs[i]) ||
485528
matchGlobPattern(absolutePath, excludeGlobs[i])
486-
) return true;
529+
)
530+
return true;
487531
}
488532
}
489533

@@ -493,7 +537,8 @@ class TestCoverage {
493537
if (
494538
matchGlobPattern(relativePath, includeGlobs[i]) ||
495539
matchGlobPattern(absolutePath, includeGlobs[i])
496-
) return false;
540+
)
541+
return false;
497542
}
498543
return true;
499544
}
@@ -595,9 +640,11 @@ function mergeCoverageScripts(oldScript, newScript) {
595640
for (let j = 0; j < oldScript.functions.length; ++j) {
596641
const oldFn = oldScript.functions[j];
597642

598-
if (newFn.functionName === oldFn.functionName &&
599-
newFn.ranges?.[0].startOffset === oldFn.ranges?.[0].startOffset &&
600-
newFn.ranges?.[0].endOffset === oldFn.ranges?.[0].endOffset) {
643+
if (
644+
newFn.functionName === oldFn.functionName &&
645+
newFn.ranges?.[0].startOffset === oldFn.ranges?.[0].startOffset &&
646+
newFn.ranges?.[0].endOffset === oldFn.ranges?.[0].endOffset
647+
) {
601648
// These are the same functions.
602649
found = true;
603650

@@ -686,13 +733,17 @@ function mergeCoverageRanges(oldFn, newFn) {
686733
}
687734

688735
function doesRangeEqualOtherRange(range, otherRange) {
689-
return range.startOffset === otherRange.startOffset &&
690-
range.endOffset === otherRange.endOffset;
736+
return (
737+
range.startOffset === otherRange.startOffset &&
738+
range.endOffset === otherRange.endOffset
739+
);
691740
}
692741

693742
function doesRangeContainOtherRange(range, otherRange) {
694-
return range.startOffset <= otherRange.startOffset &&
695-
range.endOffset >= otherRange.endOffset;
743+
return (
744+
range.startOffset <= otherRange.startOffset &&
745+
range.endOffset >= otherRange.endOffset
746+
);
696747
}
697748

698749
module.exports = { setupCoverage, TestCoverage };

0 commit comments

Comments
 (0)