Skip to content

fix(shared): raise timeout on fxa-shared DB integration setup hooks - #21298

Open
vbudhram wants to merge 1 commit into
mainfrom
fxa-14596
Open

vbudhram wants to merge 1 commit into
mainfrom
fxa-14596

Conversation

@vbudhram

@vbudhram vbudhram commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Because

  • Four fxa-shared DB integration suites flaked on the CI job "Integration Test - Libraries (PR)".
  • Their before() hook calls testDatabaseSetup. That drops and recreates the test database and runs about 30 SQL schema files, so it can pass mocha's default 2000 ms timeout.
  • When the hook timed out, knex was never assigned. The after() hook then threw "Cannot read properties of undefined (reading 'destroy')", which hid the real failure.

This pull request

  • Gives the before() hook in each of the four specs a 20 second timeout. The callback is now function () so it can call this.timeout(20000).
  • Changes after() to await knex?.destroy() so a failed setup reports only its own error.
  • Touches auth/index.spec.ts, auth/prune-token.spec.ts, auth/sent-emails.spec.ts and profile/index.spec.ts under packages/fxa-shared/test/db/models/.

Issue that this pull request solves

Closes: FXA-14596

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: the before() and after() hooks in the four spec files.
  • Suggested review order: any order. The change is the same in each file.
  • Risky or complex parts: none. Only test code changes.

Screenshots (Optional)

Other information (Optional)

  • The four specs pass locally with mocha against MySQL: 70 passing, 0 failing.
  • npx tsc -p packages/fxa-shared/tsconfig.json --noEmit and npx nx lint fxa-shared both exit 0.
  • No new tests. The change is to the tests themselves. Functional tests did not run, because this change does not affect them.

## Because

- Four fxa-shared DB integration suites flaked on the CI job "Integration Test - Libraries (PR)".
- Their `before()` hook calls `testDatabaseSetup`. That drops and recreates the test database and runs about 30 SQL schema files, so it can pass mocha's default 2000 ms timeout.
- When the hook timed out, `knex` was never assigned. The `after()` hook then threw "Cannot read properties of undefined (reading 'destroy')", which hid the real failure.

## This pull request

- Gives the `before()` hook in each of the four specs a 20 second timeout. The callback is now `function ()` so it can call `this.timeout(20000)`.
- Changes `after()` to `await knex?.destroy()` so a failed setup reports only its own error.
- Touches `auth/index.spec.ts`, `auth/prune-token.spec.ts`, `auth/sent-emails.spec.ts` and `profile/index.spec.ts` under `packages/fxa-shared/test/db/models/`.

## Issue that this pull request solves

Closes: FXA-14596
@vbudhram
vbudhram requested a review from a team as a code owner September 23, 2026 21:59
@vbudhram vbudhram added the auto label Sep 23, 2026
Copilot AI balanced review requested due to automatic review settings September 23, 2026 21:59
@vbudhram vbudhram added the auto label Sep 23, 2026
@vbudhram vbudhram assigned LZoog and vbudhram and unassigned LZoog Sep 23, 2026

Copilot AI left a comment

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.

Copilot review overview

🟢 Approval recommended

The focused test-only changes correctly address slow setup and prevent teardown from masking setup failures.

Review effort: Balanced
Findings: None

What changed in this PR

Stabilizes four fxa-shared database integration suites and preserves the original setup failure.

Changes:

  • Raises database setup hook timeouts to 20 seconds.
  • Makes teardown safe when setup fails before assigning knex.
File Description
auth/​index.spec.ts Hardens auth integration setup and teardown.
auth/​prune-token.spec.ts Hardens token-pruning integration setup and teardown.
auth/​sent-emails.spec.ts Hardens sent-email integration setup and teardown.
profile/​index.spec.ts Hardens profile integration setup and teardown.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants