Skip to content

test(fxa-auth-server): migrate DB, Redis & storage tests from Mocha PART 3#20144

Merged
vbudhram merged 1 commit intomainfrom
fxa-12564-migrate-db-redis-storage-tests
Mar 9, 2026
Merged

test(fxa-auth-server): migrate DB, Redis & storage tests from Mocha PART 3#20144
vbudhram merged 1 commit intomainfrom
fxa-12564-migrate-db-redis-storage-tests

Conversation

@vbudhram
Copy link
Copy Markdown
Contributor

@vbudhram vbudhram commented Mar 5, 2026

Because

  • The fxa-auth-server is migrating from Mocha/Chai to Jest as part of FXA-12564
  • 8 test files in test/local/ covering DB, Redis, bounces, account lifecycle, and reminder modules needed migration
  • Some tests classified as "local" actually require real Redis, warranting reclassification as integration tests

This pull request

  • Adds 5 co-located unit tests in lib/:
    • lib/bounces.spec.ts (from test/local/bounces.js)
    • lib/account-events.spec.ts (from test/local/account-events.js)
    • lib/account-delete.spec.ts (from test/local/account-delete.js)
    • lib/db.spec.ts (from test/local/db.js)
    • lib/inactive-accounts/index.spec.ts (from test/local/inactive-accounts/index.js)
  • Adds 3 integration tests in test/remote/ (require real Redis):
    • test/remote/redis.in.spec.ts (from test/local/redis.js)
    • test/remote/cad-reminders.in.spec.ts (from test/local/cad-reminders.js)
    • test/remote/subscription-account-reminders.in.spec.ts (from test/local/subscription-account-reminders.js)
  • Converts proxyquire to jest.mock() factories, chai assertions to expect(), forEach-wrapped it() to it.each()
  • Fixes Redis contention in reminder tests by flushing stale keys in beforeEach and properly awaiting close() in afterEach

Issue

Closes: https://mozilla-hub.atlassian.net/browse/FXA-12564

Checklist

  • My commit is GPG signed
  • Tests pass locally (if applicable)
  • Documentation updated (if applicable)
  • RTL rendering verified (if UI changed)

Other Information

Stability: Unit tests (300 total suite) passed 5/5 runs. Integration reminder tests passed 5/5 runs after the Redis contention fix.

Note: Original Mocha test files in test/local/ are intentionally kept in place. Deletion will be handled in a follow-up PR once the migration is validated in CI.

Test Parity

All 167 runtime tests have full 1:1 parity — no tests added, dropped, or changed in behavior.

Mocha Source Jest Target Type Mocha Tests Jest Tests Parity
test/local/bounces.js lib/bounces.spec.ts Unit 8 8 FULL
test/local/account-events.js lib/account-events.spec.ts Unit 6 6 FULL
test/local/account-delete.js lib/account-delete.spec.ts Unit 15 15 FULL
test/local/db.js lib/db.spec.ts Unit 25 25 FULL
test/local/inactive-accounts/index.js lib/inactive-accounts/index.spec.ts Unit 10 10 FULL
test/local/redis.js test/remote/redis.in.spec.ts Integration 43 43 FULL
test/local/cad-reminders.js test/remote/cad-reminders.in.spec.ts Integration 17 17 FULL
test/local/subscription-account-reminders.js test/remote/subscription-account-reminders.in.spec.ts Integration 43 43 FULL
Total 167 167 FULL

@vbudhram vbudhram requested a review from a team as a code owner March 5, 2026 15:46
@vbudhram vbudhram changed the title test(fxa-auth-server): migrate DB, Redis & storage tests from Mocha PART 5 test(fxa-auth-server): migrate DB, Redis & storage tests from Mocha PART 3 Mar 5, 2026
@vbudhram vbudhram force-pushed the fxa-12564-migrate-db-redis-storage-tests branch from ef3f9a6 to 80e3020 Compare March 5, 2026 16:27
@vbudhram
Copy link
Copy Markdown
Contributor Author

vbudhram commented Mar 5, 2026

These PRs are pretty big to be reviewed manually. Here are some steps/prompts that you can use to verify the parity between them.

  • Compare the assertions between the mocha and jest tests
  • Compare the number of tests
  • Run 5 times to see if anything is flaky
  • Ask to find any gaps in the migrated tests with their originals

removePublicAndCanGrantTokens: sinon.fake.resolves(undefined),
};

Container.set(StripeHelper, mockStripeHelper);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question, non-blocking: I noticed between this PR and others that there appears a lot of similar container setup, did you find that was the case and there's a lot of duplicate setup, or are they all just slightly different?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They were sligthly different mocks for each test. I didn't want to refactor it out since it would change unrelated files. We can probably re-evaluate in the follow up issue.

expect(args[0].foo).toBe('bar');
expect(args[0].baz).toBe('qux');
expect(args[0].prefix).toBe('wibble');
expect(args[0].blee).toBeUndefined();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The original tests had an assertion that the log was passed to the redis constructor

        assert.equal(args[1], log, 'redisPool was passed log');

Probably safe to omit it, just wanted to flag!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should these be unit or integration since they're creating db with each test? I guess I would classify anything that creates a db instance and connects to it an integration test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair question! I can make these integration tests


import sinon from 'sinon';

// Mock fxa-shared/db to prevent real DB connections
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nshirley I was confused by function createDB, that gets mocked by below so no real db connection is made.

@nshirley nshirley self-assigned this Mar 9, 2026
@vbudhram vbudhram force-pushed the fxa-12564-migrate-db-redis-storage-tests branch from 1bcae6b to dff5ce7 Compare March 9, 2026 19:34
…o Jest (FXA-12564)

- Convert DB, Redis, and storage test files from Mocha/Chai to Jest
- Set maxWorkers=4 for Jest unit tests to prevent OOM kills
- Use page.waitForURL for pairing flow success check in functional tests
@vbudhram vbudhram force-pushed the fxa-12564-migrate-db-redis-storage-tests branch from dff5ce7 to 7fe9eff Compare March 9, 2026 19:42
@vbudhram vbudhram merged commit 2fac797 into main Mar 9, 2026
21 checks passed
@vbudhram vbudhram deleted the fxa-12564-migrate-db-redis-storage-tests branch March 9, 2026 20:06
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