fix/coverage-ignore-brda-61586

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++;
             }
 

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 };

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');
+});

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 },
 ];

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');