Improved Defer Fulfillment (#5127)

* improved defer fullfillment

* applied also for async streams

* fix xcodeproject

* better documentation

* restict deferFulfillment  only for Publisher which can never fail

ElementX.xcodeproj/project.pbxproj

@@ -823,6 +823,7 @@
 		8DF0EBD97753033C715D716E /* RoomFlowCoordinatorStateMachine.swift in Sources */ = {isa = PBXBuildFile; fileRef = 407C8DD85179D2DB896FC0FA /* RoomFlowCoordinatorStateMachine.swift */; };
 		8E650379587C31D7912ED67B /* UNNotification+Creator.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC0AEA686E425F86F6BA0404 /* UNNotification+Creator.swift */; };
 		8E7A902CA16E24928F83646C /* ElementCallServiceMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = E321E840DCC63790049984F4 /* ElementCallServiceMock.swift */; };
+		8ECD4727BA96EF64DFCEC18F /* DeferredFulfillment.swift in Sources */ = {isa = PBXBuildFile; fileRef = C39E32F0B876B962E418B5C2 /* DeferredFulfillment.swift */; };
 		8ED8AF57A06F5EE9978ED23F /* AuthenticationStartScreenViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8FB89DC7F9A4A91020037001 /* AuthenticationStartScreenViewModelTests.swift */; };
 		8F2FAA98457750D9D664136F /* LRUCache in Frameworks */ = {isa = PBXBuildFile; productRef = 1081D3630AAD3ACEDDEC3A98 /* LRUCache */; };
 		8F3AD08F2E706AA60F1A1D4D /* portrait_test_image.jpg in Resources */ = {isa = PBXBuildFile; fileRef = BC51BF90469412ABDE658CDD /* portrait_test_image.jpg */; };
@@ -1413,7 +1414,6 @@
 		F6F49E37272AD7397CD29A01 /* HomeScreenViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 505208F28007C0FEC14E1FF0 /* HomeScreenViewModelTests.swift */; };
 		F71C2B24AFB566119ACCDDA1 /* Secrets.swift in Sources */ = {isa = PBXBuildFile; fileRef = D3557ACB95D0F666EF5AF0CE /* Secrets.swift */; };
 		F7567DD6635434E8C563BF85 /* AnalyticsClientProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = E3B97591B2D3D4D67553506D /* AnalyticsClientProtocol.swift */; };
-		F769F921D7823C2F1CBB5047 /* DeferredFulfillment.swift in Sources */ = {isa = PBXBuildFile; fileRef = C39E32F0B876B962E418B5C2 /* DeferredFulfillment.swift */; };
 		F777C6FEE7D106136E2ED2B2 /* MessageForwardingScreenViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6F6E6EDC4BBF962B2ED595A4 /* MessageForwardingScreenViewModelTests.swift */; };
 		F78BAD28482A467287A9A5A3 /* EventBasedMessageTimelineItemProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0900BBF0A5D5D775E917C70 /* EventBasedMessageTimelineItemProtocol.swift */; };
 		F7932A3F075B0D3F24DEECB5 /* VoiceMessagePreviewComposer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2AE807361805463F5AEDD1CA /* VoiceMessagePreviewComposer.swift */; };
@@ -7623,6 +7623,7 @@
 				CD0088B763CD970CF1CBF8CB /* DateTests.swift in Sources */,
 				80F6C8EFCA4564B67F0D34B0 /* DeactivateAccountScreenViewModelTests.swift in Sources */,
 				34390DAE0C574DAD30CCA7D9 /* DeclineAndBlockScreenViewModelTests.swift in Sources */,
+				8ECD4727BA96EF64DFCEC18F /* DeferredFulfillment.swift in Sources */,
 				A583B70939707197B0B21DFC /* DeferredFulfillmentTests.swift in Sources */,
 				EDB6915EC953BB2A44AA608E /* EditRoomAddressScreenViewModelTests.swift in Sources */,
 				D820B3C223E4C2E77BB2A2BF /* ElementCallServiceTests.swift in Sources */,
@@ -8010,7 +8011,6 @@
 				0743CF689EBDAAF1CC0B4283 /* DeclineAndBlockScreenViewModel.swift in Sources */,
 				F7DA19B5122AD8FA8F91B753 /* DeclineAndBlockScreenViewModelProtocol.swift in Sources */,
 				EE8491AD81F47DF3C192497B /* DecorationTimelineItemProtocol.swift in Sources */,
-				F769F921D7823C2F1CBB5047 /* DeferredFulfillment.swift in Sources */,
 				5780E444F405AA1304E1C23E /* DeveloperOptionsScreen.swift in Sources */,
 				5DD85A0FE3D85AEC3C7EFE36 /* DeveloperOptionsScreenCoordinator.swift in Sources */,
 				6BAE34CFA9821709CFE61E50 /* DeveloperOptionsScreenHook.swift in Sources */,

ElementX/Sources/Other/DeferredFulfillment.swift

@@ -7,6 +7,7 @@
 //
 
 import Combine
+import Testing
 
 struct DeferredFulfillment<T> {
     let closure: () async throws -> T
@@ -18,225 +19,254 @@ struct DeferredFulfillment<T> {
 }
 
 struct DeferredFulfillmentError: Error {
-    enum Kind {
+    static func noOutput(message: String?, sourceLocation: SourceLocation) -> Self {
-        case noOutput
+        defer { Issue.record(Comment(rawValue: message ?? "No Output"), sourceLocation: sourceLocation) }
-        case unexpectedFulfillment
+        return .init()
     }
     
-    let kind: Kind
+    static func unexpectedFulfillment(message: String?, sourceLocation: SourceLocation) -> Self {
-    let message: String?
+        defer { Issue.record(Comment(rawValue: message ?? "Unexpected Fulfillment"), sourceLocation: sourceLocation) }
-    
+        return .init()
-    static func noOutput(message: String?) -> Self {
-        .init(kind: .noOutput, message: message)
     }
     
-    static func unexpectedFulfillment(message: String?) -> Self {
+    static var empty: Self {
-        .init(kind: .unexpectedFulfillment, message: message)
+        .init()
     }
 }
 
-/// Utility that assists in subscribing to a publisher and deferring the fulfilment and results until some other actions have been performed.
+/// Test utility that assists in subscribing to a publisher and deferring the fulfilment and results until some other actions have been performed.
 /// - Parameters:
 ///   - publisher: The publisher to wait on.
 ///   - timeout: A timeout after which we give up.
+///   - message: An optional message to include in the error if the condition is never met.
+///   - sourceLocation: The source location to attach to any recorded issues.
 ///   - until: callback that evaluates outputs until some condition is reached
 /// - Returns: The deferred fulfilment to be executed after some actions and that returns the result of the publisher.
-func deferFulfillment<P: Publisher>(_ publisher: P,
+func deferFulfillment<P: Publisher<P.Output, Never>>(_ publisher: P,
                                                      timeout: Duration = .seconds(10),
                                                      message: String? = nil,
+                                                     sourceLocation: SourceLocation = #_sourceLocation,
                                                      until condition: @escaping (P.Output) -> Bool) -> DeferredFulfillment<P.Output> {
-    var result: Result<P.Output, Error>?
+    let (stream, continuation) = AsyncStream<P.Output>.makeStream()
-    var hasFulfilled = false
     
     let cancellable = publisher
-        .sink { completion in
+        .sink { _ in
-            switch completion {
+            continuation.finish()
-            case .failure(let error):
-                result = .failure(error)
-                hasFulfilled = true
-            case .finished:
-                break
-            }
         } receiveValue: { value in
-            if condition(value), !hasFulfilled {
+            guard condition(value) else { return }
-                result = .success(value)
+            continuation.yield(value)
-                hasFulfilled = true
+            continuation.finish()
-            }
         }
     
-    return DeferredFulfillment<P.Output> {
+    return DeferredFulfillment {
-        let startTime = ContinuousClock.now
+        defer { cancellable.cancel() }
         
-        while !hasFulfilled {
+        return try await withThrowingTaskGroup(of: P.Output.self) { group in
-            await Task.yield()
+            group.addTask {
-            if ContinuousClock.now - startTime >= timeout {
+                for await result in stream {
-                break
+                    return result
                 }
+                guard !Task.isCancelled else {
+                    // Required to avoid a double recording of the issue in the case where the task is cancelled due to timeout.
+                    throw DeferredFulfillmentError.empty
+                }
+                throw DeferredFulfillmentError.noOutput(message: message, sourceLocation: sourceLocation)
             }
             
-        cancellable.cancel()
+            group.addTask {
-        
+                try await Task.sleep(for: timeout)
-        guard let unwrappedResult = result else {
+                throw DeferredFulfillmentError.noOutput(message: message, sourceLocation: sourceLocation)
-            throw DeferredFulfillmentError.noOutput(message: message)
+            }
+            
+            defer { group.cancelAll() }
+            return try #require(try await group.next())
         }
-        return try unwrappedResult.get()
     }
 }
 
-/// Utility that assists in observing an async sequence, deferring the fulfilment and results until some condition has been met.
+/// Test utility that assists in observing an async sequence, deferring the fulfilment and results until some condition has been met.
 /// - Parameters:
 ///   - asyncSequence: The sequence to wait on.
 ///   - timeout: A timeout after which we give up.
+///   - message: An optional message to include in the error if the condition is never met.
+///   - sourceLocation: The source location to attach to any recorded issues.
 ///   - until: callback that evaluates outputs until some condition is reached
 /// - Returns: The deferred fulfilment to be executed after some actions and that returns the result of the sequence.
 func deferFulfillment<Value>(_ asyncSequence: any AsyncSequence<Value, Never>,
                              timeout: Duration = .seconds(10),
                              message: String? = nil,
+                             sourceLocation: SourceLocation = #_sourceLocation,
                              until condition: @escaping (Value) -> Bool) -> DeferredFulfillment<Value> {
-    var result: Result<Value, Error>?
+    let (stream, continuation) = AsyncStream<Value>.makeStream()
-    var hasFulfilled = false
     
     let task = Task {
-        for await value in asyncSequence {
+        for await value in asyncSequence where condition(value) {
-            if condition(value), !hasFulfilled {
+            continuation.yield(value)
-                result = .success(value)
+            continuation.finish()
-                hasFulfilled = true
+            return
-            }
         }
+        continuation.finish()
     }
     
-    return DeferredFulfillment<Value> {
+    return DeferredFulfillment {
-        let startTime = ContinuousClock.now
+        defer { task.cancel() }
         
-        while !hasFulfilled {
+        return try await withThrowingTaskGroup(of: Value.self) { group in
-            await Task.yield()
+            group.addTask {
-            if ContinuousClock.now - startTime >= timeout {
+                for await value in stream {
-                break
+                    return value
                 }
+                guard !Task.isCancelled else {
+                    // Required to avoid a double recording of the issue in the case where the task is cancelled due to timeout.
+                    throw DeferredFulfillmentError.empty
+                }
+                throw DeferredFulfillmentError.noOutput(message: message, sourceLocation: sourceLocation)
+            }
+            group.addTask {
+                try await Task.sleep(for: timeout)
+                throw DeferredFulfillmentError.noOutput(message: message, sourceLocation: sourceLocation)
             }
             
-        task.cancel()
+            defer { group.cancelAll() }
             
-        guard let unwrappedResult = result else {
+            return try #require(try await group.next())
-            throw DeferredFulfillmentError.noOutput(message: message)
         }
-        return try unwrappedResult.get()
     }
 }
 
-/// Utility that assists in subscribing to a publisher and deferring the fulfilment and results until some other actions have been performed.
+/// Test utility that assists in subscribing to a publisher and deferring the fulfilment and results until some other actions have been performed.
 /// - Parameters:
 ///   - publisher: The publisher to wait on.
 ///   - keyPath: the key path for the expected values
 ///   - transitionValues: the values through which the keypath needs to transition through
 ///   - timeout: A timeout after which we give up.
+///   - sourceLocation: The source location to attach to any recorded issues.
 /// - Returns: The deferred fulfilment to be executed after some actions and that returns the result of the publisher.
-func deferFulfillment<P: Publisher, K: KeyPath<P.Output, V>, V: Equatable>(_ publisher: P,
+func deferFulfillment<P: Publisher<P.Output, Never>, K: KeyPath<P.Output, V>, V: Equatable>(_ publisher: P,
                                                                                             keyPath: K,
                                                                                             transitionValues: [V],
-                                                                           timeout: Duration = .seconds(10)) -> DeferredFulfillment<P.Output> {
+                                                                                            timeout: Duration = .seconds(10),
+                                                                                            message: String? = nil,
+                                                                                            sourceLocation: SourceLocation = #_sourceLocation) -> DeferredFulfillment<P.Output> {
     var expectedOrder = transitionValues
-    return deferFulfillment(publisher, timeout: timeout) { value in
+    return deferFulfillment(publisher, timeout: timeout, message: message, sourceLocation: sourceLocation) { value in
         let receivedValue = value[keyPath: keyPath]
         if let index = expectedOrder.firstIndex(where: { $0 == receivedValue }), index == 0 {
             expectedOrder.remove(at: index)
         }
-        
         return expectedOrder.isEmpty
     }
 }
 
-/// Utility that assists in subscribing to an async sequence and deferring the fulfilment and results until some other actions have been performed.
+/// Test utility that assists in subscribing to an async sequence and deferring the fulfilment and results until some other actions have been performed.
 /// - Parameters:
 ///   - asyncSequence: The sequence to wait on.
 ///   - transitionValues: the values through which the sequence needs to transition through
 ///   - timeout: A timeout after which we give up.
+///   - sourceLocation: The source location to attach to any recorded issues.
 /// - Returns: The deferred fulfilment to be executed after some actions and that returns the result of the sequence.
 func deferFulfillment<Value: Equatable>(_ asyncSequence: any AsyncSequence<Value, Never>,
                                         transitionValues: [Value],
-                                        timeout: Duration = .seconds(10)) -> DeferredFulfillment<Value> {
+                                        timeout: Duration = .seconds(10),
+                                        message: String? = nil,
+                                        sourceLocation: SourceLocation = #_sourceLocation) -> DeferredFulfillment<Value> {
     var expectedOrder = transitionValues
-    return deferFulfillment(asyncSequence, timeout: timeout) { value in
+    return deferFulfillment(asyncSequence, timeout: timeout, message: message, sourceLocation: sourceLocation) { value in
         if let index = expectedOrder.firstIndex(where: { $0 == value }), index == 0 {
             expectedOrder.remove(at: index)
         }
-        
         return expectedOrder.isEmpty
     }
 }
 
-/// Utility that assists in subscribing to a publisher and deferring the failure for a particular value until some other actions have been performed.
+/// Test utility that assists in subscribing to a publisher and deferring the failure for a particular value until some other actions have been performed.
 /// - Parameters:
 ///   - publisher: The publisher to wait on.
 ///   - timeout: A timeout after which we give up.
+///   - message: An optional message to include in the error if the condition is unexpectedly met.
+///   - sourceLocation: The source location to attach to any recorded issues.
 ///   - until: callback that evaluates outputs until some condition is reached
 /// - Returns: The deferred fulfilment to be executed after some actions. The publisher's result is not returned from this fulfilment.
-func deferFailure<P: Publisher>(_ publisher: P,
+func deferFailure<P: Publisher<P.Output, Never>>(_ publisher: P,
                                                  timeout: Duration,
                                                  message: String? = nil,
+                                                 sourceLocation: SourceLocation = #_sourceLocation,
                                                  until condition: @escaping (P.Output) -> Bool) -> DeferredFulfillment<Void> where P.Failure == Never {
-    var hasFulfilled = false
+    let (stream, continuation) = AsyncStream<Void>.makeStream()
+    
     let cancellable = publisher
         .sink { value in
-            if condition(value), !hasFulfilled {
+            guard condition(value) else { return }
-                hasFulfilled = true
+            continuation.yield(())
-            }
+            continuation.finish()
         }
     
-    return DeferredFulfillment<Void> {
+    return DeferredFulfillment {
-        let startTime = ContinuousClock.now
+        defer { cancellable.cancel() }
         
-        while !hasFulfilled {
+        try await withThrowingTaskGroup(of: Void.self) { group in
-            await Task.yield()
+            // If the condition fires before timeout, that's the unexpected failure.
-            if ContinuousClock.now - startTime >= timeout {
+            group.addTask {
-                break
+                for await _ in stream {
+                    throw DeferredFulfillmentError.unexpectedFulfillment(message: message, sourceLocation: sourceLocation)
                 }
+                // Stream finished without condition firing — this shouldn't happen
+                // but is safe to treat as success.
+            }
+            // Timeout elapsing without the condition firing = success.
+            group.addTask {
+                try await Task.sleep(for: timeout)
             }
             
-        cancellable.cancel()
+            defer { group.cancelAll() }
             
-        // For deferFailure, if hasFulfilled is true, it means the condition was met (which is a failure)
+            return try #require(try await group.next())
-        if hasFulfilled {
-            throw DeferredFulfillmentError.unexpectedFulfillment(message: message)
         }
     }
 }
 
-/// Utility that assists in subscribing to an async sequence and deferring the failure for a particular value until some other actions have been performed.
+/// Test utility that assists in subscribing to an async sequence and deferring the failure for a particular value until some other actions have been performed.
 /// - Parameters:
 ///   - asyncSequence: The sequence to wait on.
 ///   - timeout: A timeout after which we give up.
+///   - message: An optional message to include in the error if the condition is unexpectedly met.
+///   - sourceLocation: The source location to attach to any recorded issues.
 ///   - until: callback that evaluates outputs until some condition is reached
 /// - Returns: The deferred fulfilment to be executed after some actions. The sequence's result is not returned from this fulfilment.
 func deferFailure<Value>(_ asyncSequence: any AsyncSequence<Value, Never>,
                          timeout: Duration,
                          message: String? = nil,
+                         sourceLocation: SourceLocation = #_sourceLocation,
                          until condition: @escaping (Value) -> Bool) -> DeferredFulfillment<Void> {
-    var hasFulfilled = false
+    let (stream, continuation) = AsyncStream<Void>.makeStream()
     
     let task = Task {
-        for await value in asyncSequence {
+        for await value in asyncSequence where condition(value) {
-            if condition(value), !hasFulfilled {
+            continuation.yield(())
-                hasFulfilled = true
+            continuation.finish()
-            }
+            return
         }
+        continuation.finish()
     }
     
-    return DeferredFulfillment<Void> {
+    return DeferredFulfillment {
-        let startTime = ContinuousClock.now
+        defer { task.cancel() }
         
-        while !hasFulfilled {
+        try await withThrowingTaskGroup(of: Void.self) { group in
-            await Task.yield()
+            // If the condition fires before timeout, that's the unexpected failure.
-            if ContinuousClock.now - startTime >= timeout {
+            group.addTask {
-                break
+                for await _ in stream {
+                    throw DeferredFulfillmentError.unexpectedFulfillment(message: message, sourceLocation: sourceLocation)
                 }
             }
+            // Timeout elapsing without the condition firing = success.
+            group.addTask {
+                try await Task.sleep(for: timeout)
+            }
             
-        task.cancel()
+            defer { group.cancelAll() }
             
-        // For deferFailure, if hasFulfilled is true, it means the condition was met (which is a failure)
+            return try #require(try await group.next())
-        if hasFulfilled {
-            throw DeferredFulfillmentError.unexpectedFulfillment(message: message)
         }
     }
 }

ElementX/SupportingFiles/target.yml

@@ -258,6 +258,7 @@ targets:
     - path: ../Sources
       excludes:
         - Other/Extensions/XCTestCase.swift
+        - Other/DeferredFulfillment.swift
         - Other/Extensions/XCUIElement.swift
     - path: ../../Secrets/Secrets.swift
     - path: ../Resources

UnitTests/SupportingFiles/target.yml

@@ -52,6 +52,7 @@ targets:
     - path: ../../DevelopmentAssets
     - path: ../../ElementX/Sources/Other/Extensions/Publisher.swift
     - path: ../../ElementX/Sources/Other/Extensions/XCTestCase.swift
+    - path: ../../ElementX/Sources/Other/DeferredFulfillment.swift
     - path: ../../ElementX/Sources/Other/InfoPlistReader.swift
     - path: ../../Tools/Scripts/Templates/SimpleScreenExample/Tests/Unit