test_runner: exclude BRDA entries for ignored lines in LCOV reporter#62740
test_runner: exclude BRDA entries for ignored lines in LCOV reporter#62740sfgrdsgdsfdrfyg-code wants to merge 1 commit intonodejs:mainfrom
Conversation
|
Review requested:
|
|
can you please add / adjust tests? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62740 +/- ##
==========================================
- Coverage 89.69% 89.66% -0.04%
==========================================
Files 706 706
Lines 218129 218143 +14
Branches 41739 41733 -6
==========================================
- Hits 195649 195592 -57
- Misses 14411 14463 +52
- Partials 8069 8088 +19
🚀 New features to boost your workflow:
|
When a line is marked with /* node:coverage ignore next */, the DA entry for that line is correctly excluded from the LCOV output. However, the corresponding BRDA entry was still being emitted, causing branch coverage to report incorrect percentages when using the lcov reporter. PR-URL: nodejs#62740
0facea0 to
a7f3aed
Compare
|
Thanks for the review! I've amended the commit message with PR-URL. I'm looking at the existing lcov test fixture now to understand how to add coverage for the ignore-line BRDA filtering behavior. Will push a test update shortly. |
When a line is marked with /* node:coverage ignore next */, the DA entry for that line is correctly excluded from the LCOV output. However, the corresponding BRDA entry was still being emitted, causing branch coverage to report incorrect percentages when using the lcov reporter. This commit adds: - A new test fixture (brda_ignore_output.js) with a branch that has /* node:coverage ignore next */ to test the BRDA exclusion behavior - A new test runner (lcov_reporter_brda_ignore.js) for the fixture - A separate snapshot test case for this specific scenario - Updates to lcov.js to filter BRDA entries for ignored lines PR-URL: nodejs#62740
a7f3aed to
9dbb702
Compare
|
Update: Added specific test case for BRDA exclusion on ignored lines. Added:
The test exercises the new filtering logic in lcov.js. |
|
Duplicate of #61598 |
|
Thanks! You're right - this is a duplicate of #61598 which is already approved and mergeable. I'll close this PR in favor of #61598. The approach in #61598 (filtering at the coverage.js level) is cleaner than the lcov.js approach I used here. Thanks @RajeshKumar11 for beating me to it! |
What
When using
/* node:coverage ignore next */comments to exclude code from coverage, the DA (line coverage) entries are correctly excluded, but BRDA (branch coverage) entries for those same lines remain as uncovered. This causes branch coverage to report incorrect percentages.Why it matters
Broken branch coverage in CI pipelines that enforce thresholds. Affects anyone using
node --experimental-test-coveragewithlcovreporter.How
The
lcov.jsreporter was not filtering branches on ignored lines. Added a check to skip branches whose line is in the ignored line set.Closes #61586