CampaignDetail content-packs tests are flaky: a listPacks call from one test lands in the next #604

New issue

Open

opened 2026-09-11 14:58:29 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-09-11 14:58:29 +00:00

Contributor

What happened

CI run 9404 (PR #602, 2026-09-11 14:25 UTC), job "Frontend tests, audit, and build", failed on two tests in webapp/frontend/src/pages/CampaignDetail.test.jsx. PR #602 doesn't touch that file.

gives the GM a content packs panel for this campaign (line 326): expected "spy" to be called with arguments: [ { scope: 'campaign', campaignId: 'campaign-1' }, … ], Number of calls: 0.
does not show the content packs panel to players (line 340): expected "spy" to not be called at all, but actually been called 1 times. The call it received was { campaignId: "campaign-1", scope: "campaign" } with an AbortSignal, which is exactly the GM test's call.

The same tests passed on main's previous CI run (PR #601) and in a full local vitest run of the same branch (863 passed).

Diagnosis

The mirror-image failure means the GM test's asynchronous listPacks fetch (from an effect, with an abort signal) fired after that test's assertion ran and during the next test. That test then counted it as a player-side call.

Two gaps allow it:

The GM test asserts synchronously (expect(...).toHaveBeenCalledWith straight after a findBy… on something else), rather than awaiting the call itself.
The mock isn't reset, or the component isn't unmounted, before the next test's assertion, so a late call from the previous render is counted against the player test.

It shows up under CI's slower environment: the run's "environment" phase took 92 s.

Fix

In the GM test, wait for the call: await waitFor(() => expect(packsApi.listPacks).toHaveBeenCalledWith(...)).
In the player test, isolate from earlier renders: clear the listPacks mock at the start (vi.mocked(packsApi.listPacks).mockClear(), or vi.clearAllMocks() in beforeEach if the file doesn't already), and make sure the previous test's tree is unmounted (cleanup).
Check whether the component's effect aborts the request on unmount. It passes an AbortSignal, so a test-only mock that ignores the signal will still "call through" late. Assert on the signal, or have the mock honour it.

The fix is in test code only.

## What happened CI run 9404 (PR #602, 2026-09-11 14:25 UTC), job "Frontend tests, audit, and build", failed on two tests in `webapp/frontend/src/pages/CampaignDetail.test.jsx`. PR #602 doesn't touch that file. - **`gives the GM a content packs panel for this campaign`** (line 326): `expected "spy" to be called with arguments: [ { scope: 'campaign', campaignId: 'campaign-1' }, … ]`, **Number of calls: 0**. - **`does not show the content packs panel to players`** (line 340): `expected "spy" to not be called at all, but actually been called 1 times`. The call it received was `{ campaignId: "campaign-1", scope: "campaign" }` with an `AbortSignal`, which is **exactly the GM test's call**. The same tests passed on main's previous CI run (PR #601) and in a full local vitest run of the same branch (863 passed). ## Diagnosis The mirror-image failure means the GM test's asynchronous `listPacks` fetch (from an effect, with an abort signal) fired **after** that test's assertion ran and **during** the next test. That test then counted it as a player-side call. Two gaps allow it: 1. **The GM test asserts synchronously** (`expect(...).toHaveBeenCalledWith` straight after a `findBy…` on something else), rather than awaiting the call itself. 2. **The mock isn't reset, or the component isn't unmounted,** before the next test's assertion, so a late call from the previous render is counted against the player test. It shows up under CI's slower environment: the run's "environment" phase took 92 s. ## Fix - **In the GM test,** wait for the call: `await waitFor(() => expect(packsApi.listPacks).toHaveBeenCalledWith(...))`. - **In the player test, isolate from earlier renders:** clear the `listPacks` mock at the start (`vi.mocked(packsApi.listPacks).mockClear()`, or `vi.clearAllMocks()` in `beforeEach` if the file doesn't already), and make sure the previous test's tree is unmounted (`cleanup`). - **Check whether the component's effect aborts the request on unmount.** It passes an `AbortSignal`, so a test-only mock that ignores the signal will still "call through" late. Assert on the signal, or have the mock honour it. The fix is in test code only.

claude-bot added the

bug

frontend

labels

2026-09-11 14:58:41 +00:00