From 2b99af19bcefb5210e65fcb1c2ef60e331efab49 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Thu, 5 Mar 2026 19:18:01 -0800 Subject: [PATCH] Harden multi-window notify regression test --- Sources/AppDelegate.swift | 69 +++++--- .../MultiWindowNotificationsUITests.swift | 155 +++++++++++------- 2 files changed, 137 insertions(+), 87 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index f74e65db..21569100 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -5683,6 +5683,21 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } } + func waitForSurfaceId( + on tabManager: TabManager, + tabId: UUID, + _ completion: @escaping (UUID) -> Void + ) { + if let surfaceId = tabManager.focusedPanelId(for: tabId) { + completion(surfaceId) + return + } + guard Date() < deadline else { return } + DispatchQueue.main.asyncAfter(deadline: .now() + 0.05) { + waitForSurfaceId(on: tabManager, tabId: tabId, completion) + } + } + waitForContexts(minCount: 1) { [weak self] in guard let self else { return } guard let window1 = self.mainWindowContexts.values.first else { return } @@ -5696,36 +5711,40 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent let contexts = Array(self.mainWindowContexts.values) guard let window2 = contexts.first(where: { $0.windowId != window1.windowId }) else { return } guard let tabId2 = window2.tabManager.selectedTabId ?? window2.tabManager.tabs.first?.id else { return } - guard let store = self.notificationStore else { return } + waitForSurfaceId(on: window2.tabManager, tabId: tabId2) { [weak self] surfaceId2 in + guard let self else { return } + guard let store = self.notificationStore else { return } - // Ensure the target window is currently showing the Notifications overlay, - // so opening a notification must switch it back to the terminal UI. - window2.sidebarSelectionState.selection = .notifications + // Ensure the target window is currently showing the Notifications overlay, + // so opening a notification must switch it back to the terminal UI. + window2.sidebarSelectionState.selection = .notifications - // Create notifications for both windows. Ensure W2 isn't suppressed just because it's focused. - let prevOverride = AppFocusState.overrideIsFocused - AppFocusState.overrideIsFocused = false - store.addNotification(tabId: tabId2, surfaceId: nil, title: "W2", subtitle: "multiwindow", body: "") - AppFocusState.overrideIsFocused = prevOverride + // Create notifications for both windows. Ensure W2 isn't suppressed just because it's focused. + let prevOverride = AppFocusState.overrideIsFocused + AppFocusState.overrideIsFocused = false + store.addNotification(tabId: tabId2, surfaceId: nil, title: "W2", subtitle: "multiwindow", body: "") + AppFocusState.overrideIsFocused = prevOverride - // Insert after W2 so it becomes "latest unread" (first in list). - store.addNotification(tabId: tabId1, surfaceId: nil, title: "W1", subtitle: "multiwindow", body: "") + // Insert after W2 so it becomes "latest unread" (first in list). + store.addNotification(tabId: tabId1, surfaceId: nil, title: "W1", subtitle: "multiwindow", body: "") - let notif1 = store.notifications.first(where: { $0.tabId == tabId1 && $0.title == "W1" }) - let notif2 = store.notifications.first(where: { $0.tabId == tabId2 && $0.title == "W2" }) + let notif1 = store.notifications.first(where: { $0.tabId == tabId1 && $0.title == "W1" }) + let notif2 = store.notifications.first(where: { $0.tabId == tabId2 && $0.title == "W2" }) - self.writeMultiWindowNotificationTestData([ - "window1Id": window1.windowId.uuidString, - "window2Id": window2.windowId.uuidString, - "window2InitialSidebarSelection": "notifications", - "tabId1": tabId1.uuidString, - "tabId2": tabId2.uuidString, - "notifId1": notif1?.id.uuidString ?? "", - "notifId2": notif2?.id.uuidString ?? "", - "expectedLatestWindowId": window1.windowId.uuidString, - "expectedLatestTabId": tabId1.uuidString, - ], at: path) - self.publishMultiWindowNotificationSocketStateIfNeeded(at: path) + self.writeMultiWindowNotificationTestData([ + "window1Id": window1.windowId.uuidString, + "window2Id": window2.windowId.uuidString, + "window2InitialSidebarSelection": "notifications", + "tabId1": tabId1.uuidString, + "tabId2": tabId2.uuidString, + "surfaceId2": surfaceId2.uuidString, + "notifId1": notif1?.id.uuidString ?? "", + "notifId2": notif2?.id.uuidString ?? "", + "expectedLatestWindowId": window1.windowId.uuidString, + "expectedLatestTabId": tabId1.uuidString, + ], at: path) + self.publishMultiWindowNotificationSocketStateIfNeeded(at: path) + } } } } diff --git a/cmuxUITests/MultiWindowNotificationsUITests.swift b/cmuxUITests/MultiWindowNotificationsUITests.swift index 966d060f..f42aba29 100644 --- a/cmuxUITests/MultiWindowNotificationsUITests.swift +++ b/cmuxUITests/MultiWindowNotificationsUITests.swift @@ -209,8 +209,9 @@ final class MultiWindowNotificationsUITests: XCTestCase { XCTAssertTrue( waitForDataMatch(timeout: 20.0) { data in let tabId2 = data["tabId2"] ?? "" + let surfaceId2 = data["surfaceId2"] ?? "" let socketReady = data["socketReady"] ?? "" - return !tabId2.isEmpty && !socketReady.isEmpty && socketReady != "pending" + return !tabId2.isEmpty && !surfaceId2.isEmpty && !socketReady.isEmpty && socketReady != "pending" }, "Expected multi-window notification setup data and socket readiness" ) @@ -233,34 +234,20 @@ final class MultiWindowNotificationsUITests: XCTestCase { ) return } + guard setup["socketPingResponse"] == "PONG" else { + XCTFail( + "Control socket ping sanity check failed. path=\(socketPath) " + + socketDiagnostics(from: setup) + ) + return + } + guard let surfaceId = setup["surfaceId2"], !surfaceId.isEmpty else { + XCTFail("Missing target surface id for workspace \(tabId2)") + return + } XCTAssertTrue(waitForWindowCount(atLeast: 2, app: app, timeout: 6.0)) - let pingResponse = waitForSocketPong(timeout: 20.0) - if pingResponse != "PONG", - let resolvedPath = resolveSocketPath(timeout: 5.0, requiredWorkspaceId: tabId2) { - socketPath = resolvedPath - } - - let confirmedPingResponse = pingResponse == "PONG" ? pingResponse : waitForSocketPong(timeout: 5.0) - guard confirmedPingResponse == "PONG" else { - let failureSetup = loadData() ?? setup - XCTFail( - "Control socket did not respond in time. path=\(socketPath) response=\(confirmedPingResponse ?? "") " + - socketDiagnostics(from: failureSetup) - ) - return - } - - guard let surfaceId = waitForSurfaceId(forWorkspaceId: tabId2, timeout: 12.0) else { - let failureSetup = loadData() ?? setup - XCTFail( - "Expected at least one surface in workspace \(tabId2). socket=\(socketPath) " + - socketDiagnostics(from: failureSetup) - ) - return - } - let finder = XCUIApplication(bundleIdentifier: "com.apple.finder") finder.activate() XCTAssertTrue( @@ -275,14 +262,26 @@ final class MultiWindowNotificationsUITests: XCTestCase { surfaceId: surfaceId, title: title ) - XCTAssertEqual(notifyResult.terminationStatus, 0, "Expected `cmux notify` to succeed. stderr=\(notifyResult.stderr)") - XCTAssertTrue(notifyResult.stdout.contains("OK"), "Expected notify command to return OK. stdout=\(notifyResult.stdout)") - RunLoop.current.run(until: Date().addingTimeInterval(0.5)) XCTAssertFalse( app.state == .runningForeground, - "Expected cmux to remain in background after `cmux notify`. state=\(app.state.rawValue)" + "Expected cmux to remain in background after bundled `cmux notify`. state=\(app.state.rawValue) stderr=\(notifyResult.stderr)" ) + + guard notifyResult.terminationStatus == 0 else { + let rawFallbackResponse: String? + if isSocketPermissionFailure(notifyResult.stderr) { + rawFallbackResponse = socketCommand("notify_target \(tabId2) \(surfaceId) \(title)|ui-test|focus-regression") + } else { + rawFallbackResponse = nil + } + XCTFail( + "Expected bundled `cmux notify` to succeed. stderr=\(notifyResult.stderr) " + + "rawFallback=\(rawFallbackResponse ?? "")" + ) + return + } + XCTAssertTrue(notifyResult.stdout.contains("OK"), "Expected notify command to return OK. stdout=\(notifyResult.stdout)") } private func clickNotificationPopoverRowAndWaitForFocusChange( @@ -556,7 +555,7 @@ final class MultiWindowNotificationsUITests: XCTestCase { surfaceId: String, title: String ) -> (terminationStatus: Int32, stdout: String, stderr: String) { - let result = runCmuxCommand( + runCmuxCommand( socketPath: socketPath, arguments: [ "notify", @@ -571,35 +570,33 @@ final class MultiWindowNotificationsUITests: XCTestCase { "--body", "focus-regression" ], - responseTimeoutSeconds: 4.0 - ) - if result.terminationStatus == 0 || !isSocketPermissionFailure(result.stderr) { - return result - } - - let response = socketCommand("notify_target \(workspaceId) \(surfaceId) \(title)|ui-test|focus-regression") ?? "" - let succeeded = response == "OK" - return ( - terminationStatus: succeeded ? 0 : 1, - stdout: response, - stderr: succeeded - ? "\(result.stderr) raw-socket-fallback" - : "\(result.stderr) raw-socket-fallback-response=\(response)" + responseTimeoutSeconds: 4.0, + cliStrategy: .bundledOnly ) } private func runCmuxCommand( socketPath: String, arguments: [String], - responseTimeoutSeconds: Double = 3.0 + responseTimeoutSeconds: Double = 3.0, + cliStrategy: CmuxCLIStrategy = .any ) -> (terminationStatus: Int32, stdout: String, stderr: String) { var args = ["--socket", socketPath] args.append(contentsOf: arguments) var environment = ProcessInfo.processInfo.environment environment["CMUXTERM_CLI_RESPONSE_TIMEOUT_SEC"] = String(responseTimeoutSeconds) + let cliPaths = resolveCmuxCLIPaths(strategy: cliStrategy) + if cliPaths.isEmpty, cliStrategy == .bundledOnly { + return ( + terminationStatus: -1, + stdout: "", + stderr: "Failed to locate bundled cmux CLI" + ) + } + var lastPermissionFailure: (terminationStatus: Int32, stdout: String, stderr: String)? - for cliPath in resolveCmuxCLIPaths() { + for cliPath in cliPaths { let result = executeCmuxCommand( executablePath: cliPath, arguments: args, @@ -615,6 +612,14 @@ final class MultiWindowNotificationsUITests: XCTestCase { return result } + if cliStrategy == .bundledOnly { + return lastPermissionFailure ?? ( + terminationStatus: -1, + stdout: "", + stderr: "Bundled cmux CLI command failed without an executable path" + ) + } + let fallbackArgs = ["cmux"] + args let fallbackResult = executeCmuxCommand( executablePath: "/usr/bin/env", @@ -627,6 +632,11 @@ final class MultiWindowNotificationsUITests: XCTestCase { return lastPermissionFailure ?? fallbackResult } + private enum CmuxCLIStrategy: Equatable { + case any + case bundledOnly + } + private func socketDiagnostics(from data: [String: String]) -> String { let pingResponse = data["socketPingResponse"].flatMap { $0.isEmpty ? nil : $0 } ?? "" return "mode=\(data["socketMode"] ?? "") running=\(data["socketIsRunning"] ?? "") " + @@ -635,15 +645,17 @@ final class MultiWindowNotificationsUITests: XCTestCase { "signals=\(data["socketFailureSignals"] ?? "")" } - private func resolveCmuxCLIPaths() -> [String] { + private func resolveCmuxCLIPaths(strategy: CmuxCLIStrategy) -> [String] { let fileManager = FileManager.default let env = ProcessInfo.processInfo.environment var candidates: [String] = [] var productDirectories: [String] = [] - for key in ["CMUX_UI_TEST_CLI_PATH", "CMUXTERM_CLI"] { - if let value = env[key], !value.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { - candidates.append(value) + if strategy == .any { + for key in ["CMUX_UI_TEST_CLI_PATH", "CMUXTERM_CLI"] { + if let value = env[key], !value.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { + candidates.append(value) + } } } @@ -664,12 +676,14 @@ final class MultiWindowNotificationsUITests: XCTestCase { productDirectories.append(contentsOf: inferredBuildProductsDirectories()) for productsDir in uniquePaths(productDirectories) { - appendCLIPathCandidates(fromProductsDirectory: productsDir, to: &candidates) + appendCLIPathCandidates(fromProductsDirectory: productsDir, strategy: strategy, to: &candidates) } candidates.append("/tmp/cmux-\(launchTag)/Build/Products/Debug/cmux DEV.app/Contents/Resources/bin/cmux") candidates.append("/tmp/cmux-\(launchTag)/Build/Products/Debug/cmux.app/Contents/Resources/bin/cmux") - candidates.append("/tmp/cmux-\(launchTag)/Build/Products/Debug/cmux") + if strategy == .any { + candidates.append("/tmp/cmux-\(launchTag)/Build/Products/Debug/cmux") + } var resolvedPaths: [String] = [] for path in uniquePaths(candidates) { @@ -697,21 +711,21 @@ final class MultiWindowNotificationsUITests: XCTestCase { } } - private func appendCLIPathCandidates(fromProductsDirectory productsDir: String, to candidates: inout [String]) { - candidates.append("\(productsDir)/cmux") + private func appendCLIPathCandidates( + fromProductsDirectory productsDir: String, + strategy: CmuxCLIStrategy, + to candidates: inout [String] + ) { candidates.append("\(productsDir)/cmux DEV.app/Contents/Resources/bin/cmux") candidates.append("\(productsDir)/cmux.app/Contents/Resources/bin/cmux") + if strategy == .any { + candidates.append("\(productsDir)/cmux") + } guard let entries = try? FileManager.default.contentsOfDirectory(atPath: productsDir) else { return } - for entry in entries.sorted() where entry == "cmux" { - let cliPath = URL(fileURLWithPath: productsDir) - .appendingPathComponent(entry) - .path - candidates.append(cliPath) - } for entry in entries.sorted() where entry.hasSuffix(".app") { let cliPath = URL(fileURLWithPath: productsDir) .appendingPathComponent(entry) @@ -719,6 +733,14 @@ final class MultiWindowNotificationsUITests: XCTestCase { .path candidates.append(cliPath) } + if strategy == .any { + for entry in entries.sorted() where entry == "cmux" { + let cliPath = URL(fileURLWithPath: productsDir) + .appendingPathComponent(entry) + .path + candidates.append(cliPath) + } + } } private func executeCmuxCommand( @@ -991,9 +1013,18 @@ final class MultiWindowNotificationsUITests: XCTestCase { } guard wrote else { return nil } + let deadline = Date().addingTimeInterval(2.0) var buf = [UInt8](repeating: 0, count: 4096) var accum = "" - while true { + while Date() < deadline { + var pollDescriptor = pollfd(fd: fd, events: Int16(POLLIN), revents: 0) + let ready = poll(&pollDescriptor, 1, 100) + if ready < 0 { + return nil + } + if ready == 0 { + continue + } let n = read(fd, &buf, buf.count) if n <= 0 { break } if let chunk = String(bytes: buf[0..