-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(coverage): add coverage.changed option to report only changed files #9521
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
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Initial approach looks good, but I would rather see all this logic being added into BaseCoverageProvider so that it doesn't leak into core.ts for example.
In the beginning of reportCoverage:
vitest/packages/vitest/src/node/coverage.ts
Line 333 in 7ce3417
| async reportCoverage(coverageMap: unknown, { allTestsRun }: ReportContext): Promise<void> { |
... we would do something like
if(this.options.changed) {
this.changedFiles = this.getChangedFiles()
}The getChangedFiles would use VitestGit like you've done now in core.ts. Then in isIncluded and getUntestedFilesByRoot we would be checking the this.changedFiles.
Would that work?
packages/vitest/src/node/core.ts
Outdated
| private async getCoverageReportContext(allTestsRun: boolean): Promise<ReportContext> { | ||
| const reportContext: ReportContext = { allTestsRun } | ||
| const coverageChanged = this._coverageOptions.changed | ||
| if (!coverageChanged) { | ||
| return reportContext | ||
| } | ||
| const { VitestGit } = await import('./git') | ||
| const vitestGit = new VitestGit(this.config.root) | ||
| const changedFiles = await vitestGit.findChangedFiles({ | ||
| changedSince: coverageChanged, | ||
| }) | ||
| if (!changedFiles) { | ||
| process.exitCode = 1 | ||
| throw new GitNotFoundError() | ||
| } | ||
| reportContext.changedFiles = Array.from(new Set(changedFiles)) | ||
| return reportContext | ||
| } |
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 think this logic should be here instead:
| export class BaseCoverageProvider<Options extends ResolvedCoverageOptions<'istanbul' | 'v8'>> { |
packages/vitest/src/node/coverage.ts
Outdated
| if (this.changedFiles) { | ||
| finalCoverageMap.filter(filename => this.changedFiles!.has(slash(filename))) | ||
| } |
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.
Filtering at this point will be more expensive than filtering before remapping is done, e.g. in isIncluded.
|
Sorry for the late response! That makes sense, I'll move the logic into |
|
@AriPerkkio I updated the code to move the logic into |
Description
This PR adds a new
coverage.changedoption that allows running all tests while only computing coverage for changed files. This is useful for CI pipelines that need to:Previously, this required running two separate commands, which doubled the test execution time.
Resolves #8747
Usage
The
changedoption accepts:'HEAD'- Compare against the last commit'main'- Compare against the main branch'origin/main'- Compare against remote main branchTesting Instructions
Navigate to the test directory:
cd test/coverage-testCreate a demo config file
vitest.demo.config.ts:Run tests without the
changedoption:Result: Coverage includes all files
Modify
fixtures/src/file-to-change.ts(make any change)Update config to add
changed: 'HEAD':Run tests again:
Result: Coverage includes only changed files
Note: Both test runs execute all tests (
file-to-change.test.tsandmath.test.ts), but the second run only reports coverage for the modified file.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:.