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

Source map resolution difference between Istanbul and V8 coverage plugins #5341

Closed
6 tasks done
just-boris opened this issue Mar 5, 2024 · 1 comment · Fixed by #5367
Closed
6 tasks done

Source map resolution difference between Istanbul and V8 coverage plugins #5341

just-boris opened this issue Mar 5, 2024 · 1 comment · Fixed by #5367
Labels
feat: coverage Issues and PRs related to the coverage feature p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@just-boris
Copy link

just-boris commented Mar 5, 2024

Describe the bug

In our project setup, we have a pre-processing step, so the test run against a lib folder instead of src. This causes differences in behavior between istanbul and v8 coverage providers.

When running tests with istanbul it follows the source maps and the resolves the file to the original index.ts source:

 % Coverage report from istanbul
----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files |     100 |      100 |     100 |     100 |
 index.ts |     100 |      100 |     100 |     100 |
----------|---------|----------|---------|---------|-------------------

But it does not happen with v8, the report shows transpiled index.js

 % Coverage report from v8
----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files |     100 |      100 |     100 |     100 |
 index.js |     100 |      100 |     100 |     100 |
----------|---------|----------|---------|---------|-------------------

This becomes an issue when uploading reports to tools like Codecov, because they cannot match up the coverage report against the source

Reproduction

Minimal repro can be found here: https://github.com/just-boris/vitest-coverage-issue-5341

System Info

System:
    OS: macOS 13.6.4
    CPU: (10) arm64 Apple M1 Pro
    Memory: 594.05 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.19.0 - ~/n/bin/node
    npm: 10.2.3 - ~/n/bin/npm
  Browsers:
    Chrome: 122.0.6261.94
    Edge: 122.0.2365.63
    Safari: 17.3.1
  npmPackages:
    @vitest/coverage-istanbul: ^1.3.1 => 1.3.1
    @vitest/coverage-v8: ^1.3.1 => 1.3.1
    vitest: ^1.3.1 => 1.3.1

Used Package Manager

npm

Validations

@AriPerkkio AriPerkkio added feat: coverage Issues and PRs related to the coverage feature p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Mar 10, 2024
@AriPerkkio
Copy link
Member

The root cause is in line below. V8 provider is setting the executed filename as map.sources without checking whether original source map had any other sources set.

Instead it should respect the original map.sources if it exists and resolve full path for that. With some quick testing the coverage looks much better. In the picture below the left side is current behavior and right one is with fixed sources. I added some uncovered if-branches to the original test code just to see that coverage reporting works after these changes:

image

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 p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants