Skip to content

Remove mocha (WIP)#20310

Open
vbudhram wants to merge 1 commit intomainfrom
remove-mocha
Open

Remove mocha (WIP)#20310
vbudhram wants to merge 1 commit intomainfrom
remove-mocha

Conversation

@vbudhram
Copy link
Copy Markdown
Contributor

@vbudhram vbudhram commented Apr 2, 2026

Because

This pull request

Issue that this pull request solves

Closes: (issue number)

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on:
  • Suggested review order:
  • Risky or complex parts:

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

Copilot AI review requested due to automatic review settings April 2, 2026 18:47
@vbudhram vbudhram requested a review from a team as a code owner April 2, 2026 18:47
@vbudhram vbudhram changed the title Remove mocha Remove mocha (WIP) Apr 2, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes Mocha-based tests and tooling from fxa-auth-server, standardizing on Jest for running unit/integration/script tests.

Changes:

  • Deleted legacy Mocha test suites, helpers, and coverage tooling (nyc/mocha-coverage).
  • Updated test runner scripts/CI and VS Code tasks/launch configs to use Jest.
  • Adjusted a few Jest integration tests/utilities (Redis scoped cleanup; DB closes knex; some script specs updated to Jest assertions).

Reviewed changes

Copilot reviewed 75 out of 372 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/fxa-auth-server/test/local/metrics/client-tags.ts Removed Mocha local test file.
packages/fxa-auth-server/test/local/l10n/index.ts Removed Mocha local test file.
packages/fxa-auth-server/test/local/ip_profiling.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/inactive-accounts/index.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/google-maps-services.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/getRemoteAddressChain.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/geodb.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/features.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/error.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/email/utils.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/email/notifications.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/email/delivery.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/email/delivery-delay.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/email-cloud-tasks.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/crypto/scrypt.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/crypto/random.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/crypto/pbkdf2.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/crypto/hkdf.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/crypto/butil.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/config/index.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/cad-reminders.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/bounces.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/authMethods.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/account-events.js Removed Mocha local test file.
packages/fxa-auth-server/test/local/account-delete.js Removed Mocha local test file.
packages/fxa-auth-server/test/lib/util.ts Removed Mocha-era test util.
packages/fxa-auth-server/test/lib/util.js Removed Mocha-era test util.
packages/fxa-auth-server/test/lib/server.ts Removed Mocha-era server harness.
packages/fxa-auth-server/test/lib/server.js Removed Mocha-era server harness.
packages/fxa-auth-server/test/lib/oauth-test.json Removed Mocha-era OAuth test config.
packages/fxa-auth-server/test/lib/mocks.js Removed Mocha-era dependency mocking helper.
packages/fxa-auth-server/test/key_server_stub.js Removed Mocha-era server stub.
packages/fxa-auth-server/test/e2e/push_tests.js Removed Mocha e2e test that hits external services.
packages/fxa-auth-server/test/e2e/README.txt Removed Mocha e2e README.
packages/fxa-auth-server/test/chaiWithoutTruncation.ts Removed Chai-specific configuration helper.
packages/fxa-auth-server/test/bench/index.js Removed Mocha-era benchmark harness.
packages/fxa-auth-server/test/bench/bot.js Removed Mocha-era benchmark client.
packages/fxa-auth-server/test/assert.js Removed Chai/Sinon combined assert helper.
packages/fxa-auth-server/test/.eslintrc Removed Mocha-specific ESLint config.
packages/fxa-auth-server/scripts/update-subscriptions-to-new-plan/update-subscriptions-to-new-plan.spec.ts Updated script spec to Jest expectations & adjusted fixture imports.
packages/fxa-auth-server/scripts/test-remote-quick.js Removed Mocha remote test runner script.
packages/fxa-auth-server/scripts/test-local.sh Switched local test entrypoint to Jest-only.
packages/fxa-auth-server/scripts/test-ci.sh Removed Mocha execution; CI now runs Jest unit/integration suites.
packages/fxa-auth-server/scripts/stripe-products-and-plans-to-firestore-documents/plan-language-tags-guesser.spec.ts Updated script spec to Jest expectations and local imports.
packages/fxa-auth-server/scripts/recorded-future/lib.spec.ts Updated script spec to Jest expectations and local imports.
packages/fxa-auth-server/scripts/prune-tokens.ts Avoid Redis setup for --help; close Redis after pruning.
packages/fxa-auth-server/scripts/move-customers-to-new-plan/move-customers-to-new-plan.spec.ts Updated script spec to Jest expectations and added Container reset.
packages/fxa-auth-server/scripts/mocha-coverage.js Removed Mocha+nyc coverage runner.
packages/fxa-auth-server/scripts/delete-inactive-accounts/lib.spec.ts Updated script spec to Jest expectations and local imports.
packages/fxa-auth-server/scripts/convert-customers-to-stripe-automatic-tax/helpers.spec.ts Updated script spec to Jest expectations and local imports.
packages/fxa-auth-server/scripts/cleanup-old-carts/cleanup-old-carts.spec.ts Updated script spec typings/expectations for Jest.
packages/fxa-auth-server/scripts/check-firestore-stripe-sync/check-firestore-stripe-sync.spec.ts Updated script spec for Jest + formatting cleanup.
packages/fxa-auth-server/scripts/cancel-subscriptions-to-plan/cancel-subscriptions-to-plan.spec.ts Updated script spec for Jest assertions and async rejection checks.
packages/fxa-auth-server/package.json Removed Mocha/Chai/nyc deps and scripts; streamlined test scripts.
packages/fxa-auth-server/lib/subscription-account-reminders.in.spec.ts Minor Jest integration test updates + more isolated Redis key cleanup.
packages/fxa-auth-server/lib/redis.in.spec.ts Replaced flushall() with scoped cleanup to reduce parallel-test flake risk.
packages/fxa-auth-server/lib/db.ts DB now tracks/destroys knex on close.
packages/fxa-auth-server/lib/cad-reminders.in.spec.ts Renamed Redis prefix to avoid collisions.
packages/fxa-auth-server/jest.config.js Expanded Jest default testMatch to include scripts specs; adjusted transformIgnorePatterns.
packages/fxa-auth-server/.vscode/tasks.json Updated VS Code task to run Jest current file.
packages/fxa-auth-server/.vscode/launch.json Replaced Mocha debug configs with Jest debug configs.
packages/fxa-auth-server/.mocharc.js Removed Mocha config file.
nx.json Removed Nx targets related to Mocha-based integration variants.
.circleci/config.yml Removed CircleCI workflows/jobs related to removed Mocha integration variants.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +7 to 13
"name": "Jest Current File",
"program": "${workspaceFolder}/node_modules/.bin/jest",
"args": [
"--require",
"esbuild-register",
"--recursive",
"--timeout",
"999999",
"--colors",
"${workspaceFolder}/test/local"
"--no-coverage",
"--forceExit",
"${relativeFile}"
],
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${workspaceFolder}/node_modules/.bin/jest is likely incorrect in a monorepo where dependencies live at the repo root (the old Mocha config referenced ../../node_modules/...). This will break debugging unless packages/fxa-auth-server/node_modules exists. Consider pointing program at the workspace root’s Jest binary (e.g. ../../node_modules/.bin/jest) or using a runtimeExecutable of npx with jest in args.

Copilot uses AI. Check for mistakes.
Comment on lines +210 to +216
// keys('*') returns fully-prefixed keys, so strip the prefix before del() to avoid
// double-prefixing.
const keys = await redis.redis.keys('*');
if (keys.length) {
await redis.redis.del(
...keys.map((k: string) => k.replace(prefix, ''))
);
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KEYS * is an O(N) operation and can become slow/non-deterministic if the prefixed keyspace grows (even in tests, this can introduce flakiness). Also, replace(prefix, '') is not anchored; if prefix appears later in a key, it will strip the wrong segment. Prefer iterating via SCAN and stripping only when k.startsWith(prefix) (e.g., k.slice(prefix.length)), and consider chunking deletes to avoid huge DEL argument lists.

Suggested change
// keys('*') returns fully-prefixed keys, so strip the prefix before del() to avoid
// double-prefixing.
const keys = await redis.redis.keys('*');
if (keys.length) {
await redis.redis.del(
...keys.map((k: string) => k.replace(prefix, ''))
);
// scan('*') returns fully-prefixed keys, so strip the prefix before del() to avoid
// double-prefixing. Use SCAN instead of KEYS to avoid blocking the Redis server,
// and delete in chunks to avoid huge DEL argument lists.
const keysToDelete: string[] = [];
let cursor = '0';
do {
const result = await redis.redis.scan(cursor, 'MATCH', '*', 'COUNT', '100');
cursor = result[0];
const keys = result[1];
for (const k of keys) {
if (k.startsWith(prefix)) {
keysToDelete.push(k.slice(prefix.length));
}
}
} while (cursor !== '0');
for (let i = 0; i < keysToDelete.length; i += 100) {
await redis.redis.del(...keysToDelete.slice(i, i + 100));

Copilot uses AI. Check for mistakes.
Comment on lines +35 to 38
const shouldPrintHelp =
process.argv.includes('--help') || process.argv.includes('-h');
program
.version(pckg.version)
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

init() previously behaved like a Promise<void>; returning 0 changes its inferred return type to Promise<number | void>, which can break callers expecting void (and makes the function’s contract ambiguous). If the goal is to avoid side effects on --help, consider either (1) making init consistently return an exit code (Promise<number>) and updating its callers accordingly, or (2) keeping init(): Promise<void> and simply return; here (Commander will typically print help/exit on its own).

Copilot uses AI. Check for mistakes.
.parse(process.argv);

if (shouldPrintHelp) {
return 0;
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

init() previously behaved like a Promise<void>; returning 0 changes its inferred return type to Promise<number | void>, which can break callers expecting void (and makes the function’s contract ambiguous). If the goal is to avoid side effects on --help, consider either (1) making init consistently return an exit code (Promise<number>) and updating its callers accordingly, or (2) keeping init(): Promise<void> and simply return; here (Commander will typically print help/exit on its own).

Suggested change
return 0;
return;

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +13
testMatch: [
'<rootDir>/lib/**/*.spec.ts',
'<rootDir>/config/**/*.spec.ts',
'<rootDir>/scripts/**/*.spec.ts',
],
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including <rootDir>/scripts/**/*.spec.ts in the default Jest config will also match *.in.spec.ts files (since they still end with .spec.ts). If .in.spec.ts are intended to run only under jest.integration.config.js, consider excluding them from the default run (e.g. via testPathIgnorePatterns for \\.in\\.spec\\.ts$) to keep TEST_TYPE=unit fast and hermetic.

Copilot uses AI. Check for mistakes.
@vbudhram vbudhram self-assigned this Apr 2, 2026
@vbudhram vbudhram force-pushed the remove-mocha branch 13 times, most recently from 8443e7a to 7c7bae7 Compare April 4, 2026 03:14
…tructure

Because:
- Jest migration is complete with full parity across unit, integration,
  script, and OAuth tests
- Mocha infrastructure (218 test files, 7 npm deps, 3 CI jobs) is dead
  weight that slows installs, confuses contributors, and wastes CI compute

This commit:
- Deletes all Mocha test files (test/local/, test/oauth/, test/remote/*.js,
  test/scripts/*.js, test/client/, test/bench/, test/e2e/)
- Removes mocha, chai, nyc, and related @types dependencies
- Rewrites test-local.sh and test-ci.sh to run Jest only
- Consolidates 3 CI integration jobs into 1
- Removes test-integration-v2 and test-integration-jest nx targets
- Updates .vscode debug configs from Mocha to Jest
- Removes .mocharc.js and mocha config from package.json
- Switches 6 integration tests to shared server for faster startup
- Increases integration test workers from 4 to 6
- Keeps test/mocks.js, test/routes_helpers.js, and proxyquire (follow-up)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants