Skip to content

test: fix a race in the storage tab's unavailable-storage test - #198

Merged
moul merged 1 commit into
mainfrom
fix/storage-tab-count-flake
Aug 7, 2026
Merged

test: fix a race in the storage tab's unavailable-storage test#198
moul merged 1 commit into
mainfrom
fix/storage-tab-count-flake

Conversation

@moul

@moul moul commented Aug 7, 2026

Copy link
Copy Markdown
Owner

What happened

The coverage job failed on an unrelated PR (#195) with:

AssertionError: expected '…' to be '0'
 ❯ src/shell/settings-storage-tab.test.tsx:142

Every other job on that PR passed, and the same suite passed locally —
including a full vitest run --coverage. So it is a flake, not a
regression, and it is worth fixing rather than re-running because it
blocks merges at random.

The race

await waitFor(() => expect(screen.getByText("Regenerable")).toBeTruthy());
expect(screen.getByText("Editor scripts").nextElementSibling?.textContent).toBe("0");

"Regenerable" is static markup, so it renders on the first pass. The count
beside "Editor scripts" starts at and settles a tick later. Waiting for
the heading and then asserting the count synchronously only worked when
the query happened to resolve first — usually true locally, intermittently
false under CI's slower coverage-instrumented run.

The fix

Await the thing actually being asserted. The assertion moves inside
waitFor, so it retries until the count settles.

This is strictly stronger, not weaker: the test's stated intent is "zeroes,
not a crash and not forever", and it now verifies exactly that instead
of accidentally depending on scheduling order. A genuinely stuck still
fails it, on the waitFor timeout.

Evidence

CI itself produced the failing-without-the-fix proof — it observed at
that assertion. Locally the suite is green before and after, which is the
whole problem with the old form.

@moul
moul merged commit f994391 into main Aug 7, 2026
9 checks passed
@moul
moul deleted the fix/storage-tab-count-flake branch August 7, 2026 00:24
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.

1 participant