diff --git a/lib/internal/test_runner/coverage.js b/lib/internal/test_runner/coverage.js index 8fa9c872568d1e..6168b2b7dfbdf3 100644 --- a/lib/internal/test_runner/coverage.js +++ b/lib/internal/test_runner/coverage.js @@ -114,6 +114,7 @@ class TestCoverage { if (match !== null) { ignoreCount = NumberParseInt(match.groups?.count ?? 1, 10); + coverageLine.ignore = true; } } @@ -193,14 +194,33 @@ class TestCoverage { ObjectAssign(range, mapRangeToLines(range, lines)); if (isBlockCoverage) { + // Skip branches where all lines are ignored from coverage. + if (range.ignoredLines === range.lines.length) { + continue; + } + + // For uncovered branches with some ignored lines, skip if all + // non-ignored lines are already covered by other ranges. This + if (range.count === 0 && range.ignoredLines > 0) { + let hasUncoveredNonIgnoredLine = false; + for (let l = 0; l < range.lines.length; ++l) { + if (!range.lines[l].ignore && range.lines[l].count === 0) { + hasUncoveredNonIgnoredLine = true; + break; + } + } + if (!hasUncoveredNonIgnoredLine) { + continue; + } + } + ArrayPrototypePush(branchReports, { __proto__: null, line: range.lines[0]?.line, count: range.count, }); - if (range.count !== 0 || - range.ignoredLines === range.lines.length) { + if (range.count !== 0) { branchesCovered++; } diff --git a/test/fixtures/test-runner/coverage-ignore-branch/source.js b/test/fixtures/test-runner/coverage-ignore-branch/source.js new file mode 100644 index 00000000000000..4361ef51b60ff7 --- /dev/null +++ b/test/fixtures/test-runner/coverage-ignore-branch/source.js @@ -0,0 +1,22 @@ +'use strict'; + +function getValue(condition) { + if (condition) { + return 'truthy'; + } + /* node:coverage ignore next */ + return 'falsy'; +} + +// This function has a branch where the ignored line is mixed with +// non-ignored uncovered code, so the branch should still be reported. +function getMixed(condition) { + if (condition) { + return 'yes'; + } + /* node:coverage ignore next */ + const ignored = 'ignored'; + return ignored + ' no'; +} + +module.exports = { getValue, getMixed }; diff --git a/test/fixtures/test-runner/coverage-ignore-branch/test.js b/test/fixtures/test-runner/coverage-ignore-branch/test.js new file mode 100644 index 00000000000000..f6e58899fa62c2 --- /dev/null +++ b/test/fixtures/test-runner/coverage-ignore-branch/test.js @@ -0,0 +1,15 @@ +'use strict'; +const { test } = require('node:test'); +const assert = require('node:assert'); +const { getValue, getMixed } = require('./source.js'); + +// Only call with true so the false branch is "uncovered" but ignored. +test('getValue returns truthy for true', () => { + assert.strictEqual(getValue(true), 'truthy'); +}); + +// getMixed has a branch with both ignored and non-ignored uncovered lines. +// The branch should still be reported as uncovered. +test('getMixed returns yes for true', () => { + assert.strictEqual(getMixed(true), 'yes'); +}); diff --git a/test/parallel/test-runner-coverage-thresholds.js b/test/parallel/test-runner-coverage-thresholds.js index e45e1191299ca7..f59ff6ec6691fc 100644 --- a/test/parallel/test-runner-coverage-thresholds.js +++ b/test/parallel/test-runner-coverage-thresholds.js @@ -23,19 +23,19 @@ function getTapCoverageFixtureReport() { const report = [ '# start of coverage report', - '# --------------------------------------------------------------------------------------------', + '# -----------------------------------------------------------------------------------------', '# file | line % | branch % | funcs % | uncovered lines', - '# --------------------------------------------------------------------------------------------', + '# -----------------------------------------------------------------------------------------', '# test | | | | ', '# fixtures | | | | ', '# test-runner | | | | ', - '# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72', + '# coverage.js | 79.78 | 38.46 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72', '# invalid-tap.js | 100.00 | 100.00 | 100.00 | ', '# v8-coverage | | | | ', '# throw.js | 71.43 | 50.00 | 100.00 | 5-6', - '# --------------------------------------------------------------------------------------------', - '# all files | 78.35 | 43.75 | 60.00 | ', - '# --------------------------------------------------------------------------------------------', + '# -----------------------------------------------------------------------------------------', + '# all files | 79.38 | 43.75 | 60.00 | ', + '# -----------------------------------------------------------------------------------------', '# end of coverage report', ].join('\n'); @@ -51,7 +51,7 @@ const fixture = fixtures.path('test-runner', 'coverage.js'); const reporter = fixtures.fileURL('test-runner/custom_reporters/coverage.mjs'); const coverages = [ - { flag: '--test-coverage-lines', name: 'line', actual: 78.35 }, + { flag: '--test-coverage-lines', name: 'line', actual: 79.38 }, { flag: '--test-coverage-functions', name: 'function', actual: 60.00 }, { flag: '--test-coverage-branches', name: 'branch', actual: 43.75 }, ]; diff --git a/test/parallel/test-runner-coverage.js b/test/parallel/test-runner-coverage.js index 5a8f3d743538cb..f1eb132a03f9df 100644 --- a/test/parallel/test-runner-coverage.js +++ b/test/parallel/test-runner-coverage.js @@ -25,19 +25,19 @@ function getTapCoverageFixtureReport() { const report = [ '# start of coverage report', - '# --------------------------------------------------------------------------------------------', + '# -----------------------------------------------------------------------------------------', '# file | line % | branch % | funcs % | uncovered lines', - '# --------------------------------------------------------------------------------------------', + '# -----------------------------------------------------------------------------------------', '# test | | | | ', '# fixtures | | | | ', '# test-runner | | | | ', - '# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72', + '# coverage.js | 79.78 | 38.46 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72', '# invalid-tap.js | 100.00 | 100.00 | 100.00 | ', '# v8-coverage | | | | ', '# throw.js | 71.43 | 50.00 | 100.00 | 5-6', - '# --------------------------------------------------------------------------------------------', - '# all files | 78.35 | 43.75 | 60.00 | ', - '# --------------------------------------------------------------------------------------------', + '# -----------------------------------------------------------------------------------------', + '# all files | 79.38 | 43.75 | 60.00 | ', + '# -----------------------------------------------------------------------------------------', '# end of coverage report', ].join('\n'); @@ -53,19 +53,19 @@ function getSpecCoverageFixtureReport() { const report = [ '\u2139 start of coverage report', - '\u2139 --------------------------------------------------------------------------------------------', + '\u2139 -----------------------------------------------------------------------------------------', '\u2139 file | line % | branch % | funcs % | uncovered lines', - '\u2139 --------------------------------------------------------------------------------------------', + '\u2139 -----------------------------------------------------------------------------------------', '\u2139 test | | | | ', '\u2139 fixtures | | | | ', '\u2139 test-runner | | | | ', - '\u2139 coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72', + '\u2139 coverage.js | 79.78 | 38.46 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72', '\u2139 invalid-tap.js | 100.00 | 100.00 | 100.00 | ', '\u2139 v8-coverage | | | | ', '\u2139 throw.js | 71.43 | 50.00 | 100.00 | 5-6', - '\u2139 --------------------------------------------------------------------------------------------', - '\u2139 all files | 78.35 | 43.75 | 60.00 | ', - '\u2139 --------------------------------------------------------------------------------------------', + '\u2139 -----------------------------------------------------------------------------------------', + '\u2139 all files | 79.38 | 43.75 | 60.00 | ', + '\u2139 -----------------------------------------------------------------------------------------', '\u2139 end of coverage report', ].join('\n'); @@ -415,18 +415,18 @@ test('coverage with excluded files', skipIfNoInspector, () => { const result = spawnSync(process.execPath, args); const report = [ '# start of coverage report', - '# -----------------------------------------------------------------------------------------', + '# --------------------------------------------------------------------------------------', '# file | line % | branch % | funcs % | uncovered lines', - '# -----------------------------------------------------------------------------------------', + '# --------------------------------------------------------------------------------------', '# test | | | | ', '# fixtures | | | | ', '# test-runner | | | | ', - '# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72', + '# coverage.js | 79.78 | 38.46 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72', '# v8-coverage | | | | ', '# throw.js | 71.43 | 50.00 | 100.00 | 5-6', - '# -----------------------------------------------------------------------------------------', - '# all files | 78.13 | 40.00 | 60.00 | ', - '# -----------------------------------------------------------------------------------------', + '# --------------------------------------------------------------------------------------', + '# all files | 79.17 | 40.00 | 60.00 | ', + '# --------------------------------------------------------------------------------------', '# end of coverage report', ].join('\n'); @@ -452,18 +452,18 @@ test('coverage with included files', skipIfNoInspector, () => { const result = spawnSync(process.execPath, args); const report = [ '# start of coverage report', - '# -----------------------------------------------------------------------------------------', + '# --------------------------------------------------------------------------------------', '# file | line % | branch % | funcs % | uncovered lines', - '# -----------------------------------------------------------------------------------------', + '# --------------------------------------------------------------------------------------', '# test | | | | ', '# fixtures | | | | ', '# test-runner | | | | ', - '# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72', + '# coverage.js | 79.78 | 38.46 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72', '# v8-coverage | | | | ', '# throw.js | 71.43 | 50.00 | 100.00 | 5-6', - '# -----------------------------------------------------------------------------------------', - '# all files | 78.13 | 40.00 | 60.00 | ', - '# -----------------------------------------------------------------------------------------', + '# --------------------------------------------------------------------------------------', + '# all files | 79.17 | 40.00 | 60.00 | ', + '# --------------------------------------------------------------------------------------', '# end of coverage report', ].join('\n'); @@ -488,16 +488,16 @@ test('coverage with included and excluded files', skipIfNoInspector, () => { const result = spawnSync(process.execPath, args); const report = [ '# start of coverage report', - '# -----------------------------------------------------------------------------------------', + '# --------------------------------------------------------------------------------------', '# file | line % | branch % | funcs % | uncovered lines', - '# -----------------------------------------------------------------------------------------', + '# --------------------------------------------------------------------------------------', '# test | | | | ', '# fixtures | | | | ', '# test-runner | | | | ', - '# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72', - '# -----------------------------------------------------------------------------------------', - '# all files | 78.65 | 38.46 | 60.00 | ', - '# -----------------------------------------------------------------------------------------', + '# coverage.js | 79.78 | 38.46 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72', + '# --------------------------------------------------------------------------------------', + '# all files | 79.78 | 38.46 | 60.00 | ', + '# --------------------------------------------------------------------------------------', '# end of coverage report', ].join('\n'); @@ -514,18 +514,18 @@ test('coverage with included and excluded files', skipIfNoInspector, () => { test('correctly prints the coverage report of files contained in parent directories', skipIfNoInspector, () => { let report = [ '# start of coverage report', - '# --------------------------------------------------------------------------------------------', + '# -----------------------------------------------------------------------------------------', '# file | line % | branch % | funcs % | uncovered lines', - '# --------------------------------------------------------------------------------------------', + '# -----------------------------------------------------------------------------------------', '# .. | | | | ', - '# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72', + '# coverage.js | 79.78 | 38.46 | 60.00 | 12 16-22 27 39 43-44 61-62 66-67 71-72', '# invalid-tap.js | 100.00 | 100.00 | 100.00 | ', '# .. | | | | ', '# v8-coverage | | | | ', '# throw.js | 71.43 | 50.00 | 100.00 | 5-6', - '# --------------------------------------------------------------------------------------------', - '# all files | 78.35 | 43.75 | 60.00 | ', - '# --------------------------------------------------------------------------------------------', + '# -----------------------------------------------------------------------------------------', + '# all files | 79.38 | 43.75 | 60.00 | ', + '# -----------------------------------------------------------------------------------------', '# end of coverage report', ].join('\n'); @@ -551,6 +551,51 @@ test('correctly prints the coverage report of files contained in parent director assert.strictEqual(result.status, 0); }); +// Regression test for https://github.com/nodejs/node/issues/61586 +test('coverage ignore comments exclude branches in lcov output', skipIfNoInspector, () => { + const fixture = fixtures.path( + 'test-runner', 'coverage-ignore-branch', 'test.js' + ); + const args = [ + '--experimental-test-coverage', + '--test-reporter', 'lcov', + '--test-coverage-exclude=!test/fixtures/test-runner/coverage-ignore-branch/**', + fixture, + ]; + const result = spawnSync(process.execPath, args); + const lcov = result.stdout.toString(); + + assert.strictEqual(result.stderr.toString(), ''); + assert.strictEqual(result.status, 0); + + // Extract the source.js section from the lcov output. + const sourceSection = lcov.split('end_of_record') + .find((s) => s.includes('source.js')); + assert(sourceSection, 'lcov output should contain source.js coverage'); + + const brfMatch = sourceSection.match(/BRF:(\d+)/); + const brhMatch = sourceSection.match(/BRH:(\d+)/); + assert.match(sourceSection, /BRF:\d+/, 'lcov should contain BRF'); + assert.match(sourceSection, /BRH:\d+/, 'lcov should contain BRH'); + + // getValue's false branch is fully ignored, so it should be excluded. + assert.strictEqual( + Number(brfMatch[1]), + Number(brhMatch[1]) + 1, + `Expected exactly one uncovered branch (getMixed). ` + + `BRF=${brfMatch[1]}, BRH=${brhMatch[1]}`, + ); + + // Verify the only BRDA with count=0 is from getMixed (not getValue). + const brdaEntries = sourceSection.match(/BRDA:\d+,\d+,\d+,\d+/g) || []; + const uncoveredEntries = brdaEntries.filter((e) => e.endsWith(',0')); + assert.strictEqual( + uncoveredEntries.length, + 1, + `Expected exactly one uncovered BRDA entry, got: ${uncoveredEntries}`, + ); +}); + // Regression test for https://github.com/nodejs/node/issues/61080 test('coverage with directory and file named "file"', skipIfNoInspector, () => { const fixture = fixtures.path('test-runner', 'coverage-file-name', 'test.js');