Skip to content

QuotaBurn retry-budget test flakes ~1 in 3 on testing-library's 1000ms default timeout #3474

Description

@atomantic

Problem

client/src/pages/QuotaBurn.test.jsxQuotaBurn save races"stops claiming it is saving once the retry budget is spent" fails intermittently — measured ~1 in 3 isolated runs (17/17, 17/17, 16/17 over three consecutive runs).

It fails at the first waitFor:

await waitFor(() => expect(api.saveQuotaBurn).toHaveBeenCalledTimes(2));

with a Timeout from @testing-library/dom's wait-for.js.

Cause

Nothing to do with the component. QuotaBurn.jsx uses SAVE_DEBOUNCE_MS = 500, and this test waits for the debounced save plus its retry — two sequential 500ms windows — against testing-library's 1000ms default waitFor timeout. Under parallel test-worker CPU contention that budget is simply too tight, so the assertion times out before the second call lands.

This is a known class in this repo, already diagnosed once. client/src/test/settledInput.js:15-21 carries the fix and the rationale verbatim:

waitFor defaults to a 1000ms timeout (testing-library/dom, uncustomized here — see setup.js). Under parallel test-worker CPU contention that's tight for a user.type()/user.clear() + React re-render to settle, producing phantom failures that have nothing to do with the component under test.

That fix was applied to one helper. The underlying default is still 1000ms suite-wide — neither client/vitest.config.js nor client/src/test/setup.js raises it — so every other debounce-sensitive test remains exposed.

Decision

Raise the default globally rather than patching this one call site. In client/src/test/setup.js:

import { configure } from '@testing-library/react';

// testing-library defaults asyncUtilTimeout to 1000ms. Several views debounce at
// 500ms and a couple wait on a debounce plus a retry, so under parallel-worker CPU
// contention that budget produces phantom failures unrelated to the component
// under test (see settledInput.js, which hit this first). 3000ms matches the value
// already chosen there.
configure({ asyncUtilTimeout: 3000 });

Rationale for global over local: the root cause is the suite-wide default, not this test. Patching call sites one flake at a time means rediscovering the same diagnosis each time. The cost is that a genuinely hung assertion takes 3s instead of 1s to fail, which is an acceptable trade for a suite that already runs in ~30s.

Once it lands, settledInput.js's local { timeout: 3000 } becomes redundant and can be dropped (its comment should stay, moved to setup.js).

Not caused by the Biome migration

Verified during #3473: QuotaBurn.jsx and QuotaBurn.test.jsx are byte-identical to main (git diff main..HEAD empty for both), and the flake reproduces on main.

Affected files

  • client/src/test/setup.js — add the configure call
  • client/src/test/settledInput.js — drop the now-redundant local override, keep the rationale
  • client/src/pages/QuotaBurn.test.jsx — no change expected

Acceptance criteria

  • configure({ asyncUtilTimeout: 3000 }) in client/src/test/setup.js with the rationale comment
  • The named test passes 10/10 consecutive isolated runs (npx vitest run src/pages/QuotaBurn.test.jsx)
  • Full client suite still green
  • settledInput.js no longer needs its own { timeout: 3000 }

Activity

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

Metadata

Metadata

Assignees

Labels

area:devtoolsDevtools/workspace/code-review surfacesbugSomething isn't workingplanTracked by /do:replanseverity:lowtestsTest suite / test infrastructure

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions