Make the frontend test suite deterministic (unblocks CI) - #68
Merged
Conversation
Jenkins has been red on the HouseholdSettings "surfaces a save error" test. Two separate bugs were stacked here. 1. PR #63's timeout fix never took effect. It set testTimeout/hookTimeout in `frontend/vite.config.ts`, but this repo also has `frontend/vitest.config.ts`, and Vitest resolves vitest.config.* in preference to vite.config.*. The vite.config.ts `test` block has therefore always been dead config that merely looks authoritative -- the effective timeout was still the 5000ms default. Verified rather than assumed: a probe test sleeping 7s failed at "timed out in 5000ms" before this change and passes after it. The timeouts move to vitest.config.ts, where they must stay above the asyncUtilTimeout (15s) that tests/setup.ts configures -- Vitest kills the test at testTimeout no matter what findBy* is willing to wait for, so a lower value silently caps that budget. The dead `test` block and its orphaned `src/test/setup.ts` are removed so the next person can't repeat the same fix in the wrong file. 2. The test itself had a real race, which the 5s timeout was masking. It clicked "Save Changes" after only awaiting the "Household Details" heading. The heading renders as soon as loading flips, but a follow-up effect populates the name input -- and that input is `required`, so a click in between is swallowed by HTML5 form validation: no submit handler, no updateHousehold call, no error to find. Under CI parallel load the window widens enough to hit. It now awaits the populated value, matching the sibling save test that never flaked. Evidence: the full suite failed ~1 run in 6 before (in-container, and isolated runs of the file alone passed 10/10, confirming it was load-dependent); it passes 12/12 after. Also repoints a stale sonar coverage exclusion at the surviving tests/ directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jenkins has been red on
HouseholdSettings > "surfaces a save error". Two separate bugs were stacked here, and the first one hid the second.1. PR #63's timeout fix never took effect
#63 set
testTimeout/hookTimeout: 20000infrontend/vite.config.ts. But this repo also hasfrontend/vitest.config.ts, and Vitest resolvesvitest.config.*in preference tovite.config.*. Thattestblock has therefore always been dead config that merely looks authoritative — the effective timeout was still the 5000ms default, which is why the CI log still said:Verified rather than assumed, with a probe test that sleeps 7s:
FAIL — Test timed out in 5000ms✓ probe (7002ms)The timeouts move to
vitest.config.ts. They have to stay above theasyncUtilTimeout: 15000thattests/setup.tsconfigures — Vitest kills the test attestTimeoutregardless of whatfindBy*/waitForare willing to wait for, so a lower value silently caps that budget. That was the actual misconfiguration: a 15s async-util budget inside a 5s test.The dead
testblock and its orphanedsrc/test/setup.ts(referenced by nothing else) are removed, so the next person can't repeat the same fix in the wrong file.2. The test had a real race, which the 5s timeout was masking
With the timeout raised, the failure changed from a timeout to:
The test clicked "Save Changes" after awaiting only the
Household Detailsheading. The heading renders as soon asloadingflips, but a follow-up effect populates the name input — and that input isrequired:So a click landing in that window is silently swallowed by HTML5 form validation:
onSubmitnever fires,updateHouseholdis never called, no error is ever set, and the assertion burns its full 15s. Under CI parallel load the window widens enough to hit.It now awaits the populated value — the same pattern the sibling
saves household changes and shows successtest already uses, which is why that one never flaked.Evidence
npm run test:coveragestill emitscoverage/lcov.info(151KB) — Sonar's frontend coverage input is unaffected.Also repoints a stale
sonar.coverage.exclusionsentry at the survivingfrontend/tests/**directory.Why this is separate
This is CI infrastructure, unrelated to the epic #43 work in #66/#67 — but it's what's currently reddening those PRs, so it wants to merge first.
🤖 Generated with Claude Code