Upgrade Vitest to 5 and cut the frontend suite from 42s to 10s - #347
Merged
Merged
Conversation
- Vitest 5 enables clearMocks by default, so remove 128 vi.clearAllMocks() calls, 13 hooks they left empty, and 2 mockClear() calls in beforeEach - Keep the two mid-test mockClear() calls, which still do real work - Verified the automatic clear runs before beforeEach, so setup done in a hook survives into the test - Document the behaviour so the clears do not get reintroduced - Ignore the new .vitest artifact directory
- *.log already covers the npm, yarn, pnpm, and lerna debug log patterns - The yarn, pnpm, and lerna entries name package managers this repo does not use, and there is no Cypress here
The formatting hint told you to run --fix, but --fix also applies every linter autofix. In __tests__ the vitest plugin rewrites toEqual to toStrictEqual and lowercases describe titles, so following the tool's own advice silently changed what the tests assert. - Add --format, which runs oxfmt only and never changes behaviour - Point the formatting hints and help text at --format - Warn when --fix runs that it can rewrite code, not just layout
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #347 +/- ##
=======================================
Coverage 45.37% 45.37%
=======================================
Files 948 948
Lines 49532 49532
Branches 6700 6700
=======================================
Hits 22474 22474
- Misses 26092 26093 +1
+ Partials 966 965 -1
Flags with carried forward coverage won't be shown. Click here to find out more. |
Bundle ReportBundle size has no change ✅ |
happy-dom was rebuilt once per test file, 128 times, which cost about 60% of the run. vmThreads creates one per worker and keeps per-file isolation, taking the suite from roughly 40s to 10s. - Spread four assertions whose values cross the VM realm boundary, so toStrictEqual compares contents rather than a foreign Array prototype - Verified V8 coverage still reports the same totals under this pool - Document the realm boundary, since the failure it causes reads as "Compared values have no visual difference"
rlorenzo
force-pushed
the
chore/vitest-5-upgrade
branch
from
September 22, 2026 06:01
b3baafb to
8521998
Compare
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The Vitest v5 upgrade, vmThreads pool switch, and corresponding test/doc/lint updates are consistent and do not show any correctness regressions in the reviewed changes.
Review effort: Lite
Findings: None
What changed in this PR
Upgrades the Vue frontend test runner to Vitest v5 and applies configuration/doc/test cleanups that take advantage of v5 defaults and a faster pool implementation to significantly reduce unit test runtime.
Changes:
- Upgrade Vitest + coverage provider to v5.
- Switch Vitest to the
vmThreadspool (one happy-dom per worker) and document the cross-realm assertion caveat. - Remove redundant mock-clearing calls in frontend tests and refine
scripts/lint-any.jsto support--format(oxfmt-only writes) vs--fix(includes linter autofixes).
| File | Description |
|---|---|
| VueApp/vite.config.ts | Sets Vitest pool: "vmThreads" for faster runs while keeping happy-dom. |
| VueApp/src/Students/EmergencyContact/__tests__/use-emergency-contact.test.ts | Removes redundant vi.clearAllMocks() setup blocks. |
| VueApp/src/Students/EmergencyContact/__tests__/emergency-contact-service.test.ts | Removes redundant vi.clearAllMocks() setup blocks. |
| VueApp/src/Students/__tests__/photo-gallery-teams-filter.test.ts | Removes redundant vi.clearAllMocks() from test setup. |
| VueApp/src/Students/__tests__/photo-gallery-store.test.ts | Removes redundant vi.clearAllMocks() from test setup. |
| VueApp/src/Personnel/__tests__/use-add-record-dialog.test.ts | Drops per-test vi.clearAllMocks() calls now covered by Vitest v5 defaults. |
| VueApp/src/Personnel/__tests__/svm-unit-service.test.ts | Drops per-test vi.clearAllMocks() calls. |
| VueApp/src/Personnel/__tests__/svm-section-service.test.ts | Drops per-test vi.clearAllMocks() calls. |
| VueApp/src/Personnel/__tests__/svm-phones.test.ts | Removes redundant mock clearing inside helper setup. |
| VueApp/src/Personnel/__tests__/svm-phones-maintain.test.ts | Removes redundant vi.clearAllMocks() from helper reset. |
| VueApp/src/Personnel/__tests__/svm-modified-date-service.test.ts | Drops per-test vi.clearAllMocks() calls. |
| VueApp/src/Personnel/__tests__/svm-frequent-number-service.test.ts | Drops per-test vi.clearAllMocks() calls. |
| VueApp/src/Personnel/__tests__/svm-data-fetch.test.ts | Removes repeated vi.clearAllMocks() calls in setup across many specs. |
| VueApp/src/Personnel/__tests__/svm-add-record-dialog.test.ts | Removes redundant mock clearing from test reset helper. |
| VueApp/src/Personnel/__tests__/svm-add-frequent-number-dialog.test.ts | Removes redundant mock clearing from test reset helper. |
| VueApp/src/Personnel/__tests__/router-permissions.test.ts | Removes redundant mock clearing from router factory helper. |
| VueApp/src/Personnel/__tests__/phone-person-options-service.test.ts | Drops per-test vi.clearAllMocks() calls. |
| VueApp/src/Personnel/__tests__/phone-list.test.ts | Removes redundant mock clearing from helper/setup. |
| VueApp/src/Personnel/__tests__/phone-list-unit-service.test.ts | Drops per-test vi.clearAllMocks() calls. |
| VueApp/src/Personnel/__tests__/phone-list-service.test.ts | Drops per-test vi.clearAllMocks() calls. |
| VueApp/src/Personnel/__tests__/phone-list-route-changes.test.ts | Removes redundant mock clearing in route-change specs. |
| VueApp/src/Personnel/__tests__/phone-list-modified-date-service.test.ts | Drops per-test vi.clearAllMocks() calls. |
| VueApp/src/Personnel/__tests__/phone-list-maintain.test.ts | Removes redundant mock clearing throughout maintain-page specs. |
| VueApp/src/Personnel/__tests__/phone-list-data-fetch.test.ts | Removes redundant mock clearing from data-fetch specs. |
| VueApp/src/Personnel/__tests__/phone-list-add-record-dialog.test.ts | Removes redundant mock clearing from test reset helper. |
| VueApp/src/Personnel/__tests__/person-selector.test.ts | Drops per-test vi.clearAllMocks() calls. |
| VueApp/src/Effort/__tests__/use-report-url-params.test.ts | Removes manual mockClear() now covered by v5 mock clearing. |
| VueApp/src/Effort/__tests__/use-effort-permissions.test.ts | Drops redundant vi.clearAllMocks() calls. |
| VueApp/src/Effort/__tests__/report-service.test.ts | Removes redundant vi.clearAllMocks() setup block. |
| VueApp/src/Effort/__tests__/instructor-service.test.ts | Removes redundant vi.clearAllMocks() setup blocks across describe groups. |
| VueApp/src/Effort/__tests__/instructor-edit-dialog.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/Effort/__tests__/instructor-add-dialog.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/Effort/__tests__/harvest-dialog.test.ts | Removes redundant vi.clearAllMocks() from setup blocks. |
| VueApp/src/Effort/__tests__/dashboard-service.test.ts | Removes redundant vi.clearAllMocks() setup block. |
| VueApp/src/Effort/__tests__/course-service.test.ts | Removes redundant vi.clearAllMocks() setup blocks. |
| VueApp/src/Effort/__tests__/course-link-dialog.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/Effort/__tests__/course-import-dialog.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/Effort/__tests__/course-edit-dialog.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/Effort/__tests__/course-add-dialog.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/CMS/__tests__/inline-file-upload.test.ts | Spreads FormData.getAll() results to keep toStrictEqual under vmThreads realms. |
| VueApp/src/CMS/__tests__/content-block-edit-image-upload.test.ts | Spreads cross-realm arrays (FormData + props) before toStrictEqual. |
| VueApp/src/CMS/__tests__/cms-home.test.ts | Removes manual mock clearing and relies on v5 clearing + per-test mock implementation reset. |
| VueApp/src/ClinicalScheduler/__tests__/use-optimistic-schedule-updates.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/ClinicalScheduler/__tests__/use-bulk-deletion.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/ClinicalScheduler/__tests__/test-utils.ts | Removes redundant vi.clearAllMocks() from shared setup helper. |
| VueApp/src/ClinicalScheduler/__tests__/rotation-selector.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/ClinicalScheduler/__tests__/rotation-selector-clinician-reactivity.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/ClinicalScheduler/__tests__/rotation-selector-api.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/ClinicalScheduler/__tests__/permissions-store-utilities.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/ClinicalScheduler/__tests__/permissions-store-error-handling.test.ts | Removes redundant vi.clearAllMocks() from setup. |
| VueApp/src/ClinicalScheduler/__tests__/permissions-store-computed.test.ts | Removes redundant vi.clearAllMocks() from setup blocks. |
| VueApp/src/ClinicalScheduler/__tests__/clinician-selector-helpers.ts | Removes redundant vi.clearAllMocks() from helper setup. |
| VueApp/README.md | Adds Vitest usage notes, documents mock clearing + vmThreads realm caveat and workaround. |
| VueApp/package.json | Bumps vitest and @vitest/coverage-v8 to ^5.0.0. |
| VueApp/package-lock.json | Locks Vitest v5 + updated dependency graph. |
| VueApp/.gitignore | Adds .vitest cache directory to ignores and trims redundant log patterns. |
| scripts/lint-any.js | Adds --format flag and adjusts messaging to avoid encouraging behavior-changing autofixes. |
| CLAUDE.md | Records repo-wide guidance on Vitest v5 mock clearing and vmThreads realm behavior. |
Files not reviewed (1)
- VueApp/package-lock.json: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bsedwards
approved these changes
Sep 25, 2026
bniedzie
approved these changes
Sep 25, 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.
Vitest 4.1.11 to 5.0.0, plus the mock-clearing code that upgrade makes dead, plus a pool change that takes the frontend suite from 42s to 10s.
The migration itself is a non-event. No
test.projects, no browser mode, no benchmarks, so none of v5's headline features land here. What did help is a diagnostic v5 added, which pointed at a bottleneck that had been sitting in the suite the whole time:One happy-dom per test file, 128 of them.
vmThreadsbuilds one per worker instead and still isolates per file.Warmed, three runs each, same machine, one sitting:
The lint change
Formatting this branch walked me into a trap. On a formatting failure the linter printed
Run with --fix to auto-format, and doing what it said rewrote test semantics:.oxlintrc.jsonturns on the vitest plugin for**/__tests__/**, whereprefer-strict-equalswapstoEqualfortoStrictEqualandprefer-lowercase-titlerenames describe blocks. I caught it and reverted, but the tool was aiming people straight at it.--formatnow runs oxfmt and nothing else, and the hints point there.Worth knowing while reviewing
vmThreadsa value built outside the test realm carries a foreignArrayprototype, sotoStrictEqualfails with "Compared values have no visual difference" even though contents match. I probed it: the array crosses the realm, the objects inside do not, so spreading is enough andtoStrictEqualstays. Line 201 of the same file already did this.vmThreads, since it historically could not see into VM contexts. It reports 65.84% against 65.79% on the old pool, and both CI artifacts still land where the workflow looks.beforeEach, so hook setup survives. I verified that rather than assuming it..npmrcsetsmin-release-age=7. UI fixes and edge cases we do not touch.