Replace Mocha with Vitest across all packages#429
Merged
Conversation
- Add vitest 3.1.1 and @vitest/coverage-v8 to root devDependencies - Remove mocha from root devDependencies - Create shared vitest.config.ts with 90% coverage thresholds - Update all package test scripts from mocha+c8 to vitest - Remove c8 from all package devDependencies - Update tsconfig to remove mocha types
Local vitest.config.ts overrides for 6 packages: - express-test: setupFiles for snapshot test registry - http-cache-utils, webhook-mock-receiver: no coverage enforcement (matches mocha) - errors, prometheus-metrics: coverage scoped to src/ - job-manager: ignore unhandled rejections from bree workers ESLint fixes for beforeAll/afterAll globals in 5 test configs, and mocha lint rule overrides in express-test setup file.
- Convert 87 done() callback patterns to async/await with Promises - Replace before/after with beforeAll/afterAll (5 files) - Fix runner-specific test assumptions (root-utils, jest-snapshot, metrics)
- root-utils: mock caller in test (vitest call stack differs from mocha) - jest-snapshot: use __filename instead of mocha this.test.file - job-manager: use assert.rejects for async job validation - Update MIGRATION.md to document completed vitest migration
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
WalkthroughThis PR executes a comprehensive migration of the test infrastructure from Mocha with C8 coverage to Vitest across 40+ packages. The changes include: replacing test runner scripts in package.json files to use Vitest instead of Mocha/C8, removing C8 dependencies, refactoring test files from callback-based asynchronous patterns (using Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Document the reason for each package-level vitest.config.ts override directly in the config file.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #429 +/- ##
===========================================
- Coverage 100.00% 99.55% -0.45%
===========================================
Files 133 133
Lines 8648 7177 -1471
Branches 1466 1394 -72
===========================================
- Hits 8648 7145 -1503
- Misses 0 32 +32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Use mergeConfig to extend the shared root vitest.config.ts instead of duplicating the entire configuration. Each override now only specifies what differs from the root.
http-cache-utils, webhook-mock-receiver, and express-test all have 100% coverage — no need for lowered thresholds. Remove standalone configs for the first two (use root config), and simplify express-test to only override setupFiles.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/prometheus-metrics/package.json (1)
36-36:⚠️ Potential issue | 🟡 MinorLeftover
mochadevDependency.The test script now uses Vitest, but
mocharemains in devDependencies. This appears to be an oversight—unless mocha is still needed for another purpose, it should be removed to keep dependencies clean.Suggested fix
"devDependencies": { "@types/express": "5.0.6", "@types/sinon": "21.0.0", "@types/stoppable": "1.1.3", "@types/supertest": "7.2.0", "knex": "3.1.0", - "mocha": "11.7.5", "nock": "14.0.11",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/prometheus-metrics/package.json` at line 36, Remove the leftover "mocha" devDependency from package.json's devDependencies section since tests now run with Vitest; update the lockfile (npm/yarn/pnpm) afterwards and run the test script (or npm ci) to verify nothing else depends on the "mocha" entry.packages/domain-events/package.json (1)
25-25:⚠️ Potential issue | 🟡 MinorLeftover
mochadevDependency.
mochacan be removed since tests now run via Vitest.Suggested fix
"devDependencies": { "@tryghost/logging": "^4.0.0", - "mocha": "11.7.5" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/domain-events/package.json` at line 25, Remove the leftover devDependency "mocha" from package.json's devDependencies block (the entry `"mocha": "11.7.5"`), since tests now run with Vitest; update package.json by deleting that key/value pair and run npm/yarn install to update lockfile and node_modules accordingly.packages/elasticsearch/package.json (1)
26-26:⚠️ Potential issue | 🟡 MinorLeftover
mochadevDependency.
mochacan be removed since tests now run via Vitest.Suggested fix
"devDependencies": { - "mocha": "11.7.5", "sinon": "21.0.1" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/elasticsearch/package.json` at line 26, Remove the leftover mocha devDependency from the package.json by deleting the "mocha": "11.7.5" entry in the devDependencies block; ensure package.json remains valid JSON (no trailing commas) and run the package manager's install/prune (e.g., npm/yarn/pnpm install) to update lockfile and node_modules so tests run solely via Vitest.packages/bookshelf-order/package.json (1)
26-26:⚠️ Potential issue | 🟡 MinorLeftover
mochadevDependency.Same as other packages—
mochacan be removed since tests now run via Vitest.Suggested fix
"devDependencies": { - "mocha": "11.7.5", "sinon": "21.0.1" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bookshelf-order/package.json` at line 26, Remove the leftover "mocha": "11.7.5" entry from the package's devDependencies in package.json (search for the "mocha" string under devDependencies) so tests rely on Vitest; also scan the package.json "scripts" for any mocha-specific commands and replace or remove them if present, then reinstall/update the lockfile (npm/yarn/pnpm) to persist the change.packages/http-stream/package.json (1)
29-29:⚠️ Potential issue | 🟡 MinorLeftover
mochadevDependency.
mochacan be removed since tests now run via Vitest.Suggested fix
"devDependencies": { "express": "5.2.1", - "mocha": "11.7.5", "sinon": "21.0.1" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/http-stream/package.json` at line 29, The package.json contains a leftover devDependency "mocha" that is no longer used (tests run with Vitest); remove the "mocha": "11.7.5" entry from the devDependencies section in packages/http-stream/package.json, update the lockfile by running your package manager (npm/yarn/pnpm install) to ensure the dependency is removed from node_modules and lockfile, and verify tests still run with the existing Vitest configuration (no code changes required).packages/mw-error-handler/test/mw-error-handler.test.js (1)
345-361:⚠️ Potential issue | 🟡 MinorTest title status code conflicts with assertion.
Line 345 says this case is a 406 response, but Line 360 asserts
400. Please align the test name with the asserted behavior to avoid confusion.Suggested fix
- it('Returns 406 Request Not Acceptable Error for invalid version', async function () { + it('Returns 400 Bad Request Error for invalid version', async function () {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mw-error-handler/test/mw-error-handler.test.js` around lines 345 - 361, The test description "Returns 406 Request Not Acceptable Error for invalid version" is inconsistent with the assertions expecting statusCode 400; update the test to make the behavior explicit and consistent: either change the it(...) description to say 400 (e.g., "Returns 400 Bad Request for invalid version") to match the assertion, or change the asserted statusCode and message in the callback of resourceNotFound(...) to 406 and the appropriate message; ensure the test title, the assertion on error.statusCode and error.message, and the resourceNotFound callback are all aligned.packages/database-info/package.json (1)
14-27:⚠️ Potential issue | 🟠 MajorRemove unused
mochadependency—Vitest handles the test syntax natively.The test script uses Vitest (
vitest run --coverage), and the test file'sdescribe/itsyntax is fully compatible with Vitest without requiring Mocha as a dependency. Mocha is unused and should be removed fromdevDependenciesto complete the migration.Proposed cleanup
"devDependencies": { - "knex": "3.1.0", - "mocha": "11.7.5" + "knex": "3.1.0" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/database-info/package.json` around lines 14 - 27, Remove the unused "mocha" entry from package.json's "devDependencies" because tests use the "test" script with Vitest ("vitest run --coverage --config ../../vitest.config.ts"); update the "devDependencies" block to delete the "mocha": "11.7.5" line and then run your package manager to refresh lockfiles/install so the dependency is fully removed.
🧹 Nitpick comments (4)
packages/pretty-stream/test/PrettyStream.test.js (1)
12-468: Consider extracting a shared async harness for stream output assertions.The same setup/assert/resolve flow is duplicated across most tests. A helper (e.g.,
expectPrettyOutput({mode, payload, expected})) would reduce maintenance risk and keep migrations simpler.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/pretty-stream/test/PrettyStream.test.js` around lines 12 - 468, Tests duplicate stream setup/assert/resolve logic; extract a reusable async helper (e.g., expectPrettyOutput) that creates a PrettyStream({mode}), a Writable with _write that converts data to string, performs the expected assertion (exact match or includes), and resolves the Promise; the helper should accept payload (object or string) and stringify objects before calling ghostPrettyStream.write, accept an expected string or predicate, and return a Promise so existing tests can be updated to call await expectPrettyOutput({mode:'short', payload: {...}, expected: '...'}); locate uses of PrettyStream and the many it(...) blocks to replace duplicated setup with calls to expectPrettyOutput.packages/promise/package.json (1)
25-28: Remove unusedmochadevDependency.The test script now uses Vitest, but
mocharemains indevDependencies.Proposed fix
"devDependencies": { - "mocha": "11.7.5", "sinon": "21.0.1" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/promise/package.json` around lines 25 - 28, The package.json still lists "mocha" under devDependencies even though tests use Vitest; remove the unused entry by deleting the "mocha": "11.7.5" key/value from the devDependencies object in packages/promise/package.json so only relevant test tooling (e.g., vitest) remains; ensure the package.json stays valid JSON (no trailing commas) after removing the "mocha" entry and run a quick install/check to confirm no other references to mocha exist.packages/bookshelf-plugins/package.json (1)
25-28: Remove unusedmochadevDependency.The test script now uses Vitest (line 14), but
mocharemains indevDependencies. This leftover dependency should be removed to keep the package clean and avoid confusion.Proposed fix
"devDependencies": { - "mocha": "11.7.5", "sinon": "21.0.1" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bookshelf-plugins/package.json` around lines 25 - 28, package.json currently lists "mocha" under "devDependencies" even though the package uses Vitest for testing; remove the unused "mocha" entry from the "devDependencies" object so only active test tooling (Vitest) remains, ensuring the "devDependencies" block is still valid JSON and no other scripts or files reference "mocha".packages/email-mock-receiver/package.json (1)
27-30: Remove unusedmochadevDependency.Same issue as other packages: the test script now uses Vitest, but
mocharemains indevDependencies.Proposed fix
"devDependencies": { - "mocha": "11.7.5", "sinon": "21.0.1" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email-mock-receiver/package.json` around lines 27 - 30, Remove the unused "mocha" devDependency from package.json: open the package.json for the email-mock-receiver package, delete the "mocha": "11.7.5" entry under "devDependencies" (leaving other entries like "sinon" intact), and ensure the test script refers to Vitest (so no leftover mocha references remain).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api-framework/test/http.test.js`:
- Around line 84-99: The test currently resolves the promise when next() is
called which can mask failures; change the test so next does not resolve the
promise but instead fails the test if invoked (e.g., make next.callsFake(() => {
throw new Error("next called unexpectedly"); }) or reject the promise), and only
resolve the outer promise from the res.json.callsFake handler; keep references
to the same symbols (shared.http, apiImpl, next, res.json, req, res) and ensure
apiImpl is still stubbed to resolve('data') and shared.http(apiImpl)(req, res,
next) is invoked so the promise is only fulfilled in res.json and any unexpected
next() call fails the test.
In `@packages/job-manager/vitest.config.ts`:
- Around line 7-10: Replace the global suppression
(test.dangerouslyIgnoreUnhandledErrors) with a targeted onUnhandledError
handler: remove dangerouslyIgnoreUnhandledErrors and add
test.onUnhandledError(error) that checks the error message/stack for Bree
cleanup-specific patterns (e.g., worker/cleanup text or known error signature)
and returns false to ignore those Bree errors but returns true (or lets Vitest
handle) for all other errors; update the mergeConfig/defineConfig block to
register this handler so only Bree cleanup noise is filtered while genuine
unhandled errors still surface.
In `@packages/metrics/test/metrics.test.js`:
- Around line 16-20: The afterEach cleanup currently only restores
process.env.MODE when originalMode is defined, which leaks state when MODE
started as undefined; update the afterEach callback (the one referencing
originalMode and process.env.MODE) to delete process.env.MODE when originalMode
=== undefined and otherwise restore process.env.MODE = originalMode so the
environment is returned to its original baseline in all cases.
In `@packages/pretty-stream/test/PrettyStream.test.js`:
- Around line 13-30: The tests use a custom Writable._write with an incorrect
signature and lack error paths, causing hangs; update each instance (e.g., in
PrettyStream.test.js where ghostPrettyStream and writeStream are created) to
implement writeStream._write(chunk, encoding, callback), wrap the assertion in
try { ... callback(); } catch (err) { callback(err); } and ensure the Promise
returned is constructed with (resolve, reject) and both ghostPrettyStream and
writeStream have 'error' listeners that call reject(err); also ensure any places
that previously resolved on success call resolve() only after callback()
succeeds so failures propagate properly.
In `@packages/root-utils/test/root-utils.test.js`:
- Around line 28-37: The test helper loadRootUtilsWithMocks patches Module._load
before requiring '../lib/root-utils' but does not guarantee restoration if
require throws; update loadRootUtilsWithMocks so Module._load is restored in a
finally block (i.e., assign the original loader back inside finally) around the
require('../lib/root-utils') call to ensure cleanup even on exceptions,
preserving test isolation for subsequent calls to getCallerRoot and other tests.
---
Outside diff comments:
In `@packages/bookshelf-order/package.json`:
- Line 26: Remove the leftover "mocha": "11.7.5" entry from the package's
devDependencies in package.json (search for the "mocha" string under
devDependencies) so tests rely on Vitest; also scan the package.json "scripts"
for any mocha-specific commands and replace or remove them if present, then
reinstall/update the lockfile (npm/yarn/pnpm) to persist the change.
In `@packages/database-info/package.json`:
- Around line 14-27: Remove the unused "mocha" entry from package.json's
"devDependencies" because tests use the "test" script with Vitest ("vitest run
--coverage --config ../../vitest.config.ts"); update the "devDependencies" block
to delete the "mocha": "11.7.5" line and then run your package manager to
refresh lockfiles/install so the dependency is fully removed.
In `@packages/domain-events/package.json`:
- Line 25: Remove the leftover devDependency "mocha" from package.json's
devDependencies block (the entry `"mocha": "11.7.5"`), since tests now run with
Vitest; update package.json by deleting that key/value pair and run npm/yarn
install to update lockfile and node_modules accordingly.
In `@packages/elasticsearch/package.json`:
- Line 26: Remove the leftover mocha devDependency from the package.json by
deleting the "mocha": "11.7.5" entry in the devDependencies block; ensure
package.json remains valid JSON (no trailing commas) and run the package
manager's install/prune (e.g., npm/yarn/pnpm install) to update lockfile and
node_modules so tests run solely via Vitest.
In `@packages/http-stream/package.json`:
- Line 29: The package.json contains a leftover devDependency "mocha" that is no
longer used (tests run with Vitest); remove the "mocha": "11.7.5" entry from the
devDependencies section in packages/http-stream/package.json, update the
lockfile by running your package manager (npm/yarn/pnpm install) to ensure the
dependency is removed from node_modules and lockfile, and verify tests still run
with the existing Vitest configuration (no code changes required).
In `@packages/mw-error-handler/test/mw-error-handler.test.js`:
- Around line 345-361: The test description "Returns 406 Request Not Acceptable
Error for invalid version" is inconsistent with the assertions expecting
statusCode 400; update the test to make the behavior explicit and consistent:
either change the it(...) description to say 400 (e.g., "Returns 400 Bad Request
for invalid version") to match the assertion, or change the asserted statusCode
and message in the callback of resourceNotFound(...) to 406 and the appropriate
message; ensure the test title, the assertion on error.statusCode and
error.message, and the resourceNotFound callback are all aligned.
In `@packages/prometheus-metrics/package.json`:
- Line 36: Remove the leftover "mocha" devDependency from package.json's
devDependencies section since tests now run with Vitest; update the lockfile
(npm/yarn/pnpm) afterwards and run the test script (or npm ci) to verify nothing
else depends on the "mocha" entry.
---
Nitpick comments:
In `@packages/bookshelf-plugins/package.json`:
- Around line 25-28: package.json currently lists "mocha" under
"devDependencies" even though the package uses Vitest for testing; remove the
unused "mocha" entry from the "devDependencies" object so only active test
tooling (Vitest) remains, ensuring the "devDependencies" block is still valid
JSON and no other scripts or files reference "mocha".
In `@packages/email-mock-receiver/package.json`:
- Around line 27-30: Remove the unused "mocha" devDependency from package.json:
open the package.json for the email-mock-receiver package, delete the "mocha":
"11.7.5" entry under "devDependencies" (leaving other entries like "sinon"
intact), and ensure the test script refers to Vitest (so no leftover mocha
references remain).
In `@packages/pretty-stream/test/PrettyStream.test.js`:
- Around line 12-468: Tests duplicate stream setup/assert/resolve logic; extract
a reusable async helper (e.g., expectPrettyOutput) that creates a
PrettyStream({mode}), a Writable with _write that converts data to string,
performs the expected assertion (exact match or includes), and resolves the
Promise; the helper should accept payload (object or string) and stringify
objects before calling ghostPrettyStream.write, accept an expected string or
predicate, and return a Promise so existing tests can be updated to call await
expectPrettyOutput({mode:'short', payload: {...}, expected: '...'}); locate uses
of PrettyStream and the many it(...) blocks to replace duplicated setup with
calls to expectPrettyOutput.
In `@packages/promise/package.json`:
- Around line 25-28: The package.json still lists "mocha" under devDependencies
even though tests use Vitest; remove the unused entry by deleting the "mocha":
"11.7.5" key/value from the devDependencies object in
packages/promise/package.json so only relevant test tooling (e.g., vitest)
remains; ensure the package.json stays valid JSON (no trailing commas) after
removing the "mocha" entry and run a quick install/check to confirm no other
references to mocha exist.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/express-test/test/__snapshots__/example-app.test.js.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (73)
package.jsonpackages/api-framework/package.jsonpackages/api-framework/test/http.test.jspackages/bookshelf-collision/package.jsonpackages/bookshelf-custom-query/package.jsonpackages/bookshelf-eager-load/package.jsonpackages/bookshelf-filter/package.jsonpackages/bookshelf-has-posts/package.jsonpackages/bookshelf-include-count/package.jsonpackages/bookshelf-order/package.jsonpackages/bookshelf-pagination/package.jsonpackages/bookshelf-plugins/package.jsonpackages/bookshelf-search/package.jsonpackages/bookshelf-transaction-events/package.jsonpackages/config/package.jsonpackages/database-info/package.jsonpackages/debug/package.jsonpackages/domain-events/package.jsonpackages/elasticsearch/package.jsonpackages/email-mock-receiver/package.jsonpackages/email-mock-receiver/test/.eslintrc.jspackages/email-mock-receiver/test/EmailMockReceiver.test.jspackages/errors/package.jsonpackages/errors/vitest.config.tspackages/express-test/package.jsonpackages/express-test/test/.eslintrc.jspackages/express-test/test/ExpectRequest.test.jspackages/express-test/test/Request.test.jspackages/express-test/test/example-app.test.jspackages/express-test/test/utils/overrides.jspackages/express-test/vitest.config.tspackages/http-cache-utils/package.jsonpackages/http-cache-utils/vitest.config.tspackages/http-stream/package.jsonpackages/jest-snapshot/package.jsonpackages/jest-snapshot/test/SnapshotManager.test.jspackages/job-manager/package.jsonpackages/job-manager/test/job-manager.test.jspackages/job-manager/vitest.config.tspackages/logging/package.jsonpackages/logging/test/logging.test.jspackages/metrics/package.jsonpackages/metrics/test/metrics.test.jspackages/mw-error-handler/package.jsonpackages/mw-error-handler/test/mw-error-handler.test.jspackages/mw-vhost/package.jsonpackages/nodemailer/package.jsonpackages/pretty-cli/package.jsonpackages/pretty-stream/package.jsonpackages/pretty-stream/test/PrettyStream.test.jspackages/prometheus-metrics/package.jsonpackages/prometheus-metrics/test/.eslintrc.jspackages/prometheus-metrics/test/metrics-server.test.tspackages/prometheus-metrics/vitest.config.tspackages/promise/package.jsonpackages/request/package.jsonpackages/root-utils/package.jsonpackages/root-utils/test/root-utils.test.jspackages/security/package.jsonpackages/server/package.jsonpackages/server/test/server.test.jspackages/tpl/package.jsonpackages/tsconfig.jsonpackages/validator/package.jsonpackages/version/package.jsonpackages/webhook-mock-receiver/package.jsonpackages/webhook-mock-receiver/test/.eslintrc.jspackages/webhook-mock-receiver/test/WebhookMockReceiver.test.jspackages/webhook-mock-receiver/vitest.config.tspackages/zip/package.jsonpackages/zip/test/.eslintrc.jspackages/zip/test/zip.test.jsvitest.config.ts
- metrics: restore process.env.MODE to undefined baseline in afterEach - root-utils: wrap Module._load mock in try/finally for safe cleanup
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
2 participants
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
vitest.config.tswith 90% coverage thresholdsdone()callback patterns to async/await across 9 test filesbefore/afterwith Vitest'sbeforeAll/afterAllin 5 filesvitest.config.tsoverrides documented in MIGRATION.mdTest plan
yarn testpasses for all 42 packages🤖 Generated with Claude Code