tests: move package-only suites out of the app-host bundle - #13790
Merged
teamleaderleo merged 2 commits intoSep 23, 2026
Merged
teamleaderleo merged 2 commits into
teamleaderleo merged 2 commits into
Conversation
Eleven files under cmuxTests/ reference only package types and Foundation, so the code they cover already lives in a SwiftPM package. They were paying the app-host bundle's compile and host-launch cost for nothing. Move them whole into the owning package's test target, where `swift test` runs them on a plain, parallel lane. 71 tests in 13 suites move to CmuxFoundationTests, CmuxCloudImagePasteTests, CmuxRemoteSessionTests, CmuxTerminalTests, CmuxSettingsTests and CmuxSettingsUITests. The `@testable import cmux` blocks go with them: none of these suites named an app symbol, which is what made them movable in the first place. Two supporting changes the package build needs: - `SSHPTYAttachExitCode` gains `Sendable`. It is a bare Int32-backed enum with no associated values; the app-host target never needed the conformance spelled out, but CmuxFoundationTests builds in Swift 6 language mode and `@Test(arguments:)` requires it. - `CmuxRemoteSessionTests` gains the Bonsplit product, which the native mirror layout fuzz test asserts against. Test target only; the CmuxRemoteSession library target is unchanged. The files are deleted from cmuxTests/ and unwired from the cmuxTests target via scripts/sync-test-wiring, then normalized with scripts/normalize-pbxproj.py, so lint-pbxproj-test-wiring.sh stays green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 22 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (14)
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 |
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
teamleaderleo
enabled auto-merge (squash)
September 22, 2026 23:59
teamleaderleo
added a commit
that referenced
this pull request
Sep 23, 2026
* ci: run package tests on the pull requests that change packages `macos / swift-package-tests` is gated on `inputs.full_suite == 'true'`, and `full_suite` is false for every pull request under `CI_PULL_REQUEST_SUITE=compile-only`. The job is therefore skipped on every PR. The only macOS signal a PR gets is `macOS compile admission`, which builds package *library* targets and never their test targets, so a change to a package's tests currently lands with zero CI execution of those assertions. #13786 and #13790 move ~150 of them into package test targets. Route the lane from changed paths, the way the `cli` lane selects itself (#13760). A new `swift_packages` change area asks `scripts/ci/select_package_tests.py` — the same selector the job already uses to narrow its package list — whether a diff can affect any package the job runs. Only paths that feed a package are offered to it: * The selector fails open, so the lane's own workflow and scripts select all 33 packages. On main that is right; on a pull request it is the 30-minute sweep under another name. Over the last 200 merged PRs, routing those would have queued 34 full runs. * A package outside the job's list selects nothing, so the lane would start, check out submodules and test zero packages. Measured over the last 200 merged pull requests: 28 would run the lane (0 do today), testing a median of 2 packages and never more than 15, never all 33 and never zero. The full suite is untouched: `app-host-unit-tests`, `tests-build-and-lag` and the Release jobs stay gated on `full_suite` alone, and a test in this change pins that set so a new lane cannot silently join it. The Release Ghostty CLI helper steps inside the job now also require `full_suite`, since only the `full_suite`-gated Release jobs consume that artifact. Also collect the operating system's crash report when a package test runner dies on a signal. `main` is currently red from a SIGBUS in CmuxSettingsUI, and all the job records is SwiftPM's one-line "Exited with unexpected signal code 10" — no faulting address, no frames. Replaying the retry guard against that job's captured output shows widening its signal list would not have retried it anyway: the runner had already emitted 401 lines of test output, which the guard's third clause correctly refuses to retry. The signal list is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: catch full package sweeps on mixed routed changes * ci: keep targeted package selection aligned with routing --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Moves the first batch of app-host test files that reference only package types + Foundation out of
cmuxTests/and into the owning SwiftPM package's test target. The code under test already lives in a package; these suites were sitting in the app-host bundle for no reason, paying its compile and host-launch cost on every run.This is item 1 of the placement plan in the
cmuxTestsaudit for RFC #13519 ("move focused tests out of the giant app-host bundle with their code"), and follows the package-extraction pattern of #13107 / #13119 / RFC #13108.What moved
Whole files only — no mega-file surgery, no half-split files.
SidebarTabDropIndicatorPredicateTests.swift(4 suites)CmuxFoundationTestsSSHPTYAttachExitCodeClassifierTests.swiftCmuxFoundationTestsSidebarWorkspaceSelectionAnchorPolicyTests.swiftCmuxFoundationTestsCloudImagePasteCoordinatorTests.swift(+ itsCloudImagePasteTestPeer.swifthelper)CmuxCloudImagePasteTestsRemoteTmuxNativeMirrorLayoutFuzzTests.swiftCmuxRemoteSessionTestsRemoteTmuxLayoutNodePatchingTests.swiftCmuxRemoteSessionTestsTerminalSurfaceResizePolicyTests.swiftCmuxTerminalTestsTerminalPathEnvironmentTests.swiftCmuxTerminalTestsGlobalSearchShortcutSettingsModelTests.swiftCmuxSettingsUITestsShortcutHintDebugSettingsBindingTests.swiftCmuxSettingsTests71 tests in 13 suites, 11 files, off the app-host bundle.
Supporting changes
SSHPTYAttachExitCodegainsSendable. It is a bareInt32-backed enum with no associated values; the app-host target never needed the conformance spelled out, butCmuxFoundationTestsbuilds in Swift 6 language mode and@Test(arguments:)requires it. No behavior change.CmuxRemoteSessionTestsgains theBonsplitproduct — the native-mirror layout fuzz test asserts the bonsplit plan against tmux geometry. Test target only; theCmuxRemoteSessionlibrary target is untouched.#if canImport(cmux_DEV) / @testable import cmuxblocks are dropped from every moved file. None of these suites referenced an app symbol; that is what put them in the "movable now" class.cmux.xcodeproj/project.pbxproj: the 11 files are removed from thecmuxTeststarget via./scripts/sync-test-wiring, thenpython3 scripts/normalize-pbxproj.py. The files are deleted fromcmuxTests/, not merely unwired, solint-pbxproj-test-wiring.shstays green.tests/test-execution.tomlneeds no change: it registers Python lanes only, andpython3 scripts/ci/validate_test_execution_registry.pypasses unchanged (226 tests).Verification
swift testper touched package, locally:CmuxCloudImagePasteCmuxRemoteSessionCmuxSettingsCmuxSettingsUICmuxTerminalCmuxFoundation--filterTwo caveats about the local box, both reproduced on unmodified
upstream/mainin the same worktree:CmuxFoundation's fullswift testdoes not build here: the pre-existingSSHPTYReplayOutputFilterTests.swifthits "the compiler is unable to type-check this expression in reasonable time" on two long string concatenations. Unrelated to this change — it fails identically with the changes stashed. With that expression temporarily annotated, the full run is 412 tests in 66 suites with 5 failures, all inCommandRunnerDescriptorLifecycleTestsandSSHPTYAttachRetryScriptBuilderTests(process-spawning tests that do not like this sandbox), and none in a moved suite.CommandRunner/SSHPTYAttachRetryScriptBuilderfailures above are environmental for the same reason.Fast static checks run locally and green:
tests/test_normalize_pbxproj.py,scripts/check-pbxproj.sh,scripts/check-workspace-package-groups.py --check,tests/test_sync_test_wiring.py,scripts/lint-pbxproj-test-wiring.sh(1040 files checked).What deliberately did not move
The audit counts 326 class-(a) tests across 72 suites. This PR ships the subset that is a clean whole-file move and actually compiles and passes under
swift test. Everything below is a follow-up, not an oversight:ShortcutWhenClauseTests,SocketControlPasswordStoreTests,RovoDevHookConfigTests,CommandPaletteEmojiTitleSearchTests,GhosttyCopyActionResolverAppTests(23 tests). These are the audit's "same behavior at two layers" cases: the coverage is complementary rather than duplicated, so they need a merge-and-dedupe into the existing package suite, not a move.GhosttyCopyActionResolverAppTestsasserts that the app target resolves through the package resolver — that one should probably stay where it is.GhosttyConfigTests.swift,SidebarOrderingTests.swift,ShortcutAndCommandPaletteTests.swift, …). Lifting a suite out of a 6k-line grab-bag is mechanical, but the remainder of the file can only be proven by an app-host build, so it belongs in its own PR.KeyboardShortcutSavedLayoutTemplateTestsassertsCmuxSettings.ShortcutActionstays aligned with the app target'sKeyboardShortcutSettings.Action.DebugEventLogSerializedAppendTestscalls bonsplit'sdlog, andCMUXDebugLogis pinned to macOS 13 while Bonsplit needs 14 — a 2-test move is not worth raising a package's deployment target.CloudSidebarAcceptanceTestsdrives other app-host suites as fixtures — host-required.SidebarWorkspaceRowStatusGlyphRemovalTestsscansSources/*.swiftthrough a#filePathwalk anchored atcmuxTests/— a repo-source guard, not a package test.AutoNaming*Tests,SSHPTYAttachReconnectInputFilterTests,RemoteTmuxInputCommandCaptureTestscover code inCLI/or helpers incmuxTests/, not in any package.BrowserPDFPreviewActionRegressionTestsimports WebKit.No file with an in-flight PR (#13759, #13752, #13736, #13744) is touched, and no CLI/product-subprocess suite is moved.
🤖 Generated with Claude Code
Summary by cubic
Moves 71 tests in 13 suites across 11 files out of the app-host bundle (
cmuxTests/) into the test targets of the SwiftPM packages that own the code under test, so these package-only suites no longer pay the app-host compile and host-launch cost on every run.What moved
SidebarTabDropIndicatorPredicateTests,SSHPTYAttachExitCodeClassifierTests, andSidebarWorkspaceSelectionAnchorPolicyTestsmove toCmuxFoundationTests.CloudImagePasteCoordinatorTestsand itsCloudImagePasteTestPeerhelper move toCmuxCloudImagePasteTests.RemoteTmuxNativeMirrorLayoutFuzzTestsandRemoteTmuxLayoutNodePatchingTestsmove toCmuxRemoteSessionTests.TerminalSurfaceResizePolicyTestsandTerminalPathEnvironmentTestsmove toCmuxTerminalTests.GlobalSearchShortcutSettingsModelTestsandShortcutHintDebugSettingsBindingTestsmove toCmuxSettingsUITestsandCmuxSettingsTests.@testable import cmuxblocks are dropped from every moved file; none of these suites referenced an app symbol.cmuxTests/and unwired from thecmuxTeststarget viascripts/sync-test-wiring, then normalized withscripts/normalize-pbxproj.py, solint-pbxproj-test-wiring.shstays green.Supporting changes
SSHPTYAttachExitCodenow conforms toSendable; it's a bareInt32-backed enum, andCmuxFoundationTestsbuilds in Swift 6 mode where@Test(arguments:)requires the conformance. No behavior change.CmuxRemoteSessionTestsgains theBonsplitproduct dependency for the native-mirror layout fuzz assertion; theCmuxRemoteSessionlibrary target is untouched.Written for commit 269d3ab. Summary will update on new commits.