Migrate from Karma/Mocha/Chai to Vitest with Playwright browser mode #145
Conversation
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
Co-authored-by: dgreif <[email protected]>
Co-authored-by: dgreif <[email protected]>
Copilot
AI
changed the title
[WIP] Convert test framework to Vitest with Playwright support
Migrate from Karma/Mocha/Chai to Vitest with Playwright browser mode
Oct 7, 2025
Copilot finished work on behalf of
dgreif
October 7, 2025 14:25
dgreif
reviewed
Oct 7, 2025
.github/workflows/nodejs.yml
Outdated
Comment on lines
10
to
12
| strategy: | ||
| matrix: | ||
| node-version: [22, 24] |
There was a problem hiding this comment.
@copilot remove this matrix and just use 24
There was a problem hiding this comment.
Done in 8b0f8ca. Removed the matrix strategy and now using Node 24 only.
.github/workflows/nodejs.yml
Outdated
Comment on lines
19
to
20
| - name: Install Playwright browsers | ||
| run: npx playwright install --with-deps chromium |
There was a problem hiding this comment.
@copilot I don't think we need to install these deps. Remove npx playwrite install from all workflows
There was a problem hiding this comment.
Done in 8b0f8ca. Removed npx playwright install from nodejs.yml and publish.yml. The vitest config uses system chromium at /usr/bin/chromium, so no Playwright installation is needed.
Co-authored-by: dgreif <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot finished work on behalf of
dgreif
October 7, 2025 14:33
dgreif
approved these changes
Oct 7, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the test infrastructure by migrating from Karma/Mocha/Chai to Vitest 3.2.4 with Playwright browser mode, providing faster execution and better developer experience with native TypeScript support.
Key changes:
- Replaced Karma/Mocha/Chai testing stack with Vitest and Playwright browser mode
- Updated all test files to use Vitest APIs and import directly from TypeScript source files
- Fixed TypeScript type exports to use proper
export typesyntax
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.js | New Vitest configuration with Playwright browser mode and system Chromium |
| test/test.js | Updated all assertions from Chai to Vitest expect API and async test patterns |
| test/test-radix-trie.js | Converted Chai assertions to Vitest expect syntax |
| test/test-normalize-hotkey.js | Migrated from Chai assertions to Vitest expect API |
| test/.eslintrc.json | Removed Mocha environment and global assert configuration |
| src/index.ts | Fixed type exports to use proper export type syntax |
| package.json | Updated dependencies and test scripts for Vitest |
| karma.config.cjs | Removed Karma configuration file |
| .github/workflows/*.yml | Updated Node.js version to 24 in all workflows |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
vitest.config.js
Outdated
| { | ||
| browser: 'chromium', | ||
| launch: { | ||
| executablePath: '/usr/bin/chromium' |
There was a problem hiding this comment.
Hard-coded executable path '/usr/bin/chromium' may not exist on all systems. Consider using Playwright's default browser installation or making this configurable through environment variables.
Suggested change
| executablePath: '/usr/bin/chromium' | |
| ...(process.env.CHROMIUM_EXECUTABLE_PATH ? { executablePath: process.env.CHROMIUM_EXECUTABLE_PATH } : {}) |
Comment on lines
+297
to
306
| it(`${JSON.stringify(keyEvent)} => ${expected}`, () => { | ||
| return new Promise(resolve => { | ||
| document.body.addEventListener('keydown', function handler(event) { | ||
| document.body.removeEventListener('keydown', handler) | ||
| expect(eventToHotkeyString(event, platform)).toBe(expected) | ||
| resolve() | ||
| }) | ||
| document.body.dispatchEvent(new KeyboardEvent('keydown', keyEvent)) | ||
| }) | ||
| document.body.dispatchEvent(new KeyboardEvent('keydown', keyEvent)) | ||
| }) |
There was a problem hiding this comment.
[nitpick] This test pattern can be simplified using async/await instead of manually creating a Promise. Consider using a helper function or converting to async/await for better readability.
bteng22
approved these changes
Oct 7, 2025
vitest.config.js
Outdated
| { | ||
| browser: 'chromium', | ||
| launch: { | ||
| executablePath: '/usr/bin/chromium' |
There was a problem hiding this comment.
Why did this config need the executablePath for chromium here?
There was a problem hiding this comment.
@copilot can we remove this? Or is it required for CI?
There was a problem hiding this comment.
The executablePath is required for CI to work without installing Playwright browsers. Without it, Playwright tries to use its managed browsers (which need to be installed via npx playwright install). By pointing to the system chromium at /usr/bin/chromium, we can run tests without that extra installation step in the workflows.
There was a problem hiding this comment.
Ok, I was able to remove this by adding npx playwright install chromium in ci
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot finished work on behalf of
dgreif
October 7, 2025 15:33
bteng22
approved these changes
Oct 7, 2025
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.
This PR modernizes the test infrastructure by migrating from Karma/Mocha/Chai to Vitest 3.2.4 with Playwright browser mode.
Motivation
The existing Karma-based test setup required building the TypeScript code before running tests and used older testing libraries. Vitest provides a more modern testing experience with native TypeScript support, faster execution, and better developer experience.
Changes
Test Framework
expectAPITest Files
Updated all test files to use Vitest:
describe,it,expect,beforeEach,afterEachfromvitestsrc/*.ts) instead of compiled output (dist/*.js)donecallbacks in favor of returning promises or using async/awaitConfiguration
vitest.config.jswith browser mode configuration/usr/bin/chromium) with headless modetest/test*.js)Dependencies
vitest@^3.2.4,@vitest/browser@^3.2.4,playwrightkarma,karma-chai,karma-chrome-launcher,karma-mocha,karma-mocha-reporter,mocha,chai,chromiumkarma.config.cjsPackage Scripts
testscript to runnpm run lint && vitest runpretestscript - no longer need to build before testing since Vitest runs TypeScript directlyESLint Configuration
mocha: trueenvironment fromtest/.eslintrc.jsonassert: trueglobal since we now importexpectexplicitlyGitHub Workflows
actions/checkout@v4andactions/setup-node@v4TypeScript Exports
Fixed type exports in
src/index.tsto useexport typesyntax forNormalizedHotkeyStringandNormalizedSequenceStringto ensure proper runtime behavior when importing from TypeScript source files.Testing
All 98 tests pass successfully:
Lint checks pass with no errors. Build process continues to work correctly.
Benefits
.tsfilesOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.