Repository navigation
test: give split fixtures their own window geometry instead of inheriting a narrow one - #15919
lawrencecchen wants to merge 1 commit into
Conversation
…dow geometry createMainWindow copies the current main window's size, and earlier tests in the same app host leave 320-point main windows registered. Since split admission (#15392) refuses a side-by-side split below two minimum-width panes, the fixtures' newTerminalSplit returned nil depending on which tests shared the host. Move the geometry setup #15434 used for the shortcut-routing fixtures onto Workspace and use it where these tests split. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Parked as draft: the leaked 320-pt main window cause is now owned by one root-cause fix for all affected split tests (including AppDelegateEqualizeSplitsShortcutTests). This branch may be reused or closed by that owner. |
|
All contributors have signed the CLA ✍️ ✅ |
|
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 configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughTests now use a shared helper to configure realistic window geometry before creating terminal splits. The helper is used in shortcut-routing, remote tmux mirror, and unread-state tests. ChangesRealistic-window split test setup
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to No merge-blocking issue is established for the shared split-test setup; the reported height mismatch does not affect the current callers. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
|
The split failures this fixes have a second source, which #15953 addresses. The two are complementary and merged cleanly in #15960.
#15953 resets that domain before each app-host batch, and isolates the window-shaping keys per test for every XCTest case and for the 36 Swift Testing suites that open main windows. That also covers the call sites this PR leaves alone, such as |
CI failure attributionCI failed on
Matched log linesNot re-run automatically: Written by |
|
Superseded by #15985, which fixes the root cause (the persisted cmux.session.lastWindowGeometry.v2 frame shared across app-host processes on a runner) instead of opting single tests into a larger window. |
2 participants
Summary
Two app-host tests fail on main when they share a host process with earlier window tests:
WorkspaceManualUnreadTests.testToggleFocusedNotificationUnreadKeepsManualUnreadOnOriginalPanelAfterFocusMoves(XCTUnwrap of the split panel, main run 36685498203 shard 1/7)RemoteTmuxMirrorSplitRoutingTests.localWorkspaceSplitStillCreatesLocalPanel()(panel → nil, main shards 1/3/7 since 2026-09-28 18:00 PDT)createMainWindow()copies the size of the current main window, and earlier tests in the same app host leave 320-point main windows registered. After the 240-point sidebar, the new window's split container is narrower than two 160-point panes, so split admission from #15392 correctly refuses a side-by-side split. The tests had relied on inherited window geometry. In run 36685498203 the same process (pid 51066) also failed theAppDelegateEqualizeSplitsShortcutTestssplit setups for this reason; that suite is not changed here.#15434 fixed the same symptom for the shortcut-routing fixtures with a helper local to
AppDelegateShortcutRoutingTests. This moves that geometry setup ontoWorkspace.newTerminalSplitInRealisticWindowForTesting(window:from:orientation:focus:)in the shared test support file, keeps the old helper as a forwarder, and uses it where these two tests split inside a real window. Admission logic is unchanged. Tests that split a windowlessWorkspace()still measure zero geometry, which admission treats as fitting, so they are left alone.Testing
Swift syntax check passed locally. App-host tests don't run on this Mac; the full
ci.ymlrun on this branch is the evidence (linked in a comment).Changelog
none
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes two app-host tests that failed depending on which tests shared the host process, by giving split fixtures their own window geometry instead of inheriting a narrow one.
createMainWindow()copies the current main window's size, and earlier tests leave 320-point main windows too narrow for split admission to allow a side-by-side split.Workspace.newTerminalSplitInRealisticWindowForTestingto the shared test support file and switches the failing tests over to it; the shortcut-routing helper now forwards to it. No production code changes.Written for commit 01c09c9. Summary will update on new commits.
Summary by CodeRabbit