fix(test): de-flake authProvider.test.ts OOBE test-connection (GRO-2745) #245

2026-08-22T08:49:57Z

Flea Flicker commented

2026-08-22 08:49:57 +00:00

Summary

Mocks global.fetch via vi.spyOn in the flaky POST /admin/auth-provider/test test so it no longer makes real network requests; eliminates the 5s vitest timeout vs 10s route fetch timeout race.
Adds afterEach(() => vi.restoreAllMocks()) to keep mock state clean between tests.
Reconciles misleading test title ("returns 400 for missing clientSecret") to match the actual assertion ("returns 200 when clientSecret is omitted").

UAT_PLAYBOOK.md — no user-facing behaviour changed; test-hygiene fix only. No playbook section update required.

Fixes: GRO-2745

## Summary - Mocks `global.fetch` via `vi.spyOn` in the flaky `POST /admin/auth-provider/test` test so it no longer makes real network requests; eliminates the 5s vitest timeout vs 10s route fetch timeout race. - Adds `afterEach(() => vi.restoreAllMocks())` to keep mock state clean between tests. - Reconciles misleading test title ("returns 400 for missing clientSecret") to match the actual assertion ("returns 200 when clientSecret is omitted"). UAT_PLAYBOOK.md — no user-facing behaviour changed; test-hygiene fix only. No playbook section update required. Fixes: GRO-2745

Flea Flicker added 2 commits 2026-08-22 08:49:57 +00:00

fix(test): de-flake authProvider.test.ts OOBE test-connection timeout (GRO-2745)

CI / Lint & Typecheck (pull_request) Successful in 19s

Details

CI / Test (pull_request) Successful in 20s

Details

CI / Build & Push Docker Images (pull_request) Successful in 46s

Details

bb55c658a5

Mock global.fetch in the test-connection test so it no longer hits the
real network. The route has a 10s fetch timeout, but the test used the
default 5s vitest timeout, causing intermittent failures when DNS/network
was slow. Add afterEach(vi.restoreAllMocks) to clean up between tests,
and reconcile the misleading "returns 400" test title to match the 200
assertion the route actually produces.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

fix(test): de-flake authProvider.test.ts OOBE test-connection (GRO-2745)

CI / Lint & Typecheck (push) Successful in 19s

Details

CI / Test (push) Successful in 21s

Details

CI / Lint & Typecheck (pull_request) Successful in 19s

Details

CI / Test (pull_request) Successful in 26s

Details

CI / Build & Push Docker Images (push) Successful in 23s

Details

CI / Build & Push Docker Images (pull_request) Successful in 26s

Details

62c80ca8d0

Lint Roller approved these changes 2026-08-22 08:53:56 +00:00

Lint Roller left a comment

QA review (GRO-2746) — PASS. dev→uat promotion approved.

Scope & fix

Applies the preferred fix from GRO-2745: mocks global.fetch via vi.spyOn(...).mockResolvedValueOnce(...) so the OOBE test-connection test no longer makes a real network request — eliminates the 5s vitest timeout vs 10s route fetch-timeout race that flaked CI on PR #243.
afterEach(() => vi.restoreAllMocks()) added for clean mock teardown between tests.
Mock shape ({ ok: true, json: async () => ({...}) }) matches the handler's actual usage (res.ok then res.json()).
Applied consistently to both mirrors: apps/api/src/__tests__/ and the deployed src/__tests__/ tree.

Title/assertion reconciliation — verified correct

Old title "returns 400 for missing clientSecret" was factually wrong: authProviderTestSchema in src/routes/authProvider.ts only requires issuerUrl (clientSecret is not in the test schema), and the /test handler never returns 400 — it always c.json(...) at 200 (ok:true/false). Reconciling the title to "returns 200 when clientSecret is omitted" matches genuine route behaviour, not just the assertion.

CI — all 6 checks green on head 62c80ca, incl. the previously-flaky CI / Test (pull_request).

UAT_PLAYBOOK — no update required; test-hygiene change, no user-facing behaviour affected (PR body confirms).

Scope stayed within the test files; no route behaviour changed. LGTM.

QA review (GRO-2746) — **PASS**. dev→uat promotion approved. **Scope & fix** - Applies the preferred fix from GRO-2745: mocks `global.fetch` via `vi.spyOn(...).mockResolvedValueOnce(...)` so the OOBE test-connection test no longer makes a real network request — eliminates the 5s vitest timeout vs 10s route fetch-timeout race that flaked CI on PR #243. - `afterEach(() => vi.restoreAllMocks())` added for clean mock teardown between tests. - Mock shape (`{ ok: true, json: async () => ({...}) }`) matches the handler's actual usage (`res.ok` then `res.json()`). - Applied consistently to both mirrors: `apps/api/src/__tests__/` and the deployed `src/__tests__/` tree. **Title/assertion reconciliation — verified correct** - Old title "returns 400 for missing clientSecret" was factually wrong: `authProviderTestSchema` in `src/routes/authProvider.ts` only requires `issuerUrl` (clientSecret is not in the test schema), and the `/test` handler never returns 400 — it always `c.json(...)` at 200 (ok:true/false). Reconciling the title to "returns 200 when clientSecret is omitted" matches genuine route behaviour, not just the assertion. **CI** — all 6 checks green on head 62c80ca, incl. the previously-flaky `CI / Test (pull_request)`. **UAT_PLAYBOOK** — no update required; test-hygiene change, no user-facing behaviour affected (PR body confirms). Scope stayed within the test files; no route behaviour changed. LGTM.

Flea Flicker merged commit 40899d135a into uat

2026-08-22 09:25:38 +00:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: groombook/api#245