test: enable vitest isolate:false on safe packages#5962
Open
killagu wants to merge 1 commit into
Open
Conversation
Run test files within a project in a single reused worker thread (`pool: 'threads'`, `isolate: false`) instead of spawning a fresh worker per file, to speed up the suite. The root `vitest.config.ts` already declared these options, but in Vitest 4 root `test` options are NOT inherited by glob-referenced projects, so every project was silently running with the defaults (`isolate: true`, `forks`). This sets the options explicitly on each project that is verified safe. Scope: 66 projects confirmed green under `isolate: false` + `threads`. Deliberately excluded (tracked as follow-up): - packages/cluster, plugins/schedule, plugins/mock, packages/egg, plugins/development, plugins/view-nunjucks — these spawn real Egg cluster/app child processes via `mm.cluster`, whose fixed-port allocation in plugins/mock (`globalThis.eggMockMasterPort = 17000 + process.pid % 1000`) assumes one OS process per test file. Sharing a worker resets/collides that counter, causing flaky EADDRINUSE failures. Needs a mock port-allocation fix (idempotent seed or ephemeral `port: 0`) before they can opt in. - packages/logger — sets `fileParallelism: false` (maxWorkers: 1); under isolate:false Vitest 4 requires a uniform maxWorkers per sequence.groupOrder, so it would abort whole-suite collection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (66)
📝 WalkthroughWalkthroughThis PR standardizes Vitest worker pool configuration across 70+ monorepo packages and plugins by adding or updating ChangesVitest Worker Pool and Isolation Settings
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying egg with
|
| Latest commit: |
0eb57bf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://794095be.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-vitest-isolate-false-sa.egg-cci.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request configures Vitest across multiple packages and plugins to reuse a single worker thread and disable isolation (pool: 'threads' and isolate: false) to speed up the test suite. Feedback suggests removing the unnecessary and incorrect UserWorkspaceConfig type annotations to let TypeScript infer the correct type, and replacing defineConfig with defineProject in plugins/redis and tegg/core/vitest configurations to ensure proper workspace integration.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+1
to
+11
| import { defineProject, type UserWorkspaceConfig } from 'vitest/config'; | ||
|
|
||
| const config: UserWorkspaceConfig = defineProject({ | ||
| test: { | ||
| // Reuse a single worker thread across this project's test files instead of | ||
| // spawning a fresh worker per file. The root vitest.config.ts cannot set | ||
| // these for glob-matched projects, so each project opts in explicitly. | ||
| pool: 'threads', | ||
| isolate: false, | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
The explicit UserWorkspaceConfig type annotation is incorrect for a project configuration (which should be UserProjectConfig or inferred) and is unnecessary. We can let TypeScript automatically infer the correct type returned by defineProject and remove the unused import.
This suggestion can be applied to all the newly added vitest.config.ts files in this PR to keep them clean and consistent.
Suggested change
| import { defineProject, type UserWorkspaceConfig } from 'vitest/config'; | |
| const config: UserWorkspaceConfig = defineProject({ | |
| test: { | |
| // Reuse a single worker thread across this project's test files instead of | |
| // spawning a fresh worker per file. The root vitest.config.ts cannot set | |
| // these for glob-matched projects, so each project opts in explicitly. | |
| pool: 'threads', | |
| isolate: false, | |
| }, | |
| }); | |
| import { defineProject } from 'vitest/config'; | |
| const config = defineProject({ | |
| test: { | |
| // Reuse a single worker thread across this project's test files instead of | |
| // spawning a fresh worker per file. The root vitest.config.ts cannot set | |
| // these for glob-matched projects, so each project opts in explicitly. | |
| pool: 'threads', | |
| isolate: false, | |
| }, | |
| }); |
Comment on lines
+5
to
+6
| pool: 'threads', | ||
| isolate: false, |
Contributor
There was a problem hiding this comment.
In Vitest workspaces, individual project configurations should use defineProject instead of defineConfig to ensure correct configuration merging and behavior within the workspace. Additionally, the explicit UserWorkspaceConfig type annotation can be removed to let TypeScript automatically infer the correct UserProjectConfig type returned by defineProject.
Please consider updating this file to use defineProject and remove the explicit type annotation, similar to the other project configurations.
Comment on lines
+23
to
+24
| pool: 'threads', | ||
| isolate: false, |
Contributor
There was a problem hiding this comment.
Similar to other workspace projects, tegg/core/vitest should use defineProject instead of defineConfig to ensure proper integration and configuration merging within the Vitest workspace.
Please consider updating the import and changing defineConfig to defineProject in this file.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## next #5962 +/- ##
===========================================
- Coverage 85.30% 53.31% -32.00%
===========================================
Files 670 12 -658
Lines 19552 287 -19265
Branches 3863 69 -3794
===========================================
- Hits 16678 153 -16525
+ Misses 2481 113 -2368
+ Partials 393 21 -372 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes Vitest project configs explicitly opt into pool: 'threads' + isolate: false so that glob-referenced workspace projects actually run with the intended shared-worker settings (improving test wall-clock time) rather than silently falling back to Vitest defaults.
Changes:
- Updated existing
vitest.config.tsfiles to addtest.pool = 'threads'andtest.isolate = false. - Added new per-project
vitest.config.tsfiles for projects that previously relied on defaults, explicitly opting them into shared-worker execution. - Kept package-specific overrides (timeouts, include/exclude, etc.) intact while enabling the shared-worker mode where configured.
Reviewed changes
Copilot reviewed 66 out of 66 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/create-egg/vitest.config.ts | Enable threads pool + isolate: false for this project’s tests |
| tegg/standalone/standalone/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/tegg/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/schedule/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/orm/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/mcp-proxy/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/mcp-client/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/langchain/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/eventbus/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/dal/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/controller/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/config/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/common/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/aop/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/plugin/ajv/vitest.config.ts | Add threads pool + isolate: false while preserving hook timeout |
| tegg/core/vitest/vitest.config.ts | Add threads pool + isolate: false to the custom-runner project config |
| tegg/core/types/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/transaction-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/test-util/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/tegg/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/standalone-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/schedule-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/runtime/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/orm-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/metadata/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/mcp-client/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/loader/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/lifecycle/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/langchain-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/eventbus-runtime/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/eventbus-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/dynamic-inject/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/dynamic-inject-runtime/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/dal-runtime/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/dal-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/core-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/controller-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/common-util/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/background-task/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/aop-runtime/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/aop-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/ajv-decorator/vitest.config.ts | New project config opting into threads + isolate: false |
| tegg/core/agent-runtime/vitest.config.ts | Convert empty project config to explicitly set threads + isolate: false |
| plugins/watcher/vitest.config.ts | Add threads pool + isolate: false to plugin tests |
| plugins/view/vitest.config.ts | Add threads pool + isolate: false to plugin tests |
| plugins/typebox-validate/vitest.config.ts | New project config opting into threads + isolate: false |
| plugins/tracer/vitest.config.ts | New project config opting into threads + isolate: false |
| plugins/static/vitest.config.ts | New project config opting into threads + isolate: false |
| plugins/session/vitest.config.ts | New project config opting into threads + isolate: false |
| plugins/redis/vitest.config.ts | Add threads pool + isolate: false while preserving plugin test settings |
| plugins/onerror/vitest.config.ts | Add threads pool + isolate: false to plugin tests |
| plugins/logrotator/vitest.config.ts | Add threads pool + isolate: false to plugin tests |
| plugins/jsonp/vitest.config.ts | New project config opting into threads + isolate: false |
| plugins/i18n/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/utils/vitest.config.ts | Add threads pool + isolate: false to package tests |
| packages/tsconfig/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/supertest/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/router/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/path-matching/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/loader-fs/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/koa/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/koa-static-cache/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/extend2/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/errors/vitest.config.ts | New project config opting into threads + isolate: false |
| packages/core/vitest.config.ts | Add threads pool + isolate: false to package tests |
| packages/cookies/vitest.config.ts | New project config opting into threads + isolate: false |
Deploying egg-v3 with
|
| Latest commit: |
0eb57bf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://71a95941.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-vitest-isolate-false-sa.egg-v3.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Run test files within a project in a single reused worker thread (
pool: 'threads',isolate: false) instead of spawning a fresh worker per file, to speed up the suite.The root
vitest.config.tsalready declaredisolate: false/pool: 'threads', but in Vitest 4 roottestoptions are not inherited by glob-referenced projects — every project was silently running with the defaults (isolate: true,forks). This sets the options explicitly on each project that is verified safe.On the tegg-plugin pilot this cut wall-clock from ~25s to ~18s.
Scope
66 projects confirmed green under
isolate: false+threads(each run individually via its committed config, 0 failures).Deliberately excluded (follow-up)
packages/cluster,plugins/schedule,plugins/mock,packages/egg,plugins/development,plugins/view-nunjucks— these spawn real Egg cluster/app child processes viamm.cluster, whose fixed-port allocation inplugins/mock/src/lib/cluster.ts(globalThis.eggMockMasterPort = 17000 + (process.pid % 1000)) assumes one OS process per test file. Sharing a worker resets/collides that counter → flakyEADDRINUSE. These can opt in after the mock port allocation is made isolation-safe (idempotent seed via??=, or bind an ephemeralport: 0and read it back from the existingegg-readymessage).packages/logger— setsfileParallelism: false(→maxWorkers: 1); underisolate: falseVitest 4 requires a uniformmaxWorkerspersequence.groupOrder, which would abort whole-suite collection.The root
vitest.config.tsis intentionally left unchanged in this PR (its deadisolate/pooloptions can be cleaned up separately).Test evidence
vitest list→ 3280 tests, nosequence.groupOrdererror).vitest run --project=<name>).🤖 Generated with Claude Code
Summary by CodeRabbit