Repository navigation
test: replace mocha with the Node.js built-in test runner - #1042
Merged
Merged
Conversation
Test files only used `describe`/`it`/`beforeEach` and `node:assert`, so mocha can be dropped without touching any test code. `test/setup.mjs` exposes the `node:test` API as globals, which also keeps ESLint's `RuleTester` working since it looks up global `describe`/`it`. The timeout is raised from 5s to 10s because `node --test` runs test files in parallel, unlike mocha's serial execution.
There was a problem hiding this comment.
Pull request overview
This PR migrates the repo’s test execution from mocha to Node.js’s built-in test runner (node --test) while keeping the existing test files unchanged by injecting node:test helpers (describe/it/hooks) as globals.
Changes:
- Added
test/setup.mjsto exposenode:testAPIs onglobalThisvia--import. - Updated
package.jsonto run tests withnode --test(and removedmochafrom devDependencies). - Updated ESLint globals to remove
globals.mochaand declare the injected test globals explicitly; lockfile updated accordingly.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/setup.mjs | Adds a preload module that injects node:test suite/test/hook functions as globals for existing tests and RuleTester. |
| package.json | Switches the test script to node --test with preload and increases timeout; removes mocha dependency. |
| eslint.config.mjs | Replaces globals.mocha with explicit describe/it/hook global declarations to match the new test setup. |
| pnpm-lock.yaml | Removes mocha and its transitive dependencies from the lockfile. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+14
to
+20
| // Injected by test/setup.mjs (see the `test` npm script) | ||
| describe: "readonly", | ||
| it: "readonly", | ||
| before: "readonly", | ||
| after: "readonly", | ||
| beforeEach: "readonly", | ||
| afterEach: "readonly", |
`describe`/`it`/hooks only exist while running `node --test --import ./test/setup.mjs`, so declaring them for every file misrepresents the setup. Move them into a `files: ["test/**"]` block.
nus3
reviewed
Sep 3, 2026
| */ | ||
| import { describe, it, before, after, beforeEach, afterEach } from "node:test"; | ||
|
|
||
| Object.assign(globalThis, { |
Contributor
There was a problem hiding this comment.
Looking at
eslint-config/test/es5-test.js
Line 1 in 1ae90da
assert is coming from the node:assert module there as well, so it might be nice to be consistent about this — either importing the module in each test file, or assigning them all at once here.
Contributor
Author
There was a problem hiding this comment.
Agreed — switched to explicit imports in b2c08d2. Each test file now imports describe/it from node:test (same as assert), and setup.mjs only wires RuleTester to node:test via its public API, so no globals are injected anymore.
Test files already import `assert` from a module, so import `describe`/`it` from `node:test` the same way instead of relying on globals injected by `test/setup.mjs`. The setup module now only wires ESLint's `RuleTester` to `node:test` via its public `describe`/`it` static properties, since `RuleTester` resolves them on its own and cannot see imports made in test files. This also removes the need for any test-specific globals in the ESLint config.
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.
Summary
Replaces
mochawith the Node.js built-in test runner (node --test).The existing tests only used
describe/it/beforeEachandnode:assert, so mocha could be dropped without changing a single line of test code.Changes
test/setup.mjs(new)node:testAPI as globalspackage.jsontestscript →node --test --test-timeout=10000 --import ./test/setup.mjs "test/**/*-test.js",mocharemoved from devDependencieseslint.config.mjsglobals.mochareplaced with explicit declarations of the six globals thattest/setup.mjsinjectspnpm-lock.yamlWhy the global injection
ESLint's
RuleTester(used bytest/plugin/react/rules/*-test.js) looks up the globaldescribe/itand falls back to a synchronous handler when they are missing. Withnode:testthese are module exports rather than globals, so without the injection the rule tests would run but report no test count. Assigning them toglobalThisvia--importkeepsRuleTesterreporting each case individually and requires no changes to the test files.Why the timeout was raised from 5s to 10s
node --testruns test files in parallel, unlike mocha's serial execution, so the wall-clock time of an individual test increases under CPU contention. With the original 5s limit one test was actually cancelled.Verification
pnpm test→ 208 tests / 69 suites / 208 pass / 0 fail / 0 cancelledpnpm lint→ no errorsNo workflow changes are needed: CI and the release workflow both just call
pnpm test.