test(web): give the InvoiceWorkspace queued-Issue tests real waitFor headroom (#3219) - #3284
Open
bdunncompany wants to merge 1 commit into
Open
test(web): give the InvoiceWorkspace queued-Issue tests real waitFor headroom (#3219)#3284bdunncompany wants to merge 1 commit into
bdunncompany wants to merge 1 commit into
Conversation
…headroom (LanternOps#3219) Three sightings in ten days — LanternOps#2925 (2026-07-29), LanternOps#3219, and LanternOps#3277 on LanternOps#3264's CI two days later — always the same two tests in this file, always passing locally and on a same-commit rerun. They are the only two tests here that assert the END of a multi-hop propagation chain: the PATCH promise settles, the editor reports saved/failed, the workspace clears `savePending`, the header un-gates, and only then does the queued Issue fire a fetch. Every hop is promise/render scheduling, so under CI load the whole chain can outrun Testing Library's 1000ms default `waitFor` timeout. All three recorded failures sat at ~1038ms, a fraction over that default, which is the signature of a timeout rather than a logic race. Fake timers were the other candidate and are the wrong instrument: nothing in this chain is timer-driven. The delay is promise microtasks plus React render scheduling, neither of which fake timers advance, so they would add machinery without touching the cause. Raised file-wide via `configure({ asyncUtilTimeout })` rather than per-assertion, so a future test in this file inherits the headroom instead of rediscovering the flake, and restored in `afterAll` because that setting is process-global. Verification, stated plainly: I could NOT reproduce the flake locally — the chain completes in ~30ms here against the 1000ms default, which is exactly why this only ever fails on loaded CI. What I could prove is that the knob governs the right assertions: setting `asyncUtilTimeout` to 1ms fails EXACTLY the two queued-Issue tests and leaves the other five passing. Scope note: no other web test file configures `asyncUtilTimeout`, so the whole suite runs on the 1000ms default. Deliberately NOT raised globally — that would also slow every genuine failure to its new ceiling. Worth considering separately if this shape shows up elsewhere. Gate: apps/web vitest 538 files / 5102 tests / 0 failures; tsc --noEmit exit 0; eslint clean.
Collaborator
Author
|
Fourth sighting, today. Same test, on a PR that cannot possibly have caused it.
Run 31312800729, 12:25Z. That takes the table in the PR body to:
Sighting four also strengthens the diagnosis rather than just repeating it: an API-only diff has no mechanism to perturb this component, so whatever moved was the runner's scheduling, which is exactly the ~1038ms-against-a-1000ms-default timeout story in the PR body. A same-commit rerun is green again, as before. The cost is no longer theoretical — it now reddens unrelated PRs and forces a rerun to clear them, which is the same tax #3018 charges on the Windows agent side. No competing PR exists for this (checked), and #3219 and #3277 are both still open and unassigned. |
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
1 participant
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.
Fixes #3219. Also covers #3277, which is the same two tests — worth closing as a duplicate once this lands.
Why these two tests and no others
Three sightings in ten days, always the same pair, always green locally and on a same-commit rerun:
does NOT fire a queued Issue when the pending save failsholds a header Issue click…They are the only two in the file that assert the end of a multi-hop propagation chain: the PATCH promise settles → the editor reports saved/failed → the workspace clears
savePending→ the header un-gates → the queued Issue fires a fetch. Every hop is promise/render scheduling.All three recorded failures sat at ~1038ms against Testing Library's 1000ms default
waitFortimeout. A fraction over the default is the signature of a timeout, not a logic race — and it explains why a rerun on the identical commit passes.Why not fake timers
That was your other suggested option and it's the wrong instrument here: nothing in this chain is timer-driven. The delay is promise microtasks plus React render scheduling, neither of which fake timers advance. They'd add machinery without touching the cause, and would need the manual
resolvePatch/rejectPatchpromises reworked around them.Raised file-wide as you specified, rather than per-assertion, so a future test in this file inherits the headroom instead of rediscovering the flake. Restored in
afterAllsinceconfigureis process-global.What I verified — and what I couldn't
Could not reproduce the flake locally. The chain completes in ~30ms on my machine against a 1000ms budget, which is precisely why this only ever fails on loaded CI. I'm not going to claim a green local run as proof of a fix for a CI-only timing failure.
What I could prove is that the knob governs the right assertions rather than being inert — setting
asyncUtilTimeoutto 1ms fails exactly the two queued-Issue tests and leaves the other five passing:The real proof is CI staying green on these two over the next few weeks; if it recurs, the diagnosis is wrong and the next step is instrumenting the chain rather than raising the number again.
Scope note
No other web test file configures
asyncUtilTimeout— the whole suite runs on the 1000ms default. I deliberately did not raise it globally: that would also slow every genuine failure to the new ceiling, and the blast radius is the entire web suite. Worth considering separately if this shape shows up in other files.Gate
apps/webvitest — 538 files / 5102 tests / 0 failures ·tsc --noEmitexit 0 ·eslintclean.