Skip to content

test(tui): remove fixed synchronization waits - #10988

Merged
lawrencecchen merged 3 commits into
mainfrom
fix-tui-test-sync-wave69
Aug 27, 2026
Merged

lawrencecchen merged 3 commits into
mainfrom
fix-tui-test-sync-wave69

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary:

  • scale browser and terminal screen polling deadlines with CI timeout helpers
  • wait for the subscribe response before creating the tab in the tree-change test

Validation:

  • rustfmt --edition 2024 --check on all three changed test files
  • git diff --check
  • hosted cmux-tui verification pending

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Removes fixed synchronization waits in TUI tests so they no longer time out or flake under slow CI.

  • Polling deadlines now scale with CI timeout helpers, applied once inside shared wait helpers so callers pass base durations.
  • The wait helpers compare against an absolute deadline, stopping promptly at the boundary instead of sleeping past it.
  • The tree-change test waits for the server to acknowledge the subscribe request before creating the tab, replacing a fixed 200 ms sleep.
  • Terminal host recovery waits for screens and records now scale with the CI timeout.

Written for commit e12ce40. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Tests
    • Improved test reliability by waiting for explicit subscription acknowledgements instead of relying on fixed delays.
    • Applied configurable timeout scaling consistently across runtime and terminal recovery checks.
    • Made timeout handling more precise to avoid unnecessary waiting at deadline boundaries.
    • Reduced timing-related flakiness by using deadline-bounded waits and ensuring pending output is flushed before validation.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b97d007a-28e2-4e75-acc2-38304e950a5d

📥 Commits

Reviewing files that changed from the base of the PR and between f781332 and 8a2f990.

📒 Files selected for processing (1)
  • cmux-tui/crates/cmux-tui/tests/terminal_host_recovery.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The changes scale browser and terminal test deadlines. The CLI subscription test now waits for a flushed server acknowledgement instead of using a fixed delay.

Changes

Test timing reliability

Layer / File(s) Summary
Scaled polling deadlines
cmux-tui/crates/cmux-tui-core/tests/browser_runtime.rs, cmux-tui/crates/cmux-tui/tests/terminal_host_recovery.rs
Polling deadlines now use the configured test timeout scale. Browser timeout checks return when the current time reaches the deadline.
Subscription acknowledgement synchronization
cmux-tui/crates/cmux-tui/tests/cli.rs
The subscription test flushes the request, reads channel messages until acknowledgement id 1 arrives, and asserts success. The fixed 200 ms sleep is removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 8a2f9

This PR only improves synchronization in TUI tests by replacing fixed waits with deadline-based polling and acknowledgement ordering; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: removing fixed synchronization waits from TUI tests.
Description check ✅ Passed The description explains the main changes and lists validation steps. It omits several template sections, including the checklist, review trigger, and demo video, but the core summary and testing info…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed PASS: The pull request changes only three Rust test files. The range from origin/main to HEAD contains no Swift files and no production Swift changes. Therefore, the Swift actor-isolation check does n…
Cmux Swift Blocking Runtime ✅ Passed The PR changes only three Rust test files under tests/; it introduces no Swift files or production Swift changes. The added deadline, recv_timeout, thread::sleep usage, and writer flush are test…
Cmux Browser Automation Off-Main ✅ Passed PASS: The PR changes only three Rust TUI test files: cmux-tui-core/tests/browser_runtime.rs, cmux-tui/tests/cli.rs, and cmux-tui/tests/terminal_host_recovery.rs. The origin/main..HEAD diff con…
Cmux Expensive Synchronous Load ✅ Passed PASS: The pull request changes only three Rust test files: cmux-tui/crates/cmux-tui-core/tests/browser_runtime.rs, cmux-tui/crates/cmux-tui/tests/cli.rs, and `cmux-tui/crates/cmux-tui/tests/termin…
Cmux Cache Substitution Correctness ✅ Passed PASS: The custom check applies only to production Swift, TypeScript, and JavaScript changes. The pull request changes only three Rust test files: browser_runtime.rs, cli.rs, and `terminal_host_rec…
Cmux No Hacky Sleeps ✅ Passed PASS: The PR changes only three Rust test files under cmux-tui/.../tests. The checked rule applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. The diff introduces no cover…
Cmux Algorithmic Complexity ✅ Passed PASS: The PR changes only three Rust files under cmux-tui/**/tests/. The changes update test polling deadlines and test synchronization. The algorithmic-complexity rule explicitly passes test-only s…
Cmux Swift Concurrency ✅ Passed PASS: The pull-request diff changes only three Rust test files under cmux-tui, with zero changed Swift files. Therefore, it introduces no cmux-owned Swift concurrency pattern covered by the check.
Cmux Swift @Concurrent ✅ Passed PASS: The pull request changes only three Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. The diff contains no Swift paths or Swift concurrency annotations. The Swift…
Cmux Swift Package Boundaries ✅ Passed PASS: The PR diff from origin/main to HEAD changes only three Rust test files. It introduces no Swift, SwiftPM, Xcode project, or package manifest changes. The Swift package-boundaries check is theref…
Cmux Swiftpm Lockfiles ✅ Passed PASS: The PR diff contains only three Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. It contains no Package.swift, Package.resolved, .gitignore, workflow, or X…
Cmux Swift Logging ✅ Passed PASS: The PR diff contains only three Rust test files and no changed Swift paths. The changes adjust Rust timeout polling and subscription synchronization; they do not add or materially change Swift l…
Cmux User-Facing Error Privacy ✅ Passed PASS — the complete PR diff against main changes only three Rust test files. The changes update test timeout calculations and test subscription synchronization. They do not modify production user-fa…
Cmux Full Internationalization ✅ Passed PASS: The complete diff from origin/main to HEAD changes only three Rust test files. The changes adjust test deadlines and subscription-test synchronization. The custom check explicitly allows tes…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request changes only three Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. The combined diff contains no Swift or SwiftUI paths and introduces no Swift…
Cmux Architecture Rethink ✅ Passed PASS: The pull request changes only three Rust test files. It introduces no Swift architecture changes. The new deadline loop and acknowledgement wait are test-only synchronization, which the rule exp…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The pull request diff changes only three Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. It introduces no Swift NSWindow, NSPanel, NSWindowController, Swi…
Cmux Source Artifacts ✅ Passed PASS. The diff against origin/main changes only three existing Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. The changes are test synchronization and timeout logic. No lo…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The PR changes only three Rust test files under cmux-tui/crates/.../tests/: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. The diff adds no Swift file under a production `**/…
Cmux No Ambient Global State ✅ Passed PASS: The pull request changes only three Rust test files (.rs). The committed PR-range diff contains no Swift files and no production Swift changes. The custom check applies only to production Swif…
Full details: Description check

Explanation

The description explains the main changes and lists validation steps. It omits several template sections, including the checklist, review trigger, and demo video, but the core summary and testing information are present.

Full details: Cmux Swift Actor Isolation

Explanation

PASS: The pull request changes only three Rust test files. The range from origin/main to HEAD contains no Swift files and no production Swift changes. Therefore, the Swift actor-isolation check does not apply.

Full details: Cmux Swift Blocking Runtime

Explanation

The PR changes only three Rust test files under tests/; it introduces no Swift files or production Swift changes. The added deadline, recv_timeout, thread::sleep usage, and writer flush are test synchronization and fall under the check's allowed deterministic test-only scaffolding.

Full details: Cmux Browser Automation Off-Main

Explanation

PASS: The PR changes only three Rust TUI test files: cmux-tui-core/tests/browser_runtime.rs, cmux-tui/tests/cli.rs, and cmux-tui/tests/terminal_host_recovery.rs. The origin/main..HEAD diff contains no changes to Sources/TerminalController.swift, ControlCommandExecutionPolicy.swift, socketWorkerMethods, processV2Command, or worker browser routing. Therefore, this PR introduces no browser socket automation routing change covered by the rule.

Full details: Cmux Expensive Synchronous Load

Explanation

PASS: The pull request changes only three Rust test files: cmux-tui/crates/cmux-tui-core/tests/browser_runtime.rs, cmux-tui/crates/cmux-tui/tests/cli.rs, and cmux-tui/crates/cmux-tui/tests/terminal_host_recovery.rs. The diff relative to origin/main contains no Swift paths and no production Swift changes. Therefore it does not introduce or move any synchronous agent-history load covered by this check.

Full details: Cmux Cache Substitution Correctness

Explanation

PASS: The custom check applies only to production Swift, TypeScript, and JavaScript changes. The pull request changes only three Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. The diff updates test deadlines and subscription synchronization; it does not replace an authoritative read with a cache in a persistence, history, undo, or snapshot path.

Full details: Cmux No Hacky Sleeps

Explanation

PASS: The PR changes only three Rust test files under cmux-tui/.../tests. The checked rule applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. The diff introduces no covered production runtime change. The timing loops and sleeps are test-only scaffolding, which the rule explicitly allows; the subscribe test also replaces a fixed test sleep with an acknowledgement event.

Full details: Cmux Algorithmic Complexity

Explanation

PASS: The PR changes only three Rust files under cmux-tui/**/tests/. The changes update test polling deadlines and test synchronization. The algorithmic-complexity rule explicitly passes test-only scaffolding, and no production Swift, TypeScript, JavaScript, shell, or runtime path changed.

Full details: Cmux Swift `@Concurrent`

Explanation

PASS: The pull request changes only three Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. The diff contains no Swift paths or Swift concurrency annotations. The Swift @concurrent check is therefore not applicable.

Full details: Cmux Swift Package Boundaries

Explanation

PASS: The PR diff from origin/main to HEAD changes only three Rust test files. It introduces no Swift, SwiftPM, Xcode project, or package manifest changes. The Swift package-boundaries check is therefore not applicable.

Full details: Cmux Swiftpm Lockfiles

Explanation

PASS: The PR diff contains only three Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. It contains no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project changes. The SwiftPM lockfile policy is therefore not applicable.

Full details: Cmux Swift Logging

Explanation

PASS: The PR diff contains only three Rust test files and no changed Swift paths. The changes adjust Rust timeout polling and subscription synchronization; they do not add or materially change Swift logging. Therefore the Swift logging failure conditions do not apply.

Full details: Cmux User-Facing Error Privacy

Explanation

PASS — the complete PR diff against main changes only three Rust test files. The changes update test timeout calculations and test subscription synchronization. They do not modify production user-facing errors, alerts, command output, API error bodies, or recovery copy. The custom check explicitly passes tests.

Full details: Cmux Full Internationalization

Explanation

PASS: The complete diff from origin/main to HEAD changes only three Rust test files. The changes adjust test deadlines and subscription-test synchronization. The custom check explicitly allows tests, and the diff introduces no Swift, catalog, web, API, markdown, changelog, or user-facing production text changes.

Full details: Cmux Swiftui State Layout

Explanation

PASS: The pull request changes only three Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. The combined diff contains no Swift or SwiftUI paths and introduces no SwiftUI state, geometry measurement, lazy-row store reference, or render-time mutation. The SwiftUI state-layout check is therefore not applicable.

Full details: Cmux Architecture Rethink

Explanation

PASS: The pull request changes only three Rust test files. It introduces no Swift architecture changes. The new deadline loop and acknowledgement wait are test-only synchronization, which the rule explicitly allows. The existing polling helpers remain test code and are only adjusted for timeout scaling; no Swift owner, state, lifecycle, or entrypoint wiring changes are present.

Full details: Cmux Swift Auxiliary Window Close Shortcuts

Explanation

PASS: The pull request diff changes only three Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. It introduces no Swift NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowGroup code. The auxiliary-window close-shortcut rule is therefore not applicable.

Full details: Cmux Source Artifacts

Explanation

PASS. The diff against origin/main changes only three existing Rust test files: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. The changes are test synchronization and timeout logic. No logs, screenshots, recordings, caches, build output, temp directories, dependency checkouts, or other artifact paths enter source control. All three files remain regular 100644 files.

Full details: Cmux No Test Or Debug Seam In Production Source

Explanation

PASS: The PR changes only three Rust test files under cmux-tui/crates/.../tests/: browser_runtime.rs, cli.rs, and terminal_host_recovery.rs. The diff adds no Swift file under a production **/Sources/** path, so the no-test-or-debug-seam-in-production-source check is not applicable.

Full details: Cmux No Ambient Global State

Explanation

PASS: The pull request changes only three Rust test files (.rs). The committed PR-range diff contains no Swift files and no production Swift changes. The custom check applies only to production Swift ambient global state, so its failure conditions are not applicable.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-tui-test-sync-wave69

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lawrencecchen
lawrencecchen force-pushed the fix-tui-test-sync-wave69 branch from f781332 to ac9066d Compare August 27, 2026 19:31
@lawrencecchen
lawrencecchen force-pushed the fix-tui-test-sync-wave69 branch from 8a2f990 to e12ce40 Compare August 27, 2026 20:03
@lawrencecchen
lawrencecchen merged commit e7584a4 into main Aug 27, 2026
41 checks passed
@lawrencecchen
lawrencecchen deleted the fix-tui-test-sync-wave69 branch August 27, 2026 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant