Repository navigation
test(webviews): U2 waits for conditions, not fixed times (157 waits) - #23415
Merged
Merged
Conversation
…soned marker Fails until scripts/check-fixed-waits.ts exists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
check:fixed-waits (in `bun run check`) names each test wait for a fixed number of ms: page.waitForTimeout, Bun.sleep, a setTimeout that resolves a promise after a non-zero delay, and calls to a local helper built on one. A fixed wait passes on an idle machine and fails on a loaded runner that wakes it late, or hides the bug it waits out (#23323). A test waits for a condition instead, uses fake timers when the timing is the behavior, or keeps the wait with a `// fixed-wait: <reason>` comment. The 398 waits from before the rule have per-file counts in scripts/fixed-waits-baseline.tsv. A file fails with more unmarked waits than its row, and with fewer until the row is lowered (--update-baseline), so the count only moves down; the sweep PRs lower it. pageSwitch's 16 ms no-requestAnimationFrame fallback keeps its wait with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
…imers and exported helpers Fails on the first detector: a helper with no delay parameter counted only at its definition, a helper passed as a value (act(settle)) not at all, a timer callback with a block body was missed, and so was an exported helper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
From review. A wait inside a named function makes it a helper, and each use
counts instead of the definition: a call, or the helper passed or returned as a
value (act(settle), return { open }), applied outward through helpers built on
helpers. So another `await settle()` in a file raises its count. A helper whose
delay parameter defaults to 0 waits only when a use passes a delay. An exported
helper also counts at its definition, since its callers are in other files.
Also caught: timer callbacks with a block body that resolve the promise,
window/globalThis.setTimeout and Bun.sleepSync. A marker counts anywhere in the
wait's statement, or on a comment-only line above it, so a trailing marker no
longer covers the next line. The scan covers every source under test/
(harnesses, latency helpers), not just *.test files, and skips fixtures. A
malformed baseline row now fails instead of passing as NaN.
The baseline is regenerated: 819 waits in 132 files.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
…ixed-wait count Review of #23365 found that a second wait or helper call added inside a function that already waits was not counted. These cases fail on the current detector. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
…lpers too A wait counted only through the outer uses of its enclosing helper, so a new wait or helper call inside a helper that already waited changed nothing. Now each unmarked wait site counts, and each use of a helper outside its own body, so any new wait raises the count. Waits in exported helpers count at the site, which replaces the export rule. Export lists, typeof and member declaration names are no longer uses. The baseline is regenerated: 989 waits in 132 files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
The 19 U2 unit-test files had 142 fixed waits under the stricter detector; none are left and their baseline rows are gone. - Timing under test runs on bun's fake timers: the new tab's picked-name fade, the summary card's running-time tick, the kept-attachments rest timer, the transcript's anchor settle, the tooltip's scroll quiet time (now also checked inside the quiet window) and the link bubble's no-hover-card check. - Polls that already waited on a condition (render card, dropped chips, KaTeX, prompt accepted) yield a macrotask per try instead of sleeping. - The image viewer's actions wait for the host request (or the refusal label); the changelog pages wait for their release to draw. - Menu and composer settle() helpers and PreviewCard's teardown yield one macrotask: act() flushes the rest, and the 20 ms were slack. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
…its/u2 # Conflicts: # webviews/scripts/fixed-waits-baseline.tsv
…erts The hover test now lets promises and a later task run before it asserts no bubble, and the image viewer waits one more task so a duplicate request would show. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
#23374 merged after the fixed-wait ratchet (#23365) ran CI, and it added a 3 s waitForTimeout in two tests, which puts feat-cmux-next over its baseline. Both waits are observation windows: the test records every painted frame for 3 s after the chat opens and asserts none jumped, so the time is the test. They carry a fixed-wait marker instead of a raised baseline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
…its/u2 # Conflicts: # webviews/scripts/fixed-waits-baseline.tsv
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c78-27ca-7140-a6c7-95f0db83b18c&host=huge-blue
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
Contributor
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Gallery before/after for
States
Pick other states with a Unclear: CI failed on
Failures:
Matched log linesNot re-run automatically: Written by red: web / Web status; NEW: green on the base 0b1a93e; do not merge until green; see lane-rules
Agents: Tour of |
Collaborator
Author
|
Bead |
|
Found 4 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light.svg)
Collaborator
Author
|
Review (subagent): SHIP at ae9fa29. Reds aren't this PR's own: composer-draft-wrap timeout (base flake), agent-pane-bottom-stack and settings claimPaint (files this PR doesn't touch; it changes test files only), defaults-snapshot (base, FileExplorerSectionView.swift). |
teamleaderleo
added a commit
that referenced
this pull request
Oct 21, 2026
summary-background-work.test.tsx keeps this PR's header comment and base's fake timers (#23415). The webview catalog is base plus this PR's four subagents.ended.* keys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d1f-ca00-7e31-b882-4d3c03033ee2&host=cosmic-gray
Contributor
|
Shipped in:
|
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.
Part of the fixed-wait sweep that followed the ratchet in #23365. This PR removes 157 fixed-time waits from 19 webview unit-test files, and none are left in them. Their rows leave
webviews/scripts/fixed-waits-baseline.tsv.settle()helpers and PreviewCard's teardown yield one macrotask instead of sleeping 20 ms.act()already flushes the rest.It also carries #23414 (the base-red marker fix), which drops out once that merges.
Verification: an independent subagent review said SHIP. Each changed file passed 3 runs of
bun test, andbun run checkpasses, includingcheck:fixed-waits(835 on the baseline).Agent session: huge-blue
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Removes all 157 fixed-time waits from 19 webview unit-test files so tests no longer sleep a fixed number of milliseconds. Timing under test now runs on bun's fake clock, and polls yield one macrotask per try until their condition is met.
settle()helpers and PreviewCard teardown yield one macrotask, withact()flushing the rest.// fixed-waitmarker since the observation window is the test.Written for commit ae9fa29. Summary will update on new commits.