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

Coverage report is incorrect since 0.34.0 #3888

Closed
6 tasks done
nstepien opened this issue Aug 4, 2023 · 6 comments · Fixed by #4309
Closed
6 tasks done

Coverage report is incorrect since 0.34.0 #3888

nstepien opened this issue Aug 4, 2023 · 6 comments · Fixed by #4309
Labels
feat: coverage Issues and PRs related to the coverage feature

Comments

@nstepien
Copy link
Contributor

nstepien commented Aug 4, 2023

Describe the bug

When multiple environments are used, it seems like this affects how coverage is computed.
I've also noticed that the coverage report is different on my machine vs CI.

Please check this branch: adazzle/react-data-grid#3298
The coverage dropped significantly

  • CI:
    • image
  • On my machine:
    • image
  • Codecov:
    • image

In test/ssr.test.tsx, if I skip the test, the coverage INCREASES:
image

Again in test/ssr.test.tsx, if I skip the test AND remove // @vitest-environment node, the coverage INCREASES further, back to normal levels:
image

It seems like Vitest 0.34 does not like multiple environments. 🤔

Reproduction

System Info

System:
    OS: Windows 10 10.0.22621
    CPU: (64) x64 AMD Ryzen Threadripper 3970X 32-Core Processor
    Memory: 19.10 GB / 31.88 GB
  Binaries:
    Node: 20.5.0 - C:\Program Files\nodejs\node.EXE
    npm: 9.8.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22621.2070.0), Chromium (115.0.1901.188)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @vitejs/plugin-react: ^4.0.4 => 4.0.4
    @vitest/coverage-v8: ^0.34.1 => 0.34.1
    vite: ^4.4.8 => 4.4.8
    vitest: ^0.34.1 => 0.34.1

Used Package Manager

npm

Validations

@AriPerkkio
Copy link
Member

AriPerkkio commented Aug 4, 2023

This sounds like #3251. In this case the transformMode differs.

A single file is tested in multiple environments. Vitest will pick source maps from the first result it finds.

@AriPerkkio AriPerkkio added feat: coverage Issues and PRs related to the coverage feature bug and removed pending triage labels Aug 4, 2023
@AriPerkkio
Copy link
Member

Any ideas how to setup a minimal reproduction for this case? Using whole react-data-grid for debugging is too much.

@nstepien
Copy link
Contributor Author

nstepien commented Aug 5, 2023

You can reduce vite.config.ts down to

import { defineConfig } from 'vite';

export default defineConfig({
  test: {
    environment: 'jsdom',
    globals: true,
    coverage: {
      provider: 'v8',
      enabled: true,
      include: ['src/**/*.{ts,tsx}', '!src/types.ts'],
      all: true
    },
    setupFiles: ['test/setup.ts']
  }
});

ssr.test.tsx is the only test file with the node environment, so you can delete most other test files, should be enough to generate different code coverage, for example:
image
Not sure what else to remove, we do need some code to cover after all 🙂

AriPerkkio added a commit to AriPerkkio/vitest that referenced this issue Aug 5, 2023
@AriPerkkio
Copy link
Member

Minimal reproduction case now at AriPerkkio@b1301eb. The custom Vite plugin transforms the source file differently depending on SSR/CSR. This intentionally causes different padding to source maps and messes the V8 results. Same should happen with Istanbul as well.

On the left side is shown what coverage report looks like when same file is tested in Node and JSDOM. On the right side is what it looks like when it's tested only on one environment. The left one is clearly off.

We'll need to tell coverage providers the transformMode here:

testRunner.onAfterRun = async (files) => {
const coverage = await takeCoverageInsideWorker(config.coverage, executor)
rpc().onAfterSuiteRun({ coverage })

Then they can use it in provider when picking the correct source map:

const transformResult = transformResults.get(filePath)

This requires vite-node to store the transformMode here:

fetchCache = new Map<string, {
duration?: number
timestamp: number
result: FetchResult
}>()

Currently vite-node's fetchCache is using filename as key. This won't work when same filename can have different transform result. Key should contain filename and transformMode.

@sheremet-va
Copy link
Member

Then they can use it in provider when picking the correct source map:

I also want to point out that it might not have it if tests were running using browser. I remember that it would throw an error before, but currently, it just runs it with empty coverage.

@AriPerkkio
Copy link
Member

I have now tested the idea described above and it works. However I'll need more time to finalize the implementation.

Here a file is transformed differently based on transform mode. On web it adds plenty of padding so that covered lines do not match with the SSR version. The coverage reports below are for individual test runs for each transform mode.

Here on left it's shown how the coverage is currently merged incorrectly, using the first source map Vitest finds. On right side is shown how the coverage report looks like when both coverage results are traced back using their own source maps, and then merged with Istanbul:

Happy to see that this actually works in practice. 😅

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: coverage Issues and PRs related to the coverage feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants