Skip to content

test(web): settle the lazy Guidance tab inside act() in SettingsModal guidance test - #176

Merged
ChesterRa merged 2 commits into
ChesterRa:mainfrom
chriscoveries:fix/web-settingsmodal-act-v2
Oct 23, 2026
Merged

ChesterRa merged 2 commits into
ChesterRa:mainfrom
chriscoveries:fix/web-settingsmodal-act-v2

Conversation

@chriscoveries

Copy link
Copy Markdown
Contributor

SettingsModal.guidance.test.tsx › "keeps expanded Guidance 0 keyboard focus and Escape in the child dialog" fails intermittently on CI with AssertionError: expected undefined to be defined at line 98 after about 1.39 s. Only case 0 fails. Case 1, the same code path, passes in about 100 ms. The failing case logs React's "A suspended resource finished loading inside a test, but the event was not wrapped in act(...)".

Root cause. SettingsModal loads GuidanceTab with React.lazy. In case 0 the chunk is still loading when the render's act() scope exits. When it resolves, no act() scope is open. React then commits the tab, runs its mount effect and its prompt load on the Scheduler, while the test polls with vi.waitFor. The test's readiness check (two textareas) passes before the tab has settled: the textareas render before the prompt load finishes, and the expand button stays disabled while the load runs. Its follow-up renders also stay off act(). On a loaded runner the portaled expanded dialog is then not in the document within vi.waitFor's 1000 ms default. Case 1 never suspends because the lazy component is already resolved.

Fix (test-only). Wait for the tab in back-to-back act() scopes instead of vi.waitFor. An act() scope is then open whenever the chunk resolves, and the tab's retry render, effects and prompt load are flushed by act() before the click. A single act() around vi.waitFor does not work, because act() flushes queued work only when its outermost scope exits. The DOM would never update while polling. Readiness now has a 4000 ms deadline followed by an exact two-textarea assertion, so a lazy-chunk failure reaches that assertion before the default 5 s test timeout.

Why not a longer timeout. Raising the vi.waitFor timeout only widens the window for Scheduler work to land. The test would still depend on wall-clock timing, would fail more slowly when it fails, and would hide that the tab mounts outside act().

Verification. The test-only red run (39806277470, using a reference commit based on upstream main at f1d27838) forces the lazy chunk to resolve after render act(): current fails and fixed passes (the Web job reports 1 failed and 2,912 passed). The exact fix head de391350e9383881fd412d6de7a0efad12248b53 passed all six GitHub Actions jobs on the fork, including rust-linux (run 39820886384). The gated tests remain on the fork, not in this PR.

Test environment: GitHub Actions CI on the fork (ubuntu-latest, Node 24.19.0, npm -C web run check and npm -C web test).

… guidance test

The first it.each case renders SettingsModal while the lazily imported
GuidanceTab chunk is still loading. The render's act() scope exits with the
Suspense boundary pending, so the chunk resolves outside act(): React logs
"A suspended resource finished loading inside a test, but the event was not
wrapped in act(...)" and the tab's retry render, its mount effect and its
prompt load run on the Scheduler while the test polls with vi.waitFor. On a
loaded runner the portaled expanded dialog is then not found within
vi.waitFor's 1000 ms budget. The second case is unaffected because the lazy
component is already resolved.

Wait for the tab in back-to-back act() scopes instead of vi.waitFor, so an
act() scope is open whenever the chunk resolves and every follow-up render
and effect is flushed by act() before the click. A single act() scope around
vi.waitFor would not work: act() flushes queued React work only when its
outermost scope exits, so the DOM would never update while polling.

Test-only change; no product code is touched.
@chriscoveries

Copy link
Copy Markdown
Contributor Author

CI note: on current main, rust-linux fails the five real-Chrome fixtures added in 1e5651b2 on every pull request. #175 moves them to the nightly job.

@ChesterRa
ChesterRa merged commit 4d340aa into ChesterRa:main Oct 23, 2026
4 of 6 checks passed
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.

2 participants