Skip to content

fix(tests): allow bounded main queue drain timeout - #16232

Closed
teamleaderleo wants to merge 1 commit into
manaflow-ai:mainfrom
teamleaderleo:fix/test-drain-main-queue-timeout
Closed

teamleaderleo wants to merge 1 commit into
manaflow-ai:mainfrom
teamleaderleo:fix/test-drain-main-queue-timeout

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Tests added by #15381 call the shared drainMainQueue(timeout:) helper, but the helper only accepted the no-argument form. The app-host test product therefore failed to compile at every timeout call site.

Add an optional timeout parameter while preserving the existing one-second default. This keeps the tests' bounded main-queue spin behavior and restores compilation for all current callers.

Validation:

  • swiftc -parse cmuxTests/TabManagerUnitTests.swift
  • python3 scripts/verify-local.py --only swift-syntax --swift-changed origin/main
  • git diff --check

Summary by cubic

Fixes test compilation by letting drainMainQueue(timeout:) accept an optional timeout parameter while keeping the one-second default, so tests can bound their main-queue drain time.

Written for commit 7a51764. Summary will update on new commits.

Review in cubic

@coderabbitai

coderabbitai Bot commented Sep 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d45f5fac-39e8-46f4-b89a-4da08ed1a711

📥 Commits

Reviewing files that changed from the base of the PR and between d1c9a6f and 7a51764.

📒 Files selected for processing (1)
  • cmuxTests/TabManagerUnitTests.swift
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@github-actions

Copy link
Copy Markdown
Contributor

CI failure attribution

CI failed on 7a51764720 (run 36778620428 attempt 1): 1 code.

Job Verdict Why
macos / macOS compile admission code a compile error
Matched log lines
macos / macOS compile admission: /tmp/cmux-ci/src/CLI/CMUXCLI+AutoNaming.swift:271:16: error: cannot find 'usesTemporaryConfig' in scope

Not re-run automatically: macos / macOS compile admission is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

The fleet compile exposed an independent auto-naming error in the moving main base (usesTemporaryConfig), which is already owned by #16233. This PR remains limited to the shared drainMainQueue(timeout:) test-helper signature; #16233 should land before this PR is rechecked.

@austinywang

Copy link
Copy Markdown
Contributor

main's cmuxTests bundle has three independent compile errors right now: drainMainQueue(timeout:) (#16232), CMUXCLI.vmReadyPollInterval (#16242), and PaneResizeShortcutTests' missing controller binding, dropped in #15420's merge resolution. A PR with only one fix still fails compile admission on the other two. #16245 carries all three, with this PR's commit byte for byte, so whichever lands second merges cleanly. Happy to close #16245 if you'd rather combine them here.

austinywang added a commit that referenced this pull request Sep 30, 2026
The same change as #16232 (7a51764), carried so this PR can restore
main's cmuxTests build in one piece. #15381 made
LastSurfaceClosePreferenceTests and WorkspaceCloseTabsContextMenuTests
call drainMainQueue(timeout:), but the shared helper takes no arguments.

Refs #15488

Co-authored-by: Leo Li <cheerleaderleo@outlook.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 505f346)
lawrencecchen pushed a commit that referenced this pull request Sep 30, 2026
The same change as #16232 (7a51764), carried so this PR can restore
main's cmuxTests build in one piece. #15381 made
LastSurfaceClosePreferenceTests and WorkspaceCloseTabsContextMenuTests
call drainMainQueue(timeout:), but the shared helper takes no arguments.

Refs #15488

Co-authored-by: Leo Li <cheerleaderleo@outlook.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lawrencecchen pushed a commit that referenced this pull request Sep 30, 2026
The same change as #16232 (7a51764), carried so this PR can restore
main's cmuxTests build in one piece. #15381 made
LastSurfaceClosePreferenceTests and WorkspaceCloseTabsContextMenuTests
call drainMainQueue(timeout:), but the shared helper takes no arguments.

Refs #15488

Co-authored-by: Leo Li <cheerleaderleo@outlook.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Closing in favour of #16245, which makes the same drainMainQueue(timeout:) change and also fixes the controller binding in PaneResizeShortcutTests and the CMUXCLI.vmReadyPollInterval assertion that cannot resolve from the app-host target. Between #16245 and #16260 the app-host test product compiles again; full attribution is on #16245.

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.

2 participants