Improve management of syncs to fix flaky test

Author: joshheald

Modules/Tests/PointOfSaleTests/Controllers/PointOfSaleOrderControllerTests.swift

@@ -53,52 +53,57 @@ struct PointOfSaleOrderControllerTests {
         #expect(mockOrderService.syncOrderWasCalled == false)
     }
 
-    @Test func syncOrder_when_already_syncing_doesnt_call_orderService() async throws {
+    @Test @MainActor func syncOrder_when_already_syncing_doesnt_call_orderService() async throws {
         // Given
         let sut = PointOfSaleOrderController(orderService: mockOrderService,
                                              receiptSender: mockReceiptSender,
                                              currencySettingsProvider: MockCurrencySettingsProvider(),
                                              analytics: MockPOSAnalytics())
-        mockOrderService.simulateSyncing = true
 
-        // Use a continuation to wait until the first sync has definitely started
+        // Block the sync so it doesn't complete until we manually resume it
+        mockOrderService.blockNextSync()
+
+        // Start the first sync in a detached task so it runs concurrently
+        let firstSyncTask = Task.detached {
+            await sut.syncOrder(for: Cart(purchasableItems: [makeItem(quantity: 1)]), retryHandler: {})
+        }
+
+        // Wait for the order state to actually become syncing
         await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
-            Task { @MainActor in
-                // Start observing state changes
-                @Sendable func observeOrderState() {
-                    withObservationTracking {
-                        _ = sut.orderState
-                    } onChange: {
-                        Task { @MainActor in
-                            if sut.orderState.isSyncing {
-                                // State is now syncing, resume the test
-                                continuation.resume()
-                            } else {
-                                // Keep observing
-                                observeOrderState()
-                            }
+            @Sendable func observeOrderState() {
+                withObservationTracking {
+                    _ = sut.orderState
+                } onChange: {
+                    Task { @MainActor in
+                        if sut.orderState.isSyncing {
+                            continuation.resume()
+                        } else {
+                            observeOrderState()
                         }
                     }
                 }
-                observeOrderState()
-
-                // Start the first sync in a background task
-                Task {
-                    await sut.syncOrder(for: Cart(purchasableItems: [makeItem(quantity: 1)]), retryHandler: {})
-                }
             }
+            observeOrderState()
         }
 
+        // Verify the state is actually syncing
+        #expect(sut.orderState.isSyncing == true)
+        #expect(mockOrderService.syncOrderWasCalled == true)
+
         // Reset the flag after confirming the sync has started
         mockOrderService.syncOrderWasCalled = false
 
-        // When - try to sync while already syncing
+        // When - try to sync while the first sync is still in progress
         await sut.syncOrder(for: Cart(purchasableItems: [makeItem(quantity: 2),
                                                           makeItem(quantity: 5)]),
                             retryHandler: {})
 
         // Then - the second sync should have been skipped
         #expect(mockOrderService.syncOrderWasCalled == false)
+
+        // Cleanup - allow the first sync to complete
+        mockOrderService.resumeBlockedSync()
+        _ = await firstSyncTask.result
     }
 
     @Test func syncOrder_with_no_previous_order_calls_orderService() async throws {

Modules/Tests/PointOfSaleTests/Mocks/MockPOSOrderService.swift

@@ -14,12 +14,33 @@ class MockPOSOrderService: POSOrderServiceProtocol {
     var spySyncOrderCurrency: CurrencyCode?
     var spyCashPaymentChangeDueAmount: String?
 
+    // For controlling sync timing in tests
+    private var syncContinuation: CheckedContinuation<Void, Never>?
+    private var shouldBlockSync = false
+
+    /// Blocks the next sync operation until `resumeBlockedSync()` is called
+    func blockNextSync() {
+        shouldBlockSync = true
+    }
+
+    /// Resumes a blocked sync operation
+    func resumeBlockedSync() {
+        syncContinuation?.resume()
+        syncContinuation = nil
+        shouldBlockSync = false
+    }
+
     func syncOrder(cart: Yosemite.POSCart,
                    currency: CurrencyCode) async throws -> Yosemite.Order {
         syncOrderWasCalled = true
         spySyncOrderCurrency = currency
 
-        if simulateSyncing {
+        if shouldBlockSync {
+            shouldBlockSync = false // Only block the first call
+            await withCheckedContinuation { continuation in
+                syncContinuation = continuation
+            }
+        } else if simulateSyncing {
             try await Task.sleep(nanoseconds: UInt64(1 * Double(NSEC_PER_SEC)))
         }