fix: do not let --all overwrite coverage of files that were loaded - #1622
Open
Eljees wants to merge 1 commit into
Open
fix: do not let --all overwrite coverage of files that were loaded#1622Eljees wants to merge 1 commit into
Eljees wants to merge 1 commit into
Conversation
addAllFiles wrote its zeroed placeholder into the global coverage object unconditionally, replacing real counters for files that had been exercised. The existing comment already described the intended condition; this adds it. Signed-off-by: Eljees <3.14hell@gmail.com>
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.
Fixes #1372
--allcan wipe out the coverage of files that were exercised, which is why the report comes back at0% for everything rather than just for the untouched files.
addAllFiles()writes its placeholder straight into the global coverage object, unconditionally:The comment already states the intended rule — only use this data if we don't have it without
all: true— but there is no condition in the code implementing it. When a file already has realcounters in
coverage[path], the freshly instrumented (all-zero) copy replaces them.The fix is the missing check. Everything else stays as it was: files that were never loaded still get
their zeroed placeholder.
Before / after
New case in
test/add-all-files.js— real counters are put in the global coverage object first, thenaddAllFiles()runs, and the test asserts they survived:Run with
tap test/add-all-files.jsafternpm run instrument, on node 24.I kept the wording of the original comment and only moved it above the
if, since it now describes acondition that actually exists.
AI-assisted (LLM used for drafting); the change and both runs above are mine.