chore: migrate test suite from Mocha/Chai to Vitest#1257
chore: migrate test suite from Mocha/Chai to Vitest#1257MarshallOfSound merged 1 commit intomainfrom
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|
@SocketSecurity ignore npm/vite@5.4.21 |
c262511 to
c2f1139
Compare
23745e1 to
f7574b7
Compare
dsanders11
left a comment
There was a problem hiding this comment.
Initial review has a couple of small comments, will come back to this later today.
| // Test files share a single TEST_MODULE_PATH on disk, so they cannot run | ||
| // in parallel without clobbering each other's fixtures. | ||
| fileParallelism: false, |
There was a problem hiding this comment.
Follow-up for a different PR, but this would probably be easy enough to change if it gave us a speed up on how long CI takes, since it's 5-10 mins.
There was a problem hiding this comment.
Agreed, follow-up. The blocker is TEST_MODULE_PATH being a single shared path on disk (os.tmpdir()/electron-rebuild-test), so any two test files that share it would clobber each other's fixtures if they ran concurrently. Making it per-file (e.g., os.tmpdir()/electron-rebuild-test-${path.basename(import.meta.filename)}) would unlock parallelism.
Generated by Claude Code
vitest.config.ts
Outdated
| test: { | ||
| include: ['test/*.ts'], | ||
| reporters: process.env.GITHUB_ACTIONS ? ['default', 'github-actions'] : 'default', | ||
| testTimeout: 10 * 60 * 1000, |
There was a problem hiding this comment.
We had more fine-grained timeouts before, now everything is 10 minutes.
There was a problem hiding this comment.
Restored. The describe-level { timeout: ... } option in vitest only applies to tests, not to beforeAll/afterAll, so the per-suite timeouts are now passed directly on each describe and the global hookTimeout is set to TIMEOUT_IN_MILLISECONDS (mirroring what mocha's this.timeout() did at the suite level for hooks). The two macOS/Windows-specific 5-minute test overrides also got restored as a third arg on the relevant it() calls, and the 10-minute debug/clang suites are explicit again.
Generated by Claude Code
test/helpers/module-setup.ts
Outdated
| process.execArgv = process.execArgv.filter((arg, i, all) => { | ||
| if (arg === '--experimental-import-meta-resolve') return false; | ||
| if (arg === '--require' && all[i + 1]?.includes('vitest')) return false; | ||
| if (i > 0 && all[i - 1] === '--require' && arg.includes('vitest')) return false; | ||
| if (arg === '--conditions' && all[i + 1] === 'development') return false; | ||
| if (i > 0 && all[i - 1] === '--conditions' && arg === 'development') return false; | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
This might prove to be fragile over time. 🤔
There was a problem hiding this comment.
Agreed. Simplified to just process.execArgv = [] with a comment explaining the forked node-gyp worker doesn't need any of vitest's worker flags. That's much less coupled to vitest's internal flag list than the earlier per-flag filter.
Generated by Claude Code
Replace mocha + chai + chai-as-promised + nyc with vitest + @vitest/coverage-v8. - Replace describe/it/before/after assertions with vitest equivalents - Replace chai matchers (to.equal, to.deep.equal, to.be.true, etc.) with vitest matchers (toBe, toEqual, etc.) - Replace 'context' aliases with 'describe' - Replace per-test/suite this.timeout() calls with a global testTimeout in vitest.config.ts - Externalize lib/ in vitest's SSR transformer so import.meta.resolve in lib/electron-locator.js works correctly - Disable file parallelism since test files share TEST_MODULE_PATH - Drop the now-obsolete tsx execArgv filter from module-setup.ts
f7574b7 to
822fb79
Compare
This PR migrates the entire test suite from Mocha and Chai to Vitest, a modern and faster test framework.
Summary
The test infrastructure has been completely refactored to use Vitest instead of Mocha/Chai. This includes updating test syntax, assertion methods, and configuration files to align with Vitest's API and conventions.
Key Changes
expect().to.equal()) to Vitest assertions (e.g.,expect().toBe())before,after) to Vitest style (beforeAll,afterAll,beforeEach,afterEach)this.timeout()callsvitest.config.tswith test configuration including timeout settings, coverage configuration, and file parallelism settings.mocharc.jsonc(Mocha configuration)package.jsonscripts to usevitest runinstead ofmochamocha,chai,chai-as-promised,nyc,@types/mocha,@types/chai,@types/chai-as-promised,eslint-plugin-mochavitest,@vitest/coverage-v8execArgvfiltering for tsx).eslintrc.jstsconfig.jsonto remove Mocha typesNotable Implementation Details
vitest.config.ts(10 minutes) rather than per-testfileParallelism: false) since test files share a singleTEST_MODULE_PATHfixture on disk.toBe(),.toContain(),.toEqual(),.toHaveLength(),.rejects.toThrow())https://preview.claude.ai/code/session_01QHrqXbnQUtPxuynmvQsJrz