Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix usage of excludeAfterRemap not to set the coverage always to 100 #463

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

prantlf
Copy link

@prantlf prantlf commented Apr 2, 2023

Do not use the excludePath callback. Remove the excluded sources at the end, after all coverage parts have been merged. This might be a problem in v8-to-istanbul, because istanbul-lib-coverage doesn't offer a method to filter the coverage data at the end.

Attempts to fix #462

Checklist
  • npm test, tests passing
  • npm run test:snap (to update the snapshot)
  • tests and/or benchmarks are included
  • documentation is changed or added

@tbo
Copy link

tbo commented Jun 18, 2023

This fixes the issue for me. @bcoe is there any chance to get this fix merged? Or is there another workaround?

… 100

Do not use the `excludePath` callback. Remove the excluded sources at the end, after all coverage parts have been merged. This might be a problem in `v8-to-istanbul`, because `istanbul-lib-coverage` doesn't offer a method to filter the coverage data at the end.

Attempts to fix bcoe#462
@prantlf prantlf force-pushed the exclude-after-remap-always-100 branch from 9113449 to 1aab44c Compare June 18, 2023 21:36
@bcoe
Copy link
Owner

bcoe commented Jan 3, 2024

Any chance you could add a test?

@timokoessler
Copy link

Unfortunately, I cannot use c8 because of the problem. Is there a chance to merge this pull request anyway?

@bcoe
Copy link
Owner

bcoe commented Jun 10, 2024

@timokoessler, @tbo, I would need to see a failing test to be able to land this (I would be very appreciative of a PR that just adds the failing test 👏

@prantlf has provided a repository with a reproduction, so I can use this as a starting point. I'll do my best to start putting a bit more work into this library again, and it seems like this bug should be high up the list of things to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If excludeAfterRemap is enabled, the coverage stats become constant 100
4 participants