From 08e63dcdfd7c44c6b9380f161ad5ecba6d89de53 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Fri, 25 Sep 2026 00:31:54 -0400 Subject: [PATCH 1/4] test: fail when a test leaves its AppDelegate installed as shared AppDelegate.init installs itself as AppDelegate.shared. A test that builds a throwaway delegate leaves it there for the next test in the same host, so the detached-inspector Cmd-W tests failed on main whenever the timing-based shard layout ran them after AppDelegateWindowContextRoutingTests. Co-Authored-By: Claude Opus 5.5 --- cmuxTests/WindowAndDragTests.swift | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/cmuxTests/WindowAndDragTests.swift b/cmuxTests/WindowAndDragTests.swift index cc9a2d4ec8bc..59d2d8f6f1fe 100644 --- a/cmuxTests/WindowAndDragTests.swift +++ b/cmuxTests/WindowAndDragTests.swift @@ -587,6 +587,32 @@ final class AppDelegateWindowContextRoutingTests: XCTestCase { } +/// `AppDelegate.init` installs the new delegate as `AppDelegate.shared`, and +/// many tests build a throwaway one without putting the host's back. The next +/// test in the same host then ran against the leftover: detached-inspector +/// Cmd-W tests failed on main whenever the shard layout placed them after +/// AppDelegateWindowContextRoutingTests. XCTest runs these two in name order. +@MainActor +final class AppDelegateSharedIsolationTests: XCTestCase { + private static var sharedBeforeLeak: AppDelegate?? + + func test1ConstructingAnAppDelegateReplacesShared() { + Self.sharedBeforeLeak = .some(AppDelegate.shared) + let leaked = AppDelegate() + XCTAssertTrue(AppDelegate.shared === leaked) + } + + func test2NextTestStartsWithTheHostSharedDelegate() throws { + guard let expected = Self.sharedBeforeLeak else { + throw XCTSkip("Runs after test1ConstructingAnAppDelegateReplacesShared in the same host") + } + XCTAssertTrue( + AppDelegate.shared === expected, + "A delegate a previous test constructed must not stay installed as AppDelegate.shared" + ) + } +} + @MainActor final class AppDelegateLaunchServicesRegistrationTests: XCTestCase { func testDefaultTerminalRegistrationKeepsAllAdvertisedTargets() { From 5cfa5429a249c3214f69ac2df73ce3937a1f798c Mon Sep 17 00:00:00 2001 From: Leo Li Date: Fri, 25 Sep 2026 00:33:06 -0400 Subject: [PATCH 2/4] test: restore AppDelegate.shared after every XCTest case AppDelegate.init installs itself as AppDelegate.shared, and 26 test files build a throwaway delegate without restoring the host's. Register a bundle principal class that records shared when each XCTest case starts and puts it back when the case finishes, so a suite's leftover delegate no longer reaches whichever suite the shard layout runs next. Co-Authored-By: Claude Opus 5.5 --- cmux.xcodeproj/project.pbxproj | 2 ++ .../AppDelegateMainWindowTestingSupport.swift | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index 0e832f4f7c36..95c4d9f7e5d9 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -17705,6 +17705,7 @@ CODE_SIGN_STYLE = Automatic; CURRENT_PROJECT_VERSION = 106; GENERATE_INFOPLIST_FILE = YES; + INFOPLIST_KEY_NSPrincipalClass = CmuxTestsPrincipal; MACOSX_DEPLOYMENT_TARGET = 14.0; MARKETING_VERSION = 0.64.25; ONLY_ACTIVE_ARCH = YES; @@ -17725,6 +17726,7 @@ CODE_SIGN_STYLE = Automatic; CURRENT_PROJECT_VERSION = 106; GENERATE_INFOPLIST_FILE = YES; + INFOPLIST_KEY_NSPrincipalClass = CmuxTestsPrincipal; MACOSX_DEPLOYMENT_TARGET = 14.0; MARKETING_VERSION = 0.64.25; ONLY_ACTIVE_ARCH = NO; diff --git a/cmuxTests/AppDelegateMainWindowTestingSupport.swift b/cmuxTests/AppDelegateMainWindowTestingSupport.swift index c8f460fc214d..dcc5210f20c8 100644 --- a/cmuxTests/AppDelegateMainWindowTestingSupport.swift +++ b/cmuxTests/AppDelegateMainWindowTestingSupport.swift @@ -1,5 +1,6 @@ import AppKit import Foundation +import XCTest #if canImport(cmux_DEV) @testable import cmux_DEV @@ -290,3 +291,33 @@ struct PortalRenderingAuthorityUnavailable: Error, CustomStringConvertible { final class KeyStatusTestWindow: NSWindow { override var isKeyWindow: Bool { true } } + +/// The cmuxTests bundle's NSPrincipalClass. XCTest creates it when the bundle +/// loads, before the first test, and it restores `AppDelegate.shared` after +/// every XCTest case. +/// +/// `AppDelegate.init` installs the new delegate as `shared`, and hundreds of +/// tests build a throwaway delegate without restoring the host's. Whichever +/// test ran next in the same host inherited the leftover, and which tests +/// share a host depends on the timing-based shard layout, so the resulting +/// failures moved from run to run. Swift Testing tests are not observed here. +@objc(CmuxTestsPrincipal) +final class CmuxTestsPrincipal: NSObject, XCTestObservation { + private var sharedAtStart: AppDelegate? + + override init() { + super.init() + XCTestObservationCenter.shared.addTestObserver(self) + } + + func testCaseWillStart(_ testCase: XCTestCase) { + sharedAtStart = AppDelegate.shared + } + + func testCaseDidFinish(_ testCase: XCTestCase) { + if AppDelegate.shared !== sharedAtStart { + AppDelegate.shared = sharedAtStart + } + sharedAtStart = nil + } +} From 4d9c5b5833cefb00ef9014a4ca937a5e2e67a950 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Fri, 25 Sep 2026 01:38:32 -0400 Subject: [PATCH 3/4] test: put the surface registry's route retirer back with AppDelegate.shared AppDelegate.init also attaches itself as the terminal surface registry's weak route retirer. Once the observer drops a leftover delegate, that reference went nil for the rest of the host; re-attach the restored delegate, as AppDelegateShortcutRoutingTests already does by hand. Co-Authored-By: Claude Opus 5.5 --- cmuxTests/AppDelegateMainWindowTestingSupport.swift | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cmuxTests/AppDelegateMainWindowTestingSupport.swift b/cmuxTests/AppDelegateMainWindowTestingSupport.swift index dcc5210f20c8..9521d2b8779f 100644 --- a/cmuxTests/AppDelegateMainWindowTestingSupport.swift +++ b/cmuxTests/AppDelegateMainWindowTestingSupport.swift @@ -1,4 +1,5 @@ import AppKit +import CmuxTerminal import Foundation import XCTest @@ -300,7 +301,9 @@ final class KeyStatusTestWindow: NSWindow { /// tests build a throwaway delegate without restoring the host's. Whichever /// test ran next in the same host inherited the leftover, and which tests /// share a host depends on the timing-based shard layout, so the resulting -/// failures moved from run to run. Swift Testing tests are not observed here. +/// failures moved from run to run. `AppDelegate.init` also points the surface +/// registry's weak route retirer at itself, so that is put back too. Swift +/// Testing tests are not observed here; their suites restore `shared` themselves. @objc(CmuxTestsPrincipal) final class CmuxTestsPrincipal: NSObject, XCTestObservation { private var sharedAtStart: AppDelegate? @@ -317,6 +320,9 @@ final class CmuxTestsPrincipal: NSObject, XCTestObservation { func testCaseDidFinish(_ testCase: XCTestCase) { if AppDelegate.shared !== sharedAtStart { AppDelegate.shared = sharedAtStart + if let sharedAtStart { + GhosttyApp.terminalSurfaceRegistry.attachRouteRetirer(sharedAtStart) + } } sharedAtStart = nil } From 0b514d5d200f6a2d61e700fd5fe952a49498b173 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Fri, 25 Sep 2026 02:57:12 -0400 Subject: [PATCH 4/4] test: serialize Swift Testing suites that swap AppDelegate.shared CmuxTestsPrincipal only observes XCTest cases. NewCloudWorkspaceShortcutTests builds AppDelegate() without restoring shared, and it, the display-config suite and WorkspaceGroupCycleShortcutTests read shared across suspension points, so a parallel suite could swap it mid-test. The new .exclusiveAppContext trait runs each of their tests inside AppContextSerialGate and puts shared and the route retirer back after it. Co-Authored-By: Claude Opus 5.5 --- ...AppDelegateDisplayConfigRestoreTests.swift | 2 +- .../AppDelegateMainWindowTestingSupport.swift | 41 ++++++++++++++++++- .../NewCloudWorkspaceShortcutTests.swift | 2 +- .../WorkspaceGroupCycleShortcutTests.swift | 2 +- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/cmuxTests/AppDelegateDisplayConfigRestoreTests.swift b/cmuxTests/AppDelegateDisplayConfigRestoreTests.swift index 4a0249200440..59de86f6c07c 100644 --- a/cmuxTests/AppDelegateDisplayConfigRestoreTests.swift +++ b/cmuxTests/AppDelegateDisplayConfigRestoreTests.swift @@ -8,7 +8,7 @@ import Testing @testable import cmux #endif /// Round-trip coverage for per-monitor window-geometry memory (issue #2135). -@Suite(.serialized) +@Suite(.serialized, .exclusiveAppContext) @MainActor struct AppDelegateDisplayConfigRestoreTests { // MARK: fixtures diff --git a/cmuxTests/AppDelegateMainWindowTestingSupport.swift b/cmuxTests/AppDelegateMainWindowTestingSupport.swift index 9521d2b8779f..77d0a381344d 100644 --- a/cmuxTests/AppDelegateMainWindowTestingSupport.swift +++ b/cmuxTests/AppDelegateMainWindowTestingSupport.swift @@ -1,6 +1,7 @@ import AppKit import CmuxTerminal import Foundation +import Testing import XCTest #if canImport(cmux_DEV) @@ -303,7 +304,10 @@ final class KeyStatusTestWindow: NSWindow { /// share a host depends on the timing-based shard layout, so the resulting /// failures moved from run to run. `AppDelegate.init` also points the surface /// registry's weak route retirer at itself, so that is put back too. Swift -/// Testing tests are not observed here; their suites restore `shared` themselves. +/// Testing tests are not observed here; a Swift Testing suite that constructs +/// `AppDelegate()` or reads `shared` across a suspension point takes +/// `.exclusiveAppContext`, which serializes it with the other app-context tests +/// and restores `shared` the same way. @objc(CmuxTestsPrincipal) final class CmuxTestsPrincipal: NSObject, XCTestObservation { private var sharedAtStart: AppDelegate? @@ -327,3 +331,38 @@ final class CmuxTestsPrincipal: NSObject, XCTestObservation { sharedAtStart = nil } } + +/// Swift Testing counterpart of `CmuxTestsPrincipal`: runs each test in the +/// suite inside `AppContextSerialGate`, so suites in parallel cannot swap +/// `AppDelegate.shared` under each other at a suspension point, and then puts +/// `shared` and the surface registry's route retirer back. +struct ExclusiveAppContextTrait: SuiteTrait, TestTrait, TestScoping { + var isRecursive: Bool { true } + + func scopeProvider(for test: Test, testCase: Test.Case?) -> Self? { + testCase == nil ? nil : self + } + + func provideScope( + for test: Test, + testCase: Test.Case?, + performing function: @Sendable () async throws -> Void + ) async throws { + try await AppContextSerialGate.withExclusiveAppContext { + let sharedAtStart = AppDelegate.shared + defer { + if AppDelegate.shared !== sharedAtStart { + AppDelegate.shared = sharedAtStart + if let sharedAtStart { + GhosttyApp.terminalSurfaceRegistry.attachRouteRetirer(sharedAtStart) + } + } + } + try await function() + } + } +} + +extension Trait where Self == ExclusiveAppContextTrait { + static var exclusiveAppContext: Self { Self() } +} diff --git a/cmuxTests/NewCloudWorkspaceShortcutTests.swift b/cmuxTests/NewCloudWorkspaceShortcutTests.swift index b1331828b9b0..a99aea1af232 100644 --- a/cmuxTests/NewCloudWorkspaceShortcutTests.swift +++ b/cmuxTests/NewCloudWorkspaceShortcutTests.swift @@ -14,7 +14,7 @@ import Testing /// rows with their live shortcut hints, and the shared action every /// entrypoint routes through. @MainActor -@Suite(.serialized) +@Suite(.serialized, .exclusiveAppContext) final class NewCloudWorkspaceShortcutTests { private final class RecordingSheetPresenter: NewMachineSheetPresenting { private(set) var presentCount = 0 diff --git a/cmuxTests/WorkspaceGroupCycleShortcutTests.swift b/cmuxTests/WorkspaceGroupCycleShortcutTests.swift index 4f1cebc08f44..107b40839c9f 100644 --- a/cmuxTests/WorkspaceGroupCycleShortcutTests.swift +++ b/cmuxTests/WorkspaceGroupCycleShortcutTests.swift @@ -11,7 +11,7 @@ import Testing #if DEBUG @MainActor -@Suite("Workspace group cycle shortcuts", .serialized) +@Suite("Workspace group cycle shortcuts", .serialized, .exclusiveAppContext) struct WorkspaceGroupCycleShortcutTests { @Test func actionsAreVisibleAndUnboundByDefault() throws { let actions: [KeyboardShortcutSettings.Action] = [