test(ui): poll for the copy failure state instead of a fixed tick budget - #3476
Merged
Conversation
`code-block.test.tsx` failed once in CI on the assistive-technology assertion and passed on re-run, on a branch that touches no React code. `settle` spends a fixed budget of one microtask and one macrotask. That covers the success path, where the clipboard promise resolves and React re-renders. The failure path is longer: a rejected `clipboard.writeText` falls back to `execCommand`, and only once that returns false is the failed state set. On a loaded machine the assertion can run against the pre-failure render, which is why only the two failure-path tests are affected while the success-path ones never flake. Both now poll for the state under test rather than guessing how many ticks it takes. The helper throws a named timeout, so a real regression still fails rather than being waited out. I could not reproduce the flake locally -- not in isolation, not with the whole ui directory in one process, not under coverage -- so this removes a race that is visible in the code rather than one I was able to observe.
📝 WalkthroughWalkthroughThe code-block tests add a polling helper for asynchronous state changes. Clipboard failure assertions now wait for the failed state and accessibility feedback instead of using a fixed delay. ChangesClipboard failure test synchronization
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
kwakayama
enabled auto-merge
August 8, 2026 17:52
kwakayama
approved these changes
Aug 8, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/react/components/ui/code-block.test.tsx`:
- Around line 76-82: Update waitFor to use a named timeout duration and a
deadline-based polling loop instead of the fixed 100-attempt limit. Continue
checking until the deadline is reached, and avoid sleeping after the final
failed predicate check before throwing the existing timeout error.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c97af67-e0bb-432a-82fc-77fc532b07e2
📒 Files selected for processing (1)
src/react/components/ui/code-block.test.tsx
This was referenced Aug 8, 2026
Merged
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.
code-block.test.tsx › exposes failed copy feedback to assistive technologyfailed once in CI on #3474 — a branch that changes one line of CSP caching and no React code — and passed on re-run.The race.
settlespends a fixed budget of one microtask and one macrotask:That covers the success path, where the clipboard promise resolves and React re-renders. The failure path is longer: a rejected
clipboard.writeTextfalls back toexecCommand, and only once that returns false is the failed state set. On a loaded machine the assertion can run against the pre-failure render. It explains the shape of the flake exactly — only the two failure-path tests are exposed, and the success-path ones never flake.The change. Both failure-path tests poll for the state they are actually about instead of guessing a tick count. The helper throws a named timeout, so a genuine regression still fails loudly rather than being waited out — I checked that by pointing the predicate at a sentinel that is never true and confirming it reports
Timed out waiting for the copy control to report failurerather than passing.What I cannot claim. I could not reproduce the flake locally: not in isolation (5/5 green), not with the whole
ui/directory in one process, not under--coverage, and not on a pristineorigin/mainworktree. So this removes a race that is visible in the code, not one I was able to observe. If it recurs, the timeout message will now say which condition was never reached, which is more than the previous failure gave us.Separate from #3474 on purpose — an unrelated test-harness change does not belong in a one-line behavioural fix.
Summary by CodeRabbit