Repository navigation
test(webviews): U1 waits for conditions, not fixed times (114 waits) - #23416
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
Removes the fixed sleeps from the U1 batch's 20 unit-test files. - Hover and dwell delays (session hover card, sidebar preview, minimap turn preview, URL chip grace, settings backdrop and theme dwell, scroll anchor settle, warm New Tab post, prompt delivery deadlines) run on bun:test's fake timers: each test checks the behavior is absent one tick before its delay and present at it, so a slow runner cannot change the result. - Async results (shell reads, sign-in polls, client reconnects and resyncs, pasted image chips, KaTeX and diagram rendering, the code-card handoff, changelog and settings host calls, menu picks) are awaited with bounded `until` loops that yield with setTimeout 0 and fail naming what they waited for. - The settings claim-paint test counts calls and stream events in flight and waits until all are delivered, rather than waiting 300 ms. - The reveal frame helper in transcript-switch-back is left for the shared post-paint frame 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
#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/u1 # 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
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
Not re-run automatically: Written by red: web / Web status; NEW: green on the base bc5ce39; do not merge until green; see lane-rules
Agents: Tour of |
Collaborator
Author
|
Bead |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light.svg)
…its/u1 # Conflicts: # webviews/test/session-sidebar-preview.test.tsx
# Conflicts: # webviews/scripts/fixed-waits-baseline.tsv
Collaborator
Author
|
|
…its/u1 # Conflicts: # webviews/scripts/fixed-waits-baseline.tsv
Collaborator
Author
|
Owned by window-focus-opacity on huge-blue · session |
Collaborator
Author
|
Review (subagent): SHIP at b3b55b4 (re-review of the base merges: same test change; the baseline only lowers this PR's own rows; check:fixed-waits passes). Reds aren't this PR's: check:browser-lanes on test/diff-paint-perf.test.ts (from #23154, fixed on base by #23480), defaults-snapshot (base), and gallery health (it renders app source; this PR changes test files only). fast gates follows from these. |
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 114 fixed-time waits from 20 webview unit-test files, counted against current feat-cmux-next.
untilloops. Each loop yields withsetTimeout 0and fails with a message naming what it waited for. This covers shell reads, sign-in polls, client reconnects and resyncs, pasted image chips, KaTeX and diagram rendering, the code-card handoff, changelog and settings host calls, and menu picks.transcript-switch-back.test.tsxwill move to the shared post-paint helper from webviews tests: frame tests read layout after the frame paints #23374.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(878 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 114 fixed-time waits from 20 webview unit-test files, so a loaded runner that wakes a wait late can no longer change a test's result.
untilloops that yield withsetTimeout 0and fail naming what they waited for.transcript-switch-back.test.tsxstay for a follow-up to the shared post-paint helper, and the baseline ratchet is lowered accordingly.Written for commit b3b55b4. Summary will update on new commits.