Merge pull request #128 from darkroomengineering/ci/harden-timing-tests

test: harden timing-sensitive unit tests for slow CI runners

Author: arzafran

programaTests/AppDelegateShortcutRoutingTests.swift

@@ -4358,7 +4358,9 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
         // apart, since the SwiftUI-hosted field may not be laid out yet on the first
         // attempt). A single fixed 0.05s spin can land before that loop finishes,
         // causing rare flakes. Poll instead of assuming one spin is enough.
-        let searchFieldDeadline = Date(timeIntervalSinceNow: 2.0)
+        // See `ciScale` (TabManagerUnitTests.swift): scale the poll deadline under CI,
+        // where this retry loop can legitimately need longer than a fast local machine.
+        let searchFieldDeadline = Date(timeIntervalSinceNow: 2.0 * ciScale)
         var searchField: NSTextField?
         while Date() < searchFieldDeadline {
             RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
@@ -4397,7 +4399,17 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
         }
 
         window.sendEvent(keyDown)
-        RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
+        // The focus-repair path triggered by sendEvent runs asynchronously (see the
+        // searchFieldDeadline poll above for the same class of flake). A single fixed
+        // 0.05s spin can land before it completes under a full serial suite run with
+        // CPU contention from hundreds of prior tests — poll instead.
+        let repairDeadline = Date(timeIntervalSinceNow: 2.0 * ciScale)
+        while Date() < repairDeadline {
+            RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
+            if firstResponderOwnsTextField(window.firstResponder, textField: searchField) {
+                break
+            }
+        }
 
         XCTAssertTrue(
             firstResponderOwnsTextField(window.firstResponder, textField: searchField),

programaTests/BrowserConfigTests.swift

@@ -2041,10 +2041,24 @@ final class BrowserDeveloperToolsVisibilityPersistenceTests: XCTestCase {
         return nil
     }
 
-    private func waitForDeveloperToolsTransitions() {
+    private func waitForDeveloperToolsTransitions(
+        timeout: TimeInterval = 2.0,
+        until condition: (() -> Bool)? = nil
+    ) {
         // Give real headroom under a full serial suite run, where the main queue can
-        // carry a genuine backlog from other tests' pending async work.
-        RunLoop.current.run(until: Date().addingTimeInterval(2.0))
+        // carry a genuine backlog from other tests' pending async work. When a
+        // completion condition is supplied, poll for it directly instead of trusting
+        // a fixed spin alone — a queued transition (e.g. toggleDeveloperTools' coalesced
+        // hide) can still be in flight when the spin ends under CI contention.
+        guard let condition else {
+            RunLoop.current.run(until: Date().addingTimeInterval(timeout))
+            return
+        }
+        let deadline = Date().addingTimeInterval(timeout)
+        while Date() < deadline {
+            RunLoop.current.run(until: Date().addingTimeInterval(0.05))
+            if condition() { return }
+        }
     }
 
     private func findWindowBrowserSlotView(in root: NSView) -> WindowBrowserSlotView? {
@@ -2192,7 +2206,9 @@ final class BrowserDeveloperToolsVisibilityPersistenceTests: XCTestCase {
         XCTAssertEqual(inspector.showCount, 1)
         XCTAssertEqual(inspector.closeCount, 0)
 
-        waitForDeveloperToolsTransitions()
+        waitForDeveloperToolsTransitions(timeout: 10.0) {
+            inspector.closeCount == 1
+        }
 
         XCTAssertFalse(panel.isDeveloperToolsVisible())
         XCTAssertEqual(inspector.showCount, 1)

programaTests/NotificationAndMenuBarTests.swift

@@ -489,7 +489,10 @@ final class NotificationDockBadgeTests: XCTestCase {
             },
             object: NSObject()
         )
-        XCTAssertEqual(XCTWaiter().wait(for: [commandFinished], timeout: 2.0), .completed)
+        // This spawns a real shell subprocess to write commandOutputURL. Under a full
+        // serial suite run with CPU contention from hundreds of prior tests, process
+        // scheduling can legitimately take longer than a couple seconds.
+        XCTAssertEqual(XCTWaiter().wait(for: [commandFinished], timeout: 15.0), .completed)
         XCTAssertTrue(deliveredNotificationIDs.isEmpty)
 
         let output = try String(contentsOf: commandOutputURL, encoding: .utf8)
@@ -577,9 +580,13 @@ final class NotificationDockBadgeTests: XCTestCase {
         )
 
         store.promptToEnableNotificationsForTesting()
+        // See the identical comment on this pattern in
+        // testNotificationSettingsPromptRetriesUntilWindowExists below: under a full
+        // serial suite run the main queue can carry a real backlog from other tests'
+        // pending async work, so give this more than the bare minimum headroom.
         let drained = expectation(description: "main queue drained")
         DispatchQueue.main.async { drained.fulfill() }
-        wait(for: [drained], timeout: 1.0)
+        wait(for: [drained], timeout: 5.0)
 
         XCTAssertEqual(alertSpy.beginSheetModalCallCount, 1)
         XCTAssertEqual(alertSpy.runModalCallCount, 0)

programaTests/TabManagerUnitTests.swift

@@ -15,6 +15,12 @@ import UserNotifications
 
 let lastSurfaceCloseShortcutDefaultsKey = "closeWorkspaceOnLastSurfaceShortcut"
 
+// GitHub Actions always sets CI=true, and shared macos-26 runners can be under enough
+// scheduler/subprocess contention (see GitMetadataProber's git/gh probing) that fixed
+// local-machine timeouts flake even when generously sized. Scale wait budgets up under
+// CI only, so local runs stay fast.
+let ciScale: Double = ProcessInfo.processInfo.environment["CI"] != nil ? 4.0 : 1.0
+
 func drainMainQueue() {
     // A fixed 1s wait isn't reliable headroom for this main-queue turn under a full
     // serial suite run, where the queue can carry a real backlog from hundreds of prior
@@ -38,8 +44,10 @@ private func waitForCondition(
         return true
     }
 
+    // Scale the caller's timeout (default or explicit) under CI — see `ciScale`.
+    let scaledTimeout = timeout * ciScale
     let expectation = XCTestExpectation(description: "wait for condition")
-    let deadline = Date().addingTimeInterval(timeout)
+    let deadline = Date().addingTimeInterval(scaledTimeout)
 
     func poll() {
         if condition() {
@@ -56,7 +64,7 @@ private func waitForCondition(
         poll()
     }
 
-    let result = XCTWaiter().wait(for: [expectation], timeout: timeout + pollInterval + 0.1)
+    let result = XCTWaiter().wait(for: [expectation], timeout: scaledTimeout + pollInterval + 0.1)
     if result != .completed {
         XCTFail("Timed out waiting for condition", file: file, line: line)
         return false
@@ -304,6 +312,76 @@ final class TabManagerWorkspaceOwnershipTests: XCTestCase {
 
 @MainActor
 final class TabManagerPullRequestProbeTests: XCTestCase {
+    // GitMetadataProber shells out to the ambient `gh` CLI to resolve pull-request
+    // metadata for any TabManager-created workspace whose directory resolves to a real
+    // git repo on a non-main/master branch (see
+    // GitMetadataProber.workspacePullRequestSnapshot). On CI that directory can be this
+    // very checkout (a real repo, checked out to a non-main PR branch), so a
+    // `TabManager()`'s default workspace can trigger a real, unauthenticated `gh pr
+    // list`/`gh pr checks` call that blocks for up to its 5s timeout per repo remote —
+    // on the same per-instance serial probe queue these tests are themselves waiting
+    // on. Stub `gh` on PATH for the duration of each test so these fixtures never
+    // depend on a real `gh` binary or network access, mirroring the PATH-stub pattern
+    // already used for shell-integration tests (e.g. GhosttyConfigTests' fake
+    // `tmux`/`programa` binaries).
+    private var originalPATHForGHStub: String?
+    private var ghStubBinDirectory: URL?
+
+    override func setUp() {
+        super.setUp()
+        let fileManager = FileManager.default
+        let currentPath = ProcessInfo.processInfo.environment["PATH"] ?? ""
+        originalPATHForGHStub = currentPath
+
+        let binDir = fileManager.temporaryDirectory.appendingPathComponent(
+            "cmux-gh-stub-\(UUID().uuidString)",
+            isDirectory: true
+        )
+        do {
+            try fileManager.createDirectory(at: binDir, withIntermediateDirectories: true)
+            let ghStub = binDir.appendingPathComponent("gh", isDirectory: false)
+            // Deterministic, network-free stand-in for the two `gh` invocations
+            // GitMetadataProber makes (`pr list`, `pr checks`). Both fast-exit with an
+            // empty JSON array, which the prober decodes as "no pull request found" —
+            // exactly right for these branch/metadata-focused tests, none of which
+            // assert on actual PR data.
+            let script = """
+            #!/bin/sh
+            case "$1 $2" in
+              "pr list")
+                echo '[]'
+                exit 0
+                ;;
+              "pr checks")
+                echo '[]'
+                exit 0
+                ;;
+              *)
+                exit 1
+                ;;
+            esac
+            """
+            try script.write(to: ghStub, atomically: true, encoding: .utf8)
+            try fileManager.setAttributes([.posixPermissions: 0o755], ofItemAtPath: ghStub.path)
+            ghStubBinDirectory = binDir
+            setenv("PATH", "\(binDir.path):\(currentPath)", 1)
+        } catch {
+            XCTFail("Failed to install gh stub: \(error)")
+        }
+    }
+
+    override func tearDown() {
+        if let originalPATHForGHStub {
+            setenv("PATH", originalPATHForGHStub, 1)
+        }
+        if let ghStubBinDirectory {
+            try? FileManager.default.removeItem(at: ghStubBinDirectory)
+        }
+        originalPATHForGHStub = nil
+        ghStubBinDirectory = nil
+        super.tearDown()
+    }
+
     func testGitHubRepositorySlugsPrioritizeUpstreamThenOriginAndDeduplicate() {
         let output = """
         origin https://github.com/austinwang/cmux.git (fetch)
@@ -526,7 +604,10 @@ final class TabManagerPullRequestProbeTests: XCTestCase {
 
         XCTAssertNotEqual(manager.selectedTabId, backgroundWorkspace.id)
         XCTAssertTrue(
-            waitForCondition {
+            // Real git subprocess + GitMetadataProber round trip, not a fixed dispatch
+            // delay — needs the same headroom as testRemoteSplitSkipsInitialGitMetadataProbe
+            // below under a full serial suite run's CPU contention.
+            waitForCondition(timeout: 12.0) {
                 backgroundWorkspace.panelGitBranches[backgroundPanelId]?.branch == "main"
             }
         )
@@ -570,7 +651,9 @@ final class TabManagerPullRequestProbeTests: XCTestCase {
         manager.refreshTrackedWorkspaceGitMetadataForTesting()
 
         XCTAssertTrue(
-            waitForCondition {
+            // See timeout comment on testInheritedBackgroundWorkspaceFetchesGitBranchWithoutSelection
+            // above: real git subprocess + probe round trip needs headroom under CI load.
+            waitForCondition(timeout: 12.0) {
                 workspace.panelGitBranches[panelId]?.branch == "feature/sidebar-live-refresh"
             }
         )
@@ -613,7 +696,9 @@ final class TabManagerPullRequestProbeTests: XCTestCase {
         manager.refreshTrackedWorkspaceGitMetadataForTesting()
 
         XCTAssertTrue(
-            waitForCondition {
+            // See timeout comment on testInheritedBackgroundWorkspaceFetchesGitBranchWithoutSelection
+            // above: real git subprocess + probe round trip needs headroom under CI load.
+            waitForCondition(timeout: 12.0) {
                 workspace.panelGitBranches[panelId]?.branch == "main"
             }
         )
@@ -733,7 +818,9 @@ final class TabManagerPullRequestProbeTests: XCTestCase {
         manager.refreshTrackedWorkspaceGitMetadataForTesting()
 
         XCTAssertTrue(
-            waitForCondition {
+            // See timeout comment on testInheritedBackgroundWorkspaceFetchesGitBranchWithoutSelection
+            // above: real git subprocess + probe round trip needs headroom under CI load.
+            waitForCondition(timeout: 12.0) {
                 workspace.panelGitBranches[panelId]?.branch == "main"
                     && workspace.panelPullRequests[panelId] == nil
             }

programaTests/TerminalAndGhosttyTests.swift

@@ -2612,12 +2612,17 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
         contentView.layoutSubtreeIfNeeded()
         hostedView.setVisibleInUI(true)
         hostedView.setActive(true)
-        RunLoop.current.run(until: Date().addingTimeInterval(0.05))
+        RunLoop.current.run(until: Date().addingTimeInterval(0.3))
 
         let searchState = TerminalSurface.SearchState(needle: "")
         surface.searchState = searchState
         hostedView.setSearchOverlay(searchState: searchState)
-        RunLoop.current.run(until: Date().addingTimeInterval(0.05))
+        // Poll for the deferred mount instead of trusting one fixed spin — see the
+        // identical rationale on the other waitUntil("search overlay to mount") call
+        // sites in this class.
+        waitUntil(timeout: 3.0, description: "find overlay to mount") {
+            self.findEditableTextField(in: hostedView) != nil
+        }
 
         guard let searchField = findEditableTextField(in: hostedView) else {
             XCTFail("Expected mounted find text field")
@@ -2661,7 +2666,9 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
 
         NSApp.sendEvent(escapeKeyDown)
         NSApp.sendEvent(escapeKeyUp)
-        RunLoop.current.run(until: Date().addingTimeInterval(0.05))
+        waitUntil(timeout: 3.0, description: "find overlay to dismiss after Escape") {
+            surface.searchState == nil
+        }
 
         XCTAssertNil(surface.searchState, "Escape should dismiss find overlay when search text is empty")
         XCTAssertEqual(
@@ -2730,7 +2737,9 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
         XCTAssertEqual(state.frame.size.height, 112, accuracy: 0.5)
 
         hostedView.setDropZoneOverlay(zone: nil)
-        RunLoop.current.run(until: Date().addingTimeInterval(0.25))
+        waitUntil(timeout: 3.0, description: "drop zone overlay to hide") {
+            hostedView.debugDropZoneOverlayState().isHidden
+        }
         XCTAssertTrue(hostedView.debugDropZoneOverlayState().isHidden)
     }
 
@@ -3473,7 +3482,18 @@ final class TerminalWindowPortalLifecycleTests: XCTestCase {
             window.displayIfNeeded()
         }
 
-        RunLoop.current.run(until: Date().addingTimeInterval(0.3))
+        // The queued layout shift (posted via DispatchQueue.main.async above) needs a
+        // main-run-loop turn to execute. A single fixed 0.3s spin can land before it
+        // has settled under a full serial suite run's CPU contention — poll for the
+        // real completion signal (the anchor frame actually reflecting the shift)
+        // instead of assuming one spin is enough. See `ciScale` (TabManagerUnitTests.swift).
+        let layoutShiftDeadline = Date(timeIntervalSinceNow: 0.3 * ciScale)
+        while Date() < layoutShiftDeadline {
+            RunLoop.current.run(until: Date(timeIntervalSinceNow: 0.02))
+            if anchor.convert(anchor.bounds, to: nil).minX > originalAnchorFrameInWindow.minX + 1 {
+                break
+            }
+        }
 
         let shiftedAnchorFrameInWindow = anchor.convert(anchor.bounds, to: nil)
         XCTAssertGreaterThan(
@@ -3494,6 +3514,22 @@ final class TerminalWindowPortalLifecycleTests: XCTestCase {
             x: (originalAnchorFrameInWindow.maxX + shiftedAnchorFrameInWindow.maxX) / 2,
             y: shiftedAnchorFrameInWindow.midY
         )
+
+        // The layout shift settling (above) and the *separately* scheduled external
+        // geometry sync racing against it are two independent async operations — the
+        // anchor moving doesn't mean the queued sync has also caught up and re-bound
+        // the portal to the new position yet. Poll for that actual completion signal
+        // (the hit-test state the assertions below check) rather than checking it once
+        // immediately after only the first operation has been confirmed.
+        let externalSyncDeadline = Date(timeIntervalSinceNow: 0.3 * ciScale)
+        while Date() < externalSyncDeadline {
+            RunLoop.current.run(until: Date(timeIntervalSinceNow: 0.02))
+            if TerminalWindowPortalRegistry.terminalViewAtWindowPoint(retiredStaleWindowPoint, in: window) == nil,
+               TerminalWindowPortalRegistry.terminalViewAtWindowPoint(shiftedWindowPoint, in: window) != nil {
+                break
+            }
+        }
+
         XCTAssertNil(
             TerminalWindowPortalRegistry.terminalViewAtWindowPoint(retiredStaleWindowPoint, in: window),
             "The queued external sync should wait until the later layout shift settles, clearing the stale portal location"