-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix(coverage): apply patch from istanbuljs/istanbuljs#838 #9480
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Steve Zhang <huigang.zhang@gmail.com>
@vitest/browser
@vitest/browser-playwright
@vitest/browser-preview
@vitest/browser-webdriverio
@vitest/coverage-istanbul
@vitest/coverage-v8
@vitest/expect
@vitest/mocker
@vitest/pretty-format
@vitest/runner
@vitest/snapshot
@vitest/spy
@vitest/ui
@vitest/utils
vitest
@vitest/web-worker
@vitest/ws-client
commit: |
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@stevez could you verify if these preview version fix the issue in your project? |
|
Hi @AriPerkkio, I can verify that #9366 is getting fixed in this preview build, |
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 still need test case for this to pick up regressions. I've been trying to come up with one but haven't figured out good way without relying on third party plugins.
Maybe we need to manually construct incompatible transform results and feed those via custom plugin.
| expect(fileCoverage).toMatchInlineSnapshot(` | ||
| { | ||
| "branches": "0/0 (100%)", | ||
| "functions": "2/8 (25%)", | ||
| "lines": "1/4 (25%)", | ||
| "statements": "1/4 (25%)", | ||
| } | ||
| `) | ||
|
|
||
| expect(Object.values(fileCoverage.fnMap).map(fn => fn.name)).toMatchInlineSnapshot(` | ||
| [ | ||
| "sum", | ||
| "subtract", | ||
| "multiply", | ||
| "remainder", | ||
| "sum", | ||
| "subtract", | ||
| "multiply", | ||
| "remainder", | ||
| ] | ||
| `) |
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.
@stevez looks like merging is still wrong. Functions of this file are shown twice due to different end.column mappings.
This is the file tests are loading:
vitest/test/coverage-test/fixtures/src/math.ts
Lines 1 to 15 in c8d2c41
| export function sum(a: number, b: number) { | |
| return a + b | |
| } | |
| export function subtract(a: number, b: number) { | |
| return a - b | |
| } | |
| export function multiply(a: number, b: number) { | |
| return a * b | |
| } | |
| export function remainder(a: number, b:number) { | |
| return a % b | |
| } |
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.
Let me take a look tonight
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.
@AriPerkkio, I want to confirm the test case, the test failed due to trying to merge 2 different transforms coverage result, now both have different start column as well, I want to confirm do we need to support this scenario, or I feel it is a kind of expanding the scope
Description
Patching
istanbul-lib-coverageis tricky as it's still installed by multiple packages that depend on it. I'm a bit concerned users might run into this error due to multipleistanbul-lib-coverageinstances on runtime:https://github.com/istanbuljs/istanbuljs/blob/28ffdbc314596bdcb3007e85d30a62372602b262/packages/istanbul-lib-coverage/lib/file-coverage.js#L37-L42
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.Changesets
feat:,fix:,perf:,docs:, orchore:.