Fix remaining chunkErrorRecovery.spec.ts hydration/guard races under CI shard contention - #399
Merged
Merged
Conversation
…CI shard contention
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.
Symptom
After #397 merged, PR #395's CI (run 30043836352) still failed shard 1/4's "two ChunkLoadErrors in quick succession only trigger one reload attempt" test -
expect.pollgot 0 - across all 3 attempts including retries. Also reproduced locally on this box:chunkErrorRecovery.spec.tsfails intermittently at--workers=4(and occasionally the unhandled-promise-rejection test too), passes reliably at--workers=1.Root causes (two, both fixed)
(a) Guard-clear/dispatch gap: #397 called
clearReloadGuard(page)anddispatchChunkError(page)as two separatepage.evaluate()round-trips. That left a real (if narrow) window for the same class of dev-server on-demand-compilation chunk noise #397's own diagnosis identified (the self-referential "Editor" nav-link prefetch triggering a real recompile ofpages/editor.jsmid-test) to land in and consume the guard again.(b) Hydration race:
loadPageWithDefaultBackend()'s "Choose Art" click is a raw DOM click - Chromium dispatches it regardless of whether React has hydrated anduseChunkErrorRecovery'suseEffecthas actually registered its listeners yet. Under CPU contention (confirmed: PR #395 un-skipped 58 ported parity tests into the same 4 CI shards, raising per-shard dev-server contention - shard composition changed, notplaywright.config.ts'sfullyParallel/workers: undefined, which predate this spec), hydration can still be pending when the test dispatches its synthetic error. A native DOM event dispatched before a listener exists is simply lost - polling longer can't recover it.Fix
page.evaluate()call per test (clearGuardAndDispatchChunkError(Error)AsRejection) - a single synchronous browser-side task nothing else on the page's event loop can interleave with.awaitHydrated(), which retries clicking "Choose Art" (the same established patterntest-utils.ts's ownopenAddCardsDropdown()already uses for the identical symptom) untilProjectEditor.tsx's "editor" tab content - specificallyCardGrid.tsx's "Your project is empty at the moment." empty-state text - actually becomes visible. That can only happen onceTab.Container'sonSelecthandler has bound and fired, i.e. once the same hydration/effect-flush pass that mountsuseChunkErrorRecovery's own listeners has completed. No extra network dispatch or navigation needed.Two earlier attempts at (b) - a
page.route()-intercepted, aborted warm-up reload, and a real uncontrolled warm-up reload followed by re-navigating - were each independently found via local repro to destabilise the page further under the same--workers=4stress (aSecurityError: ... Access is denied for this documenton the aborted-navigation path, and aNavigation ... is interrupted by another navigationon the real-reload path, since the self-referential "Editor" nav-link prefetch is a genuinely recurring background navigation under load, not a one-off). The DOM-visibility approach needed neither.Why the assertions are not weakened
Both guarantees stay exactly as strict: the one-reload assertions (
toBe(1)) are unchanged, and the guard-suppression test still clears the guard only once, before its first dispatch, so its second dispatch genuinely exercises real guard-suppression behaviour (not a cleared one).Verification
npx prettier@2.7.1 --checkon both changed files: passnpx tsc --noEmit: passnpx jest(full suite, 64 suites/568 tests): all passnpx playwright test tests/chunkErrorRecovery.spec.ts --workers=4 --repeat-each=10(cold.nextcache): 40/40, twice--workers=4 --repeat-each=15: 60/60--workers=8 --repeat-each=10(this box's full core count): 40/40--shard=1/4(the exact CI shard this spec lands in - confirmed viaplaywright test --list --shard=1/4), cold cache, default (4) workers, matching real shard composition/contention post-Port import/card-detail/export test clusters onto DisplayPage #395: clean, twice--shard=1/4 --repeat-each=3(246 tests, cold cache): 244 passed, 3 flaky-then-passed-on-retry - all three unrelated to this spec (DisplayFinishFooter.spec.ts,DisplayPage.spec.ts), confirmed pre-existing by reproducing standalone outside this changeDisplayFinishFooter.spec.ts), confirmed pre-existing by re-running that spec alone (6/6 passed)Two earlier local-only reproductions of dead-end fix attempts (the SecurityError and the interrupted-navigation error) are documented in
docs/troubleshooting.mdand in the spec file's own comments, so neither gets re-derived.Docs
Edited
docs/troubleshooting.md's existingchunkErrorRecovery.spec.tsentry in place (repo convention: edit, don't append a dated section) to cover both root causes and both dead ends, plus how to verify any future fix here (--shard=1/4, not the file in isolation).Not merging - per this session's operating rules, PRs are reviewed and merged by the owner. This branch is a fresh worktree off current
origin/master(the prior branch, #397, already merged).