build(deps-dev): bump vitest to 5.0.0 - #2654
Merged
Merged
Conversation
@vitest/coverage-v8@5.0.0 declares an exact peer on vitest@5.0.0, so the two have to move together. Bumping only the coverage package fails npm ci with ERESOLVE, which is why #2652 is red on every build job. Vitest 5 no longer leaks spy state between tests. That exposed four assertions in webserver.test.ts that were counting calls accumulated from earlier tests rather than their own, so they asserted 2, 2, 2 and 3 for spies that are called once. They are corrected in this commit rather than a separate one because the old and new counts cannot both hold: 2/2/2/3 passes only on vitest 4, and 1 passes only on vitest 5. Verified locally: 92 test files and 969 tests pass, tsc build and eslint clean.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Vitest 5 declares a Node engine floor of ^22.12.0 in the lockfile while the repo (and release workflow) still target Node 20, creating a high risk of CI/release installation or runtime incompatibilities.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR upgrades the project’s test runner stack to Vitest 5 (including the tightly-coupled @vitest/coverage-v8 peer) and updates a handful of unit-test assertions in webserver.test.ts that were relying on cross-test spy call accumulation.
Changes:
- Bump
vitestand@vitest/coverage-v8devDependencies to^5.0.0. - Update
package-lock.jsonto reflect the Vitest 5 dependency graph. - Fix four
toHaveBeenCalledTimes(...)assertions insrc/server/webserver.test.tsto match per-test spy isolation in Vitest 5.
File summaries
| File | Description |
|---|---|
src/server/webserver.test.ts |
Adjusts assertions to match Vitest 5 spy behavior (no cross-test leakage). |
package.json |
Updates Vitest + coverage devDependencies to 5.0.0. |
package-lock.json |
Locks the updated dependency tree for Vitest 5 and its new transitive deps. |
Review details
- Files reviewed: 2/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 8, 2026
madhavilosetty-intel
added a commit
that referenced
this pull request
Sep 8, 2026
The test matrix has run only 22.x for a while, and many devDependencies already require node >=22 -- npm ci on Node 20 emits EBADENGINE warnings for 47 packages on main today. Keeping release and semantic on 20.x meant those two workflows were the only ones still on an unsupported version. Moves release.yml and semantic.yml to 22.x, raises engines.node to >=22, and adds 24.x to the test matrix. Verified locally on Node v24.9.0: npm ci installs with no EBADENGINE warnings, 92 test files and 969 tests pass, and the tsc build is clean -- on both current main (vitest 4) and the vitest 5 branch in #2654. node.js.yml still has two steps gated on matrix.node-version == '20.x' (ci-prettify and the codecov upload) that have therefore never run. Left as-is here because re-enabling ci-prettify fails on 10 drifted files; separate PR.
rsdmike
approved these changes
Sep 8, 2026
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.
Bumps
vitestand@vitest/coverage-v8to 5.0.0 together, and corrects four test assertions that the upgrade exposes.Why both packages
@vitest/coverage-v8@5.0.0declares an exact peer onvitest@5.0.0. Bumping only the coverage package (as #2652 does) failsnpm ci:That single error is what fails all four checks on #2652 — both docker
buildjobs,build (22.x), andValidate PR and Commits(commitlint itself passes there; the job dies at the followingnpm installstep).Why the test change is in the same commit
Vitest 5 no longer leaks spy state between tests. Four assertions in
src/server/webserver.test.tswere counting calls accumulated from earlier tests rather than their own — each creates a fresh spy, calls the method once, then asserts 2, 2, 2 or 3.These cannot be split into a separate PR:
2/2/2/3passes only on vitest 4, and1passes only on vitest 5. Either half alone turnsmainred, so they land atomically.Verification
npm test— 92 test files, 969 tests pass (was 4 failing)npm run build(tsc) — cleannpm run lint— 0 errorsnpx prettier --checkon changed files — clean