-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix(coverage-v8): fix duplicate statements when merging coverage from multiple projects #9365
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
Conversation
… multiple projects When the same file is covered by multiple projects with different transform modes (e.g., jsdom/SSR vs browser), Vite produces source maps with different end column positions for the same source location. Istanbul's native merge() treats these as different statements, causing inflated statement/function/branch counts. Root cause: Vite's SSR transform uses MagicString with `hires: "boundary"` mode which produces different source map column mappings than the browser transform. This results in `end.column: null` (from Infinity) in SSR coverage vs specific values like `end.column: 10` in browser coverage. This fix implements a smart merge strategy that matches statements by their start position only, ignoring end column differences. This correctly merges execution counts without duplicating coverage entries.
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The CI failure is a snapshot mismatch in The smart merge unit tests pass locally. Could a maintainer please re-run the failed jobs? |
AriPerkkio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need minimal reproduction for #9366 and then use that as test case here.
Once we got the minimal repro, we can actually see what's wrong.
@AriPerkkio , I created a repo to reproduce the issue, also pasted the following in to #9366, please let me know if you have any more information. Describe the bugWhen using Vitest Reproductionhttps://github.com/stevez/vitest-coverage-merge-bug git clone https://github.com/stevez/vitest-coverage-merge-bug
cd vitest-coverage-merge-bug
npm install
npm run test:coverage
node check-coverage.cjsOutput shows duplicate statements for Expected: 3 statements Root CauseVite's SSR transform (used for jsdom) produces source maps with System InfoUsed Package Managernpm Validations
|
Perfect, thanks! I hope this makes debugging this bug easier. Could you make this project public? Your root cause analysis sounds correct. I think we might want to instead apply the fix in Vite or |
sorry, I put the wrong url, this is the one: https://github.com/stevez/vitest-coverage-merge-bug |
|
@stevez could you make it even more minimal and remove Removing these lines seems to fix -import react from "@vitejs/plugin-react-swc";
export default defineConfig({
- plugins: [react()],
|
|
I can't remove the react plugin, since the test will not run, instead I tried to replace the @vitejs/plugin-react-swc with @vitejs/plugin-react I investigated further and found the root cause is in With SWC plugin:
With Babel plugin (
The SWC plugin produces different source maps for SSR vs browser transforms. The browser transform includes end column info that the SSR transform doesn't preserve. I've updated the reproduction repo to use That said, your coverage merge logic could still be more defensive - if two coverage entries have the same start location but different end columns (especially when one is |
Interesting, you are just testing the constants, for me I would rather test the button.tsx which uses the constants, then I want to see the results, yes the constants itself will work but combine with browser test using @vitejs/plugin-react-swc, then issues happens |
AriPerkkio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the correct way to fix the bug. Looking at this closer, it's visible that swc is not generating end mappings. When remapping the transpiled code back to sources we end up on wrong column. Istanbul assigns the column as Infinity in theses cases (to match the end of the line), which shows up as null in the report.
Correct way to fix this is to file bug report to SWC. Looking at their issue tracker, they've fixed similar coverage related source map bugs before too. Let's continue that on #9366.
Thanks for the PR, minimal reproduction and all the debugging! 🙌
|
It is more complicated, based on my experiment: With SWC plugin:
With Babel plugin (
the conclusion is @vitejs/plugin-react will generate end.column: null for both jsdom and browser, while @vitejs/plugin-react-swc generates end.column: null in jsdom, but generates valid value in browser mode, so the problem is not swc generates null, it is because its behavior is inconsistent between jsdom and browser; if you want to fix this issue, then you need to fix for both @vitejs/plugin-react and @vitejs/plugin-react-swc, since end.column: null is the root cause |


Description
When the same file is covered by multiple projects with different transform modes (e.g., jsdom/SSR vs browser), Vite produces source maps with different end column positions for the same source location. Istanbul's native merge() treats these as different statements, causing inflated statement/function/branch counts.
Resolves #9366
Root cause: Vite's SSR transform uses MagicString with
hires: "boundary"mode which produces different source map column mappings than the browser transform. This results inend.column: null(from Infinity) in SSR coverage vs specific values likeend.column: 10in browser coverage.This fix implements a smart merge strategy that matches statements by their start position only, ignoring end column differences. This correctly merges execution counts without duplicating coverage entries.
Example: A file with 15 statements was incorrectly reported as having 29 statements when covered by both jsdom and browser projects. After this fix, it correctly reports 15 statements.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.No documentation needed - this is an internal bug fix with no API changes.
Changesets
feat:,fix:,perf:,docs:, orchore:.