Skip to content

fix(tests): close radiogroup re-query race in fillRadio helper - #1312

Merged
gabrielseco merged 1 commit into
mainfrom
fix/contractor-onboarding-flaky-test
Sep 14, 2026
Merged

gabrielseco merged 1 commit into
mainfrom
fix/contractor-onboarding-flaky-test

Conversation

@gabrielseco

@gabrielseco gabrielseco commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Investigates a reported flaky failure in ContractorOnboarding.test.tsx > should invite the contractor when the user clicks on the 'Invite Contractor' button, where fillRadio failed to find the Payment terms radiogroup even though the surrounding waitForElementToBeRemoved(spinner) had already resolved.
  • Root cause: both fillRadio and assertRadioValue in src/tests/testHelpers.tsx waited for the radiogroup via waitFor(() => screen.getByRole('radiogroup', ...)), then discarded that result and re-queried synchronously with a second, non-retrying screen.getByRole(...) call. If the radiogroup briefly unmounted again between those two calls (e.g. a step re-entering a loading state from a second async fetch after the first resolves), the second call throws even though the first had just succeeded moments earlier — a genuine race window baked into the helper itself, not the test.
  • Fix: both helpers now keep the element waitFor() already resolved instead of re-querying it, closing the race window entirely. This affects every test in the suite that uses fillRadio/assertRadioValue (widely used across flows), not just the one that happened to be reported.

Investigation notes

  • Could not reproduce the failure directly: ran the single test 3x in isolation, the full file 3x, and the full suite 2x — all green (958/958). The failure is timing-dependent (CI load/scheduling), consistent with the race described above.
  • Confirmed via code reading that the redundant re-query pattern is present in both fillRadio (used in the failing test) and assertRadioValue (same pattern, same fix applied for consistency).
  • waitFor() already returns whatever its callback returns, so capturing the resolved element removes the redundant, racy second query without changing any test's behavior.

Test plan

  • npx vitest run — full suite passes (95 files / 958 tests)
  • npm run type-check — clean
  • npm run lint — no new warnings (pre-existing example/ console warnings only)
  • npm run check-format — clean

🤖 Generated with Claude Code


Note

Low Risk
Test-only helper change with no production code impact; reduces flaky CI failures without altering intended test behavior.

Overview
Fixes a timing race in the shared test helpers assertRadioValue and fillRadio in testHelpers.tsx.

Both helpers used to call waitFor to find a radiogroup, then immediately run a second synchronous screen.getByRole('radiogroup', …). If the group unmounted again between those steps (e.g. another loading pass), the second lookup could fail even though waitFor had just succeeded. The change reuses the element returned from waitFor and drops the redundant re-query, with a short comment explaining why.

Behavior for callers is unchanged; the goal is fewer flaky failures in tests that interact with radiogroups (including contractor onboarding flows).

Reviewed by Cursor Bugbot for commit 0843f32. Bugbot is set up for automated code reviews on this repo. Configure here.

fillRadio/assertRadioValue waited for a radiogroup via waitFor(), then
discarded that result and re-queried synchronously with getByRole. If
the group briefly unmounted again between those two calls (e.g. a step
re-entering a loading state from a second async fetch), the second
call threw even though the first had just succeeded - the likely cause
of the flaky ContractorOnboarding "Invite Contractor" test failure.
Both helpers now keep the element waitFor() already resolved instead
of re-querying.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📦 Bundle Size Report

Metric Current Previous Change Status
Total (gzip) 272.53 kB 272.53 kB +1 B (+0.0%) 🔴
Total (raw) 721.77 kB 721.77 kB 0 B (0%) 🟢
CSS (gzip) 21.77 kB 21.77 kB 0 B (0%) 🟢
CSS (raw) 113.22 kB 113.22 kB 0 B (0%) 🟢

Size Limits

  • ✅ Total gzipped: 272.53 kB / 350 kB (77.9%)
  • ✅ Total raw: 721.77 kB / 850 kB (84.9%)
  • ✅ CSS gzipped: 21.77 kB / 25 kB (87.1%)

Largest Files (Top 5)

  1. chunk-SRLBVFNQ.js - 12.94 kB (0 B (0%))
  2. styles.css - 10.88 kB (0 B (0%))
  3. index.css - 10.88 kB (0 B (0%))
  4. index.js - 6.96 kB (+1 B (+0.0%))
  5. chunk-DAUDKN32.js - 6.58 kB (0 B (0%))
View All Files (450 total)
File Size (gzip) Change
chunk-SRLBVFNQ.js 12.94 kB 0 B (0%)
styles.css 10.88 kB 0 B (0%)
index.css 10.88 kB 0 B (0%)
index.js 6.96 kB +1 B (+0.0%)
chunk-DAUDKN32.js 6.58 kB 0 B (0%)
chunk-MNHXHKTE.js 6.44 kB 0 B (0%)
chunk-SMCLTRJW.js 5.84 kB 0 B (0%)
chunk-C4HFMTSB.js 5.33 kB 0 B (0%)
chunk-KSSQR2CS.js 4.69 kB 0 B (0%)
chunk-COQVI5OD.js 4.35 kB 0 B (0%)

✅ Bundle size check passed

@github-actions

Copy link
Copy Markdown
Contributor

📊 Coverage Report

⚪ Coverage unchanged

Metric Current Previous Change Status
Lines 85.98% 85.98% 0% ⚪
Statements 85.54% 85.54% 0% ⚪
Functions 84.37% 84.37% 0% ⚪
Branches 77.28% 77.28% 0% ⚪

Detailed Breakdown

Lines Coverage
  • Covered: 4624 / 5378
  • Coverage: 85.98%
  • Change: 0% (0 lines)
Statements Coverage
  • Covered: 4704 / 5499
  • Coverage: 85.54%
  • Change: 0% (0 statements)
Functions Coverage
  • Covered: 1225 / 1452
  • Coverage: 84.37%
  • Change: 0% (0 functions)
Branches Coverage
  • Covered: 2853 / 3692
  • Coverage: 77.28%
  • Change: 0% (0 branches)

✅ Coverage check passed

@github-actions

Copy link
Copy Markdown
Contributor

Deploy preview for adp-cost-calculator ready!

Project:adp-cost-calculator
Status: ✅  Deploy successful!
Preview URL:https://adp-cost-calculator-dfuqdwctg-remotecom.vercel.app
Latest Commit:0843f32

Deployed with vercel-action

@github-actions

Copy link
Copy Markdown
Contributor

Deploy preview for remote-flows ready!

Project:remote-flows
Status: ✅  Deploy successful!
Preview URL:https://remote-flows-oj0cl7ilw-remotecom.vercel.app
Latest Commit:0843f32

Deployed with vercel-action

@gabrielseco gabrielseco self-assigned this Sep 11, 2026
@gabrielseco
gabrielseco merged commit 8aeb9bd into main Sep 14, 2026
14 checks passed
@gabrielseco
gabrielseco deleted the fix/contractor-onboarding-flaky-test branch September 14, 2026 09:03
@gabrielseco gabrielseco mentioned this pull request Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants