refactor(auth): migrate fxa-auth-server tests from sinon/proxyquire WIP by vbudhram · Pull Request #20366 · mozilla/fxa

vbudhram · 2026-04-13T18:05:37Z

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.

…o jest Remove all sinon and proxyquire usage from fxa-auth-server test files, replacing with Jest built-in mocking (jest.fn, jest.spyOn, jest.mock). Key changes: - Replace sinon.stub/spy with jest.fn/jest.spyOn across 118 files - Replace proxyquire with jest.mock for module mocking - Convert sinon assertions (calledOnce, calledWith, etc.) to Jest equivalents (toHaveBeenCalledTimes, toHaveBeenCalledWith, etc.) - Fix test/mocks.js: use targeted Account.metricsEnabled patch instead of global jest.mock that clobbered other test files' module mocks - Rename jest.setup-proxyquire.js to jest.setup-resolve.js (TS module resolution patch still needed for jest.mock with .ts index files) - Fix beforeAll/beforeEach interactions with clearMocks:true config - Fix sinon.onCall(n) patterns to jest.mockResolvedValueOnce chains - Fix jest.spyOn on jest.fn() + restoreAllMocks implementation wipe All 150 test suites pass (3343/3343 tests).

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Refactors the fxa-auth-server Jest test suite to migrate from sinon/proxyquire-style stubbing to Jest mocks/spies, and updates Jest setup to improve TypeScript module resolution during mocking.

Changes:

Replaced sinon stubs/spies/fakes with jest.fn() / jest.spyOn() across many unit tests.
Updated assertions from sinon call inspection (callCount, args, etc.) to Jest mock assertions (toHaveBeenCalledTimes, .mock.calls, etc.).
Replaced Jest setup file usage from a proxyquire-focused resolver to a .ts-aware resolve.sync patch.

Reviewed changes

Copilot reviewed 56 out of 118 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
packages/fxa-auth-server/lib/routes/ip-profiling.spec.ts	Migrates request/mailer/redis/Container stubs from sinon to Jest.
packages/fxa-auth-server/lib/routes/geo-location.spec.ts	Updates log assertions to Jest `.mock.calls` patterns.
packages/fxa-auth-server/lib/routes/cloud-tasks.spec.ts	Converts cloud task handler tests from sinon sandbox to Jest mocks.
packages/fxa-auth-server/lib/routes/cloud-scheduler.spec.ts	Replaces sinon method stubbing with `jest.spyOn(...).mockResolvedValue(...)`.
packages/fxa-auth-server/lib/routes/auth-schemes/verified-session-token.spec.ts	Converts auth scheme unit tests to Jest mocks and assertions.
packages/fxa-auth-server/lib/routes/auth-schemes/shared-secret.spec.ts	Replaces sinon fake with Jest mock and updates assertions.
packages/fxa-auth-server/lib/routes/auth-schemes/refresh-token.spec.ts	Updates spies/assertions to Jest equivalents.
packages/fxa-auth-server/lib/routes/auth-schemes/mfa.spec.ts	Migrates MFA auth scheme tests to Jest mocks/spies.
packages/fxa-auth-server/lib/routes/auth-schemes/hawk-fxa-token.spec.ts	Replaces sinon fakes with Jest mocks for hawk strategy tests.
packages/fxa-auth-server/lib/routes/auth-schemes/google-oidc.spec.ts	Updates Google OIDC strategy test stubbing to Jest.
packages/fxa-auth-server/lib/routes/attached-clients.spec.ts	Converts OAuth/DB/devices mocks + assertions from sinon to Jest.
packages/fxa-auth-server/lib/pushbox/index.spec.ts	Reworks PushboxDB stubbing away from sinon sandbox toward Jest mocks.
packages/fxa-auth-server/lib/profile/updates.spec.ts	Migrates push notification + message delete spies to Jest.
packages/fxa-auth-server/lib/payments/iap/iap-formatter.spec.ts	Updates entitlement checks to Jest mocks.
packages/fxa-auth-server/lib/payments/iap/iap-config.spec.ts	Replaces sinon sandbox and Firestore stubs with Jest mocks.
packages/fxa-auth-server/lib/payments/iap/google-play/user-manager.spec.ts	Migrates Firestore query/method stubs to Jest.
packages/fxa-auth-server/lib/payments/iap/google-play/subscriptions.spec.ts	Converts Play subscriptions tests to Jest mocks and assertions.
packages/fxa-auth-server/lib/payments/iap/google-play/purchase-manager.spec.ts	Replaces sinon fakes/stubs with Jest fn/spy patterns.
packages/fxa-auth-server/lib/payments/iap/google-play/play-billing.spec.ts	Migrates Firestore collection stubbing to Jest.
packages/fxa-auth-server/lib/payments/iap/apple-app-store/subscriptions.spec.ts	Converts App Store subscriptions tests to Jest mocks.
packages/fxa-auth-server/lib/payments/iap/apple-app-store/apple-iap.spec.ts	Replaces sinon stub usage with Jest mocks for Firestore collection.
packages/fxa-auth-server/lib/payments/iap/apple-app-store/app-store-helper.spec.ts	Migrates API/client stubs from sinon sandbox to Jest.
packages/fxa-auth-server/lib/oauth/jwt_access_token.spec.ts	Updates JWT signing spy assertions to Jest.
packages/fxa-auth-server/lib/oauth/grant.spec.ts	Converts DB/JWT/capability service stubs to Jest mocks.
packages/fxa-auth-server/lib/notifier.spec.ts	Replaces sinon spies with Jest mocks for SNS publish/logging.
packages/fxa-auth-server/lib/metrics/events.spec.ts	Updates glean + metricsContext mock usage to Jest conventions.
packages/fxa-auth-server/lib/metrics/amplitude.spec.ts	Migrates log assertions to Jest mock APIs.
packages/fxa-auth-server/lib/google-maps-services.spec.ts	Converts geocode + Sentry stubbing from sinon to Jest.
packages/fxa-auth-server/lib/geodb.spec.ts	Switches mock logger methods from sinon stubs to Jest fns.
packages/fxa-auth-server/lib/features.spec.ts	Migrates crypto hashing spies to Jest and updates assertions.
packages/fxa-auth-server/lib/email/utils/helpers.spec.ts	Moves amplitude/log assertions and AccountEventsManager stubs to Jest.
packages/fxa-auth-server/lib/email/notifications.spec.ts	Converts queue/db/log spies and Date.now stubbing to Jest.
packages/fxa-auth-server/lib/email/delivery.spec.ts	Migrates email delivery handler tests to Jest mocks/assertions.
packages/fxa-auth-server/lib/email/delivery-delay.spec.ts	Migrates delivery delay handler tests to Jest mocks/assertions.
packages/fxa-auth-server/lib/email-cloud-tasks.spec.ts	Replaces sinon sandbox stubs for cloud tasks scheduling with Jest fn mocks.
packages/fxa-auth-server/lib/devices.spec.ts	Converts DB/log/push/pushbox assertions from sinon to Jest `.mock.calls`.
packages/fxa-auth-server/lib/db.spec.ts	Migrates redis/model stubs and call assertions to Jest.
packages/fxa-auth-server/lib/customs.spec.ts	Replaces sinon sandbox stubs with Jest spies; updates matching assertions.
packages/fxa-auth-server/lib/cad-reminders.in.spec.ts	Updates log error assertions from sinon to Jest.
packages/fxa-auth-server/lib/authMethods.spec.ts	Converts DB totpToken stubs and assertions to Jest.
packages/fxa-auth-server/lib/account-events.spec.ts	Migrates Firestore + statsd stubs and assertions to Jest.
packages/fxa-auth-server/lib/account-delete.spec.ts	Converts account deletion tests from sinon sandbox to Jest mocks.
packages/fxa-auth-server/jest.setup-resolve.js	Updates resolver patch documentation to focus on Jest/TS module resolution.
packages/fxa-auth-server/jest.config.js	Switches Jest setup file from `jest.setup-proxyquire.js` to `jest.setup-resolve.js`.

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

Copilot · 2026-04-13T18:07:19Z

packages/fxa-auth-server/lib/routes/auth-schemes/mfa.spec.ts


  beforeAll(() => {
-    sinon.stub(authMethods, 'availableAuthenticationMethods');
+    jest.spyOn(authMethods, 'availableAuthenticationMethods');


Overwriting authMethods.availableAuthenticationMethods after calling jest.spyOn(...) breaks the spy (the spy is attached to the original function, but the export is then replaced). This can cause jest.restoreAllMocks() to not restore the module function correctly and can make assertions on the spy unreliable. Prefer keeping the spy and setting its implementation via mockResolvedValue(...) on the spy instance (or use jest.spyOn(...).mockResolvedValue(...) in beforeEach) instead of reassigning the exported function.

Copilot · 2026-04-13T18:07:20Z

packages/fxa-auth-server/lib/routes/auth-schemes/mfa.spec.ts

+    jest.clearAllMocks();

-    authMethods.availableAuthenticationMethods = sinon.fake.resolves(
-      new Set(['pwd', 'email'])
-    );
+    authMethods.availableAuthenticationMethods = jest
+      .fn()
+      .mockResolvedValue(new Set(['pwd', 'email']));


Overwriting authMethods.availableAuthenticationMethods after calling jest.spyOn(...) breaks the spy (the spy is attached to the original function, but the export is then replaced). This can cause jest.restoreAllMocks() to not restore the module function correctly and can make assertions on the spy unreliable. Prefer keeping the spy and setting its implementation via mockResolvedValue(...) on the spy instance (or use jest.spyOn(...).mockResolvedValue(...) in beforeEach) instead of reassigning the exported function.

Copilot · 2026-04-13T18:07:20Z

packages/fxa-auth-server/lib/email-cloud-tasks.spec.ts

 const mockEmailTasks = {
-  scheduleFirstEmail: sandbox.stub(),
-  scheduleSecondEmail: sandbox.stub(),
-  scheduleFinalEmail: sandbox.stub(),
+  scheduleFirstEmail: jest.fn(),
+  scheduleSecondEmail: jest.fn(),
+  scheduleFinalEmail: jest.fn(),
 };


mockEmailTasks.* are module-level jest.fn() mocks, but the suite no longer resets/clears them between tests. jest.restoreAllMocks() does not clear call history for standalone jest.fn() values, so call counts can leak across tests and make them order-dependent. Add an explicit reset (e.g., jest.clearAllMocks() in beforeEach/afterEach, or mockEmailTasks.scheduleFirstEmail.mockClear() etc.) to ensure isolation.

Copilot · 2026-04-13T18:07:20Z

packages/fxa-auth-server/lib/email-cloud-tasks.spec.ts

+    jest.restoreAllMocks();
    Container.reset();


mockEmailTasks.* are module-level jest.fn() mocks, but the suite no longer resets/clears them between tests. jest.restoreAllMocks() does not clear call history for standalone jest.fn() values, so call counts can leak across tests and make them order-dependent. Add an explicit reset (e.g., jest.clearAllMocks() in beforeEach/afterEach, or mockEmailTasks.scheduleFirstEmail.mockClear() etc.) to ensure isolation.

Copilot · 2026-04-13T18:07:20Z

packages/fxa-auth-server/lib/pushbox/index.spec.ts

+// eslint-disable-next-line @typescript-eslint/no-unused-vars
 const pushboxDbModule = require('./db');


Disabling the lint rule for an intentionally unused require makes the test harder to maintain and can hide real unused-variable issues nearby. If this import is only needed for typing, consider converting it to a type-only import (or using import type) and removing the runtime require; if it isn’t needed at all with the new constructor injection approach, remove it entirely.

Suggested change

// eslint-disable-next-line @typescript-eslint/no-unused-vars

const pushboxDbModule = require('./db');

Copilot · 2026-04-13T18:07:21Z

packages/fxa-auth-server/lib/routes/cloud-scheduler.spec.ts

-      sinon.assert.calledOnceWithExactly(
-        mockProcessAccountDeletionInRange,
+      expect(mockProcessAccountDeletionInRange).toHaveBeenCalledTimes(1);
+      expect(mockProcessAccountDeletionInRange).toHaveBeenCalledWith(


The previous assertion (calledOnceWithExactly) verified both the arguments and that there were no extra parameters. toHaveBeenCalledWith does not enforce “exactly” in the same way if additional args are passed. If exactness matters here, also assert the call’s arity (e.g., check mock.calls[0].length) or use toHaveBeenNthCalledWith(1, ...) together with an explicit length check.

Suggested change

expect(mockProcessAccountDeletionInRange).toHaveBeenCalledWith(

expect(mockProcessAccountDeletionInRange.mock.calls[0]).toHaveLength(7);

expect(mockProcessAccountDeletionInRange).toHaveBeenNthCalledWith(

1,

vbudhram requested a review from a team as a code owner April 13, 2026 18:05

Copilot AI review requested due to automatic review settings April 13, 2026 18:05

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(auth): migrate fxa-auth-server tests from sinon/proxyquire WIP#20366

refactor(auth): migrate fxa-auth-server tests from sinon/proxyquire WIP#20366
vbudhram wants to merge 1 commit intomainfrom
fxa-13263

vbudhram commented Apr 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// eslint-disable-next-line @typescript-eslint/no-unused-vars
		const pushboxDbModule = require('./db');

-      expect(mockProcessAccountDeletionInRange).toHaveBeenCalledWith(
+      expect(mockProcessAccountDeletionInRange.mock.calls[0]).toHaveLength(7);
+      expect(mockProcessAccountDeletionInRange).toHaveBeenNthCalledWith(
+,

Conversation

vbudhram commented Apr 13, 2026

Because

This pull request

Issue that this pull request solves

Checklist

How to review (Optional)

Screenshots (Optional)

Other information (Optional)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants