Skip to content

test: yield to the main queue while the Files tree catches up after its menu closes - #14660

Merged
lawrencecchen merged 1 commit into
mainfrom
fix-file-explorer-menu-reload-test
Sep 25, 2026
Merged

lawrencecchen merged 1 commit into
mainfrom
fix-file-explorer-menu-reload-test

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

FileExplorerContextMenuReloadTests/reloadWaitsForContextMenuToClose (added by #14451) fails on main: outlineView.numberOfRows → 3 where 1 was expected after the context menu closes. It failed on app-host shard 5 of #13981 (run 36151855238).

The product code is correct: contextMenuDidClose() queues the catch-up reload with DispatchQueue.main.async. The test waited for it by spinning RunLoop.main.run(until:). This @MainActor Swift Testing test runs inside a main-queue block, and a nested run loop cannot drain the main queue from there, so the queued reload never ran before the 2 s deadline.

The test now waits through AppKitTestEventPump().waitUntil, the shared hosted-view helper that yields to the main queue between checks. No product change.

🤖 Generated with Claude Code


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

Fixes the flaky FileExplorerContextMenuReloadTests test by waiting for the deferred Files tree reload with AppKitTestEventPump().waitUntil, which yields to the main queue, instead of spinning RunLoop.main (which can't drain the main queue from inside a main-queue block). No product change.

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

Review in cubic

Summary by CodeRabbit

  • Tests
    • Updated the context-menu reload test to wait for the outline to update after the menu closes, with a clearer failure message.

…ts menu closes

reloadWaitsForContextMenuToClose waited for the deferred reload by spinning
RunLoop.main. The reload is queued with DispatchQueue.main.async, and this
@mainactor Swift Testing test runs inside a main-queue block, where a nested
run loop cannot drain the main queue, so the wait always timed out with 3
rows. Wait through AppKitTestEventPump, which yields to the main queue.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@lawrencecchen
lawrencecchen enabled auto-merge (squash) September 25, 2026 17:28
@github-actions

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9e2757ef-0493-4bbe-af11-a2f993bb6bab

📥 Commits

Reviewing files that changed from the base of the PR and between 74917a9 and cc88f34.

📒 Files selected for processing (1)
  • cmuxTests/FileExplorerContextMenuReloadTests.swift

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


📝 Walkthrough

Walkthrough

The context-menu reload test now waits asynchronously for the outline to reach one row after the menu closes. Its failure message reports the observed row count.

Changes

File Explorer Reload Test

Layer / File(s) Summary
Asynchronous reload wait
cmuxTests/FileExplorerContextMenuReloadTests.swift
The test uses an awaited event-pump wait of up to two seconds after the menu closes. The final expectation reports the observed row count.

Priority: ⬇️ Low

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

Change: Other

Merge Risk: ⚪ Minimal · up to cc88f

This change only updates test synchronization, waiting for the expected row count with a timeout. No merge-blocking risk is identified; it appears ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 23 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description clearly explains the failure, root cause, and test-only fix, but it omits the required Testing section and verification results. It also omits the Demo Video section and checklist stat… Add the required Summary, Testing, and Demo Video sections. State which test command or CI lane ran and what passed. Explain if a demo video is not applicable. Complete the applicable checklist items and report any unverified checks.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (23 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: the test yields to the main queue while the Files tree reloads after the context menu closes.
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 Cloud Persistent Session And Early Input ✅ Passed The PR changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. It replaces a synchronous test wait with AppKitTestEventPump().waitUntil; it does not change Cloud terminal creation, transp…
Cmux Swift Actor Isolation ✅ Passed PASS: The review-scoped diff changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. It updates a @MainActor test to be async and replaces its wait loop with AppKitTestEventPump; it int…
Cmux Swift Blocking Runtime ✅ Passed PASS. The PR changes only cmuxTests/FileExplorerContextMenuReloadTests.swift; no production Swift file changes are present. The added AppKitTestEventPump().waitUntil(timeout: .seconds(2)) is test-…
Cmux Browser Automation Off-Main ✅ Passed PASS. The pull request changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. It makes a File Explorer test asynchronous and replaces a nested run-loop wait with `AppKitTestEventPump().wai…
Cmux Expensive Synchronous Load ✅ Passed PASS. The authoritative diff changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. It updates test waiting from RunLoop.main to the test-only AppKitTestEventPump; it adds no productio…
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. It updates a test from synchronous RunLoop.main polling to AppKitTestEventPump().waitUntil; it does not ch…
Cmux No Hacky Sleeps ✅ Passed PASS. The PR changes only cmuxTests/FileExplorerContextMenuReloadTests.swift, a Swift test file. The rule explicitly scopes out Swift and covers TypeScript, JavaScript, shell, and non-Swift build/ru…
Cmux Algorithmic Complexity ✅ Passed PASS. The pull request changes only cmuxTests/FileExplorerContextMenuReloadTests.swift, a test. It replaces a bounded wait loop with AppKitTestEventPump().waitUntil(timeout: .seconds(2)). The algo…
Cmux Swift Concurrency ✅ Passed PASS. The diff changes only a cmux test. It replaces a synchronous RunLoop.main wait with the existing async AppKitTestEventPump().waitUntil helper. The helper yields through `DispatchQueue.main.a…
Cmux Swift @Concurrent ✅ Passed PASS — The PR changes only a @MainActor test, making it async to coordinate UI-bound event-pump work. It adds no nonisolated async function and no @concurrent annotation. AppKitTestEventPump.waitUntil…
Cmux Swift Package Boundaries ✅ Passed The pull request changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. The diff updates test waiting behavior and adds no production Swift feature logic. The package-boundary rule explici…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The PR changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. The authoritative diff contains no Package.swift, Package.resolved, .gitignore, Xcode project/workspace, workflow,…
Cmux Swift Logging ✅ Passed The pull request changes only a Swift test. It adds no print, debugPrint, dump, NSLog, ad hoc logging, Logger declaration, or sensitive-data logging. The changed row-count text is an assertion…
Cmux User-Facing Error Privacy ✅ Passed The scoped diff changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. It updates test waiting logic, adds a developer-only comment, and includes the observed row count in a test assertion…
Cmux Full Internationalization ✅ Passed PASS: The authoritative diff changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. It updates a test to use AppKitTestEventPump and changes a test failure message; it adds no production…
Cmux Swiftui State Layout ✅ Passed The PR changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. The diff updates an AppKit test from RunLoop.main polling to await AppKitTestEventPump().waitUntil; it does not add or mod…
Cmux Architecture Rethink ✅ Passed PASS: The diff changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. It replaces a test-local RunLoop.main deadline loop with the shared, test-only AppKitTestEventPump.waitUntil, whic…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The pull request changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. It makes a test asynchronous and replaces a run-loop wait with AppKitTestEventPump().waitUntil. It adds no `…
Cmux Source Artifacts ✅ Passed The PR changes only cmuxTests/FileExplorerContextMenuReloadTests.swift, a hand-written Swift test. The diff contains no logs, screenshots, recordings, caches, build output, temporary directories, de…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The authoritative PR diff changes only cmuxTests/FileExplorerContextMenuReloadTests.swift. The patch updates a test method to use AppKitTestEventPump; it changes no Swift file under a produc…
Full details: Description check

Explanation

The description clearly explains the failure, root cause, and test-only fix, but it omits the required Testing section and verification results. It also omits the Demo Video section and checklist status.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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 merged commit f04320e into main Sep 25, 2026
49 of 50 checks passed
@lawrencecchen
lawrencecchen deleted the fix-file-explorer-menu-reload-test branch September 25, 2026 17:40
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for cc88f34d5d: every check was green at merge (14 verified; 13 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 25, 2026
bd2d34e test: expect split zoom to survive closing one tab of a zoomed pane (manaflow-ai#14664)
f8857c5 fix(scripts): append, not prepend, the cargo fallback PATH in build-cmux-cua.sh (manaflow-ai#14665)
56a3e4c fix: make the event-stream reconnect decision a value, not a static namespace (manaflow-ai#14661)
06e064d ci: pin cla.yml and claude.yml to a GitHub-hosted runner (manaflow-ai#14668)
2509187 ci: restore the git object seed before checkout in E2E and iOS macOS jobs (manaflow-ai#14669)
402d0ad docs: move team-internal fleet and session rules out of CLAUDE.md (manaflow-ai#14595)
3bfe0b6 pull_request_template: drop the commented @codex review trigger block (manaflow-ai#14599)
4f0ac55 fix(control): honor color/icon keys and validate hex in workspace.group.set_color/set_icon (manaflow-ai#13877)
805a699 ci(rescue): mint the read token with only the permissions the App has (manaflow-ai#14627)
7e662c0 Tell the user why a file upload failed (manaflow-ai#11476)
f04320e test: yield to the main queue while the Files tree catches up after its menu closes (manaflow-ai#14660)
a5f705b A mirrored tmux window with one pane shows two tab bars (manaflow-ai#11248)
74917a9 iOS: remove unshipped push reconnect banner

# Conflicts:
#	.github/workflows/ci-owned-pool-rescue.yml
#	.github/workflows/cla.yml
#	.github/workflows/claude.yml
#	.github/workflows/test-e2e.yml
#	.github/workflows/test-ios.yml
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