From ca7a047ba9958ef194ab5a0c589ff79c814a2ae1 Mon Sep 17 00:00:00 2001 From: Katherine Bertelsen Date: Wed, 6 May 2026 08:58:26 -0500 Subject: [PATCH 1/3] [PM-36604] test: Fix flaky WatchServiceTests --- .../Platform/Services/WatchServiceTests.swift | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift b/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift index 78e977b688..53109d9f25 100644 --- a/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift @@ -12,6 +12,10 @@ import WatchConnectivity // MARK: - WatchServiceTests +/// Tests create `DefaultWatchService` instances that run background tasks on the cooperative thread +/// pool. Running tests in parallel causes thread contention that can push async chains past the +/// CI 1-second per-test time limit, so the suite is serialized. +@Suite(.serialized) @MainActor struct WatchServiceTests { // swiftlint:disable:this type_body_length // MARK: Properties @@ -201,15 +205,22 @@ struct WatchServiceTests { // swiftlint:disable:this type_body_length stateService.connectToWatchSubject.send(("1", true)) } + // Capture the context at callback time rather than reading the shared property afterwards. + // A task from the first sync could otherwise overwrite it between the resume() call and the + // assertion below. + var capturedContext: [String: Any]? await withContinuationTimeout { resume in - watchSession.updateApplicationContextClosure = { _ in resume() } + watchSession.updateApplicationContextClosure = { context in + capturedContext = context + resume() + } // Now update with shouldConnect = false — the existing session will be used. stateService.connectToWatchByUserId["1"] = false stateService.connectToWatchSubject.send(("1", false)) } - let dto = try decodedDTO(from: watchSession.updateApplicationContextReceivedApplicationContext) + let dto = try decodedDTO(from: capturedContext) #expect(dto.state == .needSetup) } @@ -345,12 +356,22 @@ struct WatchServiceTests { // swiftlint:disable:this type_body_length let decryptCountAfterSetup = decryptCallCount #expect(decryptCountAfterSetup > 0) - // Lock the vault and trigger another sync. + // Lock the vault and trigger another sync (no updateApplicationContext expected). vaultTimeoutService.isClientLocked["1"] = true stateService.connectToWatchSubject.send(("1", true)) - try await Task.sleep(nanoseconds: 10_000_000) - // Confirm no additional decrypt attempts. + // Use a barrier sync to guarantee sequential processing: unlock the vault and set + // shouldConnect = false so the next sync sends .needSetup without decrypting ciphers. + // By the time the barrier callback fires, the locked sync above has already been processed. + await withContinuationTimeout { resume in + watchSession.updateApplicationContextClosure = { _ in resume() } + + vaultTimeoutService.isClientLocked["1"] = false + stateService.connectToWatchByUserId["1"] = false + stateService.connectToWatchSubject.send(("1", false)) + } + + // Confirm no additional decrypt attempts occurred during the locked sync. #expect(decryptCallCount == decryptCountAfterSetup) } @@ -365,12 +386,16 @@ struct WatchServiceTests { // swiftlint:disable:this type_body_length vaultTimeoutService.isClientLocked["1"] = true stateService.connectToWatchByUserId["1"] = true stateService.connectToWatchSubject.send(("1", true)) - try await Task.sleep(nanoseconds: 10_000_000) - #expect(watchSession.updateApplicationContextCallsCount == 0) // Unlock the vault — the publisher emits, triggering a fresh sync. + // The locked and unlock events are processed sequentially by the service's listener loop, + // so the unlock sync fires after the locked sync. Asserting callsCount == 1 inside the + // closure confirms the locked sync produced no calls before this one. await withContinuationTimeout { resume in - watchSession.updateApplicationContextClosure = { _ in resume() } + watchSession.updateApplicationContextClosure = { _ in + #expect(watchSession.updateApplicationContextCallsCount == 1) + resume() + } cipherService.ciphersSubject.send([.fixture()]) vaultTimeoutService.isClientLocked["1"] = false From 48751cafce81f420e0257ebe37a13419062d4ad1 Mon Sep 17 00:00:00 2001 From: Katherine Bertelsen Date: Fri, 8 May 2026 09:55:31 -0500 Subject: [PATCH 2/3] [PM-36604] test: Add timeLimit experiment tests --- .../Platform/Services/WatchServiceTests.swift | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift b/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift index 53109d9f25..24299445a1 100644 --- a/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift @@ -431,6 +431,22 @@ struct WatchServiceTests { // swiftlint:disable:this type_body_length #expect(watchSession.updateApplicationContextCallsCount == 2) } + // MARK: Time Limit Experiments + + /// Sleeps for 30 seconds — expected to pass under a 1-minute `.timeLimit`. + @available(iOS 16.0, *) + @Test(.timeLimit(.minutes(1))) + func timeLimitExperiment_30SecondSleep_passes() async throws { + try await Task.sleep(for: .seconds(30)) + } + + /// Sleeps for 90 seconds — expected to fail under a 1-minute `.timeLimit`. + @available(iOS 16.0, *) + @Test(.timeLimit(.minutes(1))) + func timeLimitExperiment_90SecondSleep_fails() async throws { + try await Task.sleep(for: .seconds(90)) + } + // MARK: Helpers /// Decodes a `WatchDTO` from the last-sent `updateApplicationContext` call. From 0a5b6ae984ea561a31cb5d485ed88f76f8b08dd8 Mon Sep 17 00:00:00 2001 From: Katherine Bertelsen Date: Mon, 11 May 2026 12:58:33 -0500 Subject: [PATCH 3/3] [PM-36604] test: Remove serialized suite and experiment tests --- .../Platform/Services/WatchServiceTests.swift | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift b/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift index 24299445a1..40a9c87a24 100644 --- a/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/WatchServiceTests.swift @@ -12,10 +12,6 @@ import WatchConnectivity // MARK: - WatchServiceTests -/// Tests create `DefaultWatchService` instances that run background tasks on the cooperative thread -/// pool. Running tests in parallel causes thread contention that can push async chains past the -/// CI 1-second per-test time limit, so the suite is serialized. -@Suite(.serialized) @MainActor struct WatchServiceTests { // swiftlint:disable:this type_body_length // MARK: Properties @@ -431,22 +427,6 @@ struct WatchServiceTests { // swiftlint:disable:this type_body_length #expect(watchSession.updateApplicationContextCallsCount == 2) } - // MARK: Time Limit Experiments - - /// Sleeps for 30 seconds — expected to pass under a 1-minute `.timeLimit`. - @available(iOS 16.0, *) - @Test(.timeLimit(.minutes(1))) - func timeLimitExperiment_30SecondSleep_passes() async throws { - try await Task.sleep(for: .seconds(30)) - } - - /// Sleeps for 90 seconds — expected to fail under a 1-minute `.timeLimit`. - @available(iOS 16.0, *) - @Test(.timeLimit(.minutes(1))) - func timeLimitExperiment_90SecondSleep_fails() async throws { - try await Task.sleep(for: .seconds(90)) - } - // MARK: Helpers /// Decodes a `WatchDTO` from the last-sent `updateApplicationContext` call.