fix(coverage): count a branch-junction line as covered when either arm runs#35858
Merged
bartlomieju merged 1 commit intoJul 8, 2026
Merged
Conversation
…m runs
The LCOV line-hit count for a source line is reset to zero when a
zero-count V8 coverage range overlaps the line and reaches one of its
edges. On a line that holds a branch junction the two edges belong to
different arms: the `}` that closes an `if` consequent and the `else`'s
opening `{` on a `} else {` line, and likewise `} catch {` and
`} finally {`. When only one arm runs, the arm that did not run leaves a
zero-count range that clips the opposite edge of the line, and the reset
then zeroes the whole line even though the covered side genuinely ran.
The visible effect is that a `} else {` line counts as covered only when
both arms run within the same coverage process. Run the two arms in
separate test files, so separate processes, and merge the resulting LCOV
per line, and the junction drops out of coverage. Its coverage then
depends on how the test suite happens to be split across processes.
Reset a line to zero only when a zero-count range genuinely covers its
code, judged three ways. The range spans the whole line content: a fully
uncovered line, or one in the middle of a multi-line uncovered block. The
range is an uncovered statement confined to the line that runs to the end
of its code: the never-taken `throw` in `if (!x) throw ...`. Or the range
is a whole never-called function (its ranges[0]), which keeps the
function's signature line uncovered like its body even though the range
starts a few characters into that line after an `export` or `async`
keyword. A multi-line block whose brace only clips a junction line extends
past the line end and matches none of the three, so the covered half of
the junction survives. Branch (BRDA) counts are unchanged.
A new spec runs a single arm of an `if`/`else` and asserts the `} else {`
line is covered by the arm that ran. Existing coverage expectations that
exercised one arm of a branch are updated: the junction line, and an
`if (...) {` line whose condition ran but whose body did not, now count as
covered.
Fixed denoland#35857
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(This PR was written by Opus 4.8 and reviewed by me, a human. I continue to have no real familiarity with this repository and so could not evaluate the fix for correctness, however it seems reasonable to me, though some of the details of the test expectation changes seemed a little strange. If there's anything I can do to help this land please let me know; this reduces some confusion we're seeing in our CI so I am motivated to address any feedback promptly.)
The LCOV line-hit count for a source line is reset to zero when a zero-count V8 coverage range overlaps the line and reaches one of its edges. On a line that holds a branch junction the two edges belong to different arms: the
}that closes anifconsequent and theelse's opening{on a} else {line, and likewise} catch {and} finally {. When only one arm runs, the arm that did not run leaves a zero-count range that clips the opposite edge of the line, and the reset then zeroes the whole line even though the covered side genuinely ran.The visible effect is that a
} else {line counts as covered only when both arms run within the same coverage process. Run the two arms in separate test files, so separate processes, and merge the resulting LCOV per line, and the junction drops out of coverage. Its coverage then depends on how the test suite happens to be split across processes.Reset a line to zero only when a zero-count range genuinely covers its code, judged three ways. The range spans the whole line content: a fully uncovered line, or one in the middle of a multi-line uncovered block. The range is an uncovered statement confined to the line that runs to the end of its code: the never-taken
throwinif (!x) throw .... Or the range is a whole never-called function (its ranges[0]), which keeps the function's signature line uncovered like its body even though the range starts a few characters into that line after anexportorasynckeyword. A multi-line block whose brace only clips a junction line extends past the line end and matches none of the three, so the covered half of the junction survives. Branch (BRDA) counts are unchanged.A new spec runs a single arm of an
if/elseand asserts the} else {line is covered by the arm that ran. Existing coverage expectations that exercised one arm of a branch are updated: the junction line, and anif (...) {line whose condition ran but whose body did not, now count as covered.Fixes #35857.