-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: extraneous coverage for --project (fix #6331) #7885
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: extraneous coverage for --project (fix #6331) #7885
Conversation
Coverage was reported for projects outside of the specified project filter. For example, given a monorepo with two projects, p1 and p2, `vitest run --coverage --project=p1` reported coverage for code from p2 at 0% because no tests from p2 were run.
@AriPerkkio Does this align with what you were thinking? I still need to familiarize myself with how the tests are written, but I wanted to ensure I was on the right track first. Also, should I move the duplicate code to a common location (like |
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.
This change would only filter out uncovered files outside the workspace project's root. We should filter out covered files too.
To do this, we'll need to make this.testExclude
an array, and create one for each project with their roots.
vitest/packages/coverage-istanbul/src/provider.ts
Lines 34 to 35 in 9be32fb
this.testExclude = new TestExclude({ | |
cwd: ctx.config.root, |
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
That should do it. It seems to work for my project. I will add tests when I figure that out 🙂 As an aside, I wasn't able to move the duplicate code to |
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.
Looks good so far.
As an aside, I wasn't able to move the duplicate code to BaseCoverageProvider because vitest/node doesn't reference test-exclude. I figured it wasn't worth adding the extra reference for this fix.
That's okey for now. In #7837 we'll remove test-exclude
completely and utilize packages that are available in vitest
package. That allows re-using logic from BaseProvider
.
For test cases we'll need to verify following:
- When
--project
filter is passed, files outside that project are excluded. This means covered and uncovered files. - When
--project
filter is not passed, files in projects and in the root should be visible.
You can add fixtures in test/coverage-test/fixtures/workspaces
and create test cases in new file test/coverage-test/test/workspace.test.ts
for example.
Hey @gtbuchanan, do you have any time to finalize this PR? I'm also happy to take over and finish it if you prefer that. Thanks for all the work so far! 💯 |
@AriPerkkio I would have liked to finish it myself, but I'm running into some issues:
EDIT: |
I didn't figure out how to debug the coverage tests in VS Code, but I was able to get everything working! It seems to work without updating |
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.
There must have been some caching involved. I picked the PR back up on my work computer and the test fails there as expected. Now to reapply the fix...
The test cases are running against built assets so you'll need to run pnpm build
or pnpm dev
before running the test cases. That might explain the caching issue.
I didn't figure out how to debug the coverage tests in VS Code
These test cases are running "Vitest inside Vitest" so it's likely that VS Code extension is unable to inspect those.
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.
Small improvements, otherwise this is looking good!
Btw as this PR is from a Github organization account (energyworldnet
), Vitest maintainers cannot push any changes to your PR branch. Usually I would fix small errors like this myself and push changes to your branch directly, but now it's not possible due to permission requirements.
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.
Looks good, thanks for the PR @gtbuchanan! 💯
Description
Coverage was reported for projects outside of the specified project filter. For example, given a monorepo with two projects, p1 and p2,
vitest run --coverage --project=p1
reported coverage for code from p2 at 0% because no tests from p2 were run.Resolves: #6331
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.