Test cleanup (apple#54)

* Test cleanup - Remove all >= Swift 5.5 checks in tests since project requires 5.6 now - Wait for taskGroup.isCancelled to become true in hope to reduce flakiness (resolves apple#53) * Remove unused variable * Fix 5.6 CI
PureSwift · Feb 7, 2024 · cad7a22 · cad7a22
1 parent 7c4d776
commit cad7a22
Show file tree

Hide file tree

Showing 10 changed files with 119 additions and 252 deletions.
diff --git a/Sources/ServiceDiscovery/ServiceDiscovery+AsyncAwait.swift b/Sources/ServiceDiscovery/ServiceDiscovery+AsyncAwait.swift
@@ -73,7 +73,7 @@ public extension ServiceDiscovery {
                 }
             )
 
-            continuation.onTermination = { @Sendable (_) -> Void in
+            continuation.onTermination = { @Sendable (_) in
                 cancellationToken.cancel()
             }
         })

diff --git a/Sources/ServiceDiscovery/ServiceDiscovery+Combinators.swift b/Sources/ServiceDiscovery/ServiceDiscovery+Combinators.swift
@@ -27,8 +27,10 @@ public extension ServiceDiscovery {
     /// the derived function.
     ///
     /// It is not necessarily safe to block in this closure. This closure should not block for safety.
-    func mapService<ComputedService: Hashable>(serviceType: ComputedService.Type = ComputedService.self,
-                                               _ transformer: @escaping (ComputedService) throws -> Service) -> MapServiceServiceDiscovery<Self, ComputedService> {
+    func mapService<ComputedService: Hashable>(
+        serviceType: ComputedService.Type = ComputedService.self,
+        _ transformer: @escaping (ComputedService) throws -> Service
+    ) -> MapServiceServiceDiscovery<Self, ComputedService> {
         MapServiceServiceDiscovery(originalSD: self, transformer: transformer)
     }
 

diff --git a/Tests/ServiceDiscoveryTests/AsyncAwaitTests.swift b/Tests/ServiceDiscoveryTests/AsyncAwaitTests.swift
@@ -2,7 +2,7 @@
 //
 // This source file is part of the SwiftServiceDiscovery open source project
 //
-// Copyright (c) 2023 Apple Inc. and the SwiftServiceDiscovery project authors
+// Copyright (c) 2023-2024 Apple Inc. and the SwiftServiceDiscovery project authors
 // Licensed under Apache License v2.0
 //
 // See LICENSE.txt for license information
@@ -47,7 +47,7 @@ final class AsyncAwaitTests: XCTestCase {
     func testCancellationTokenIsInvoked() async throws {
         let discoveryService = MockServiceDiscovery()
 
-        await withThrowingTaskGroup(of: Void.self) { taskGroup in
+        try await withThrowingTaskGroup(of: Void.self) { taskGroup in
             let snapshots = discoveryService.subscribe(to: "foo")
 
             taskGroup.addTask {
@@ -59,6 +59,12 @@ final class AsyncAwaitTests: XCTestCase {
             XCTAssertEqual(discoveryService.cancelCounter.load(ordering: .relaxed), 0)
             taskGroup.cancelAll()
             _ = await taskGroup.nextResult()
+
+            // Wait for task group to be cancelled and cancelCounter is incremented
+            repeat {
+                try await Task.sleep(nanoseconds: 10_000_000)
+            } while !taskGroup.isCancelled
+
             XCTAssertEqual(discoveryService.cancelCounter.load(ordering: .relaxed), 1)
         }
     }

diff --git a/Tests/ServiceDiscoveryTests/FilterInstanceServiceDiscoveryTests.swift b/Tests/ServiceDiscoveryTests/FilterInstanceServiceDiscoveryTests.swift
@@ -2,7 +2,7 @@
 //
 // This source file is part of the SwiftServiceDiscovery open source project
 //
-// Copyright (c) 2020-2023 Apple Inc. and the SwiftServiceDiscovery project authors
+// Copyright (c) 2020-2024 Apple Inc. and the SwiftServiceDiscovery project authors
 // Licensed under Apache License v2.0
 //
 // See LICENSE.txt for license information
@@ -276,64 +276,45 @@ class FilterInstanceServiceDiscoveryTests: XCTestCase {
 
     // MARK: - async/await API tests
 
-    @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
-    func test_async_lookup() throws {
-        #if !(compiler(>=5.5) && canImport(_Concurrency))
-        try XCTSkipIf(true)
-        #else
+    func test_async_lookup() async throws {
         var configuration = InMemoryServiceDiscovery<Service, Instance>.Configuration(serviceInstances: [Self.fooService: Self.fooBaseInstances])
         configuration.register(service: Self.barService, instances: Self.barBaseInstances)
 
         let serviceDiscovery = InMemoryServiceDiscovery(configuration: configuration).filterInstance { [7001, 9001, 9002].contains($0.port) }
 
-        runAsyncAndWaitFor {
-            let _fooInstances = try await serviceDiscovery.lookup(Self.fooService)
-            XCTAssertEqual(_fooInstances.count, 1, "Expected service[\(Self.fooService)] to have 1 instance, got \(_fooInstances.count)")
-            XCTAssertEqual(_fooInstances, Self.fooDerivedInstances, "Expected service[\(Self.fooService)] to have instances \(Self.fooDerivedInstances), got \(_fooInstances)")
+        let _fooInstances = try await serviceDiscovery.lookup(Self.fooService)
+        XCTAssertEqual(_fooInstances.count, 1, "Expected service[\(Self.fooService)] to have 1 instance, got \(_fooInstances.count)")
+        XCTAssertEqual(_fooInstances, Self.fooDerivedInstances, "Expected service[\(Self.fooService)] to have instances \(Self.fooDerivedInstances), got \(_fooInstances)")
 
-            let _barInstances = try await serviceDiscovery.lookup(Self.barService)
-            XCTAssertEqual(_barInstances.count, 2, "Expected service[\(Self.barService)] to have 2 instances, got \(_barInstances.count)")
-            XCTAssertEqual(_barInstances, Self.barDerivedInstances, "Expected service[\(Self.barService)] to have instances \(Self.barDerivedInstances), got \(_barInstances)")
-        }
-        #endif
+        let _barInstances = try await serviceDiscovery.lookup(Self.barService)
+        XCTAssertEqual(_barInstances.count, 2, "Expected service[\(Self.barService)] to have 2 instances, got \(_barInstances.count)")
+        XCTAssertEqual(_barInstances, Self.barDerivedInstances, "Expected service[\(Self.barService)] to have instances \(Self.barDerivedInstances), got \(_barInstances)")
     }
 
-    @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
-    func test_async_lookup_errorIfServiceUnknown() throws {
-        #if !(compiler(>=5.5) && canImport(_Concurrency))
-        try XCTSkipIf(true)
-        #else
+    func test_async_lookup_errorIfServiceUnknown() async throws {
         let unknownService = "unknown-service"
 
         let configuration = InMemoryServiceDiscovery<Service, Instance>.Configuration(serviceInstances: ["foo-service": []])
         let serviceDiscovery = InMemoryServiceDiscovery(configuration: configuration).filterInstance { $0.port == 7001 }
 
-        runAsyncAndWaitFor {
-            do {
-                _ = try await serviceDiscovery.lookup(unknownService)
-                return XCTFail("Lookup instances for service[\(unknownService)] should return an error")
-            } catch {
-                guard let lookupError = error as? LookupError, lookupError == .unknownService else {
-                    return XCTFail("Expected LookupError.unknownService, got \(error)")
-                }
+        do {
+            _ = try await serviceDiscovery.lookup(unknownService)
+            return XCTFail("Lookup instances for service[\(unknownService)] should return an error")
+        } catch {
+            guard let lookupError = error as? LookupError, lookupError == .unknownService else {
+                return XCTFail("Expected LookupError.unknownService, got \(error)")
             }
         }
-        #endif
     }
 
-    @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
-    func test_async_subscribe() throws {
-        #if !(compiler(>=5.5) && canImport(_Concurrency))
-        try XCTSkipIf(true)
-        #else
+    func test_async_subscribe() async throws {
         let configuration = InMemoryServiceDiscovery<Service, Instance>.Configuration(serviceInstances: [Self.fooService: Self.fooBaseInstances])
         let baseServiceDiscovery = InMemoryServiceDiscovery(configuration: configuration)
         let serviceDiscovery = baseServiceDiscovery.filterInstance { [7001, 9001, 9002].contains($0.port) }
 
-        let semaphore = DispatchSemaphore(value: 0)
         let counter = ManagedAtomic<Int>(0)
 
-        Task.detached {
+        Task {
             // Allow time for subscription to start
             usleep(100_000)
             // Update #1
@@ -343,7 +324,7 @@ class FilterInstanceServiceDiscoveryTests: XCTestCase {
             baseServiceDiscovery.register(Self.barService, instances: Self.barBaseInstances)
         }
 
-        let task = Task.detached { () -> Void in
+        let task = Task<Void, Error> { () in
             do {
                 for try await instances in serviceDiscovery.subscribe(to: Self.barService) {
                     switch counter.wrappingIncrementThenLoad(ordering: .relaxed) {
@@ -363,18 +344,15 @@ class FilterInstanceServiceDiscoveryTests: XCTestCase {
                     guard let serviceDiscoveryError = error as? ServiceDiscoveryError, serviceDiscoveryError == .unavailable else {
                         return XCTFail("Expected ServiceDiscoveryError.unavailable, got \(error)")
                     }
-                    // Test is complete at this point
-                    semaphore.signal()
+                // Test is complete at this point
                 default:
                     XCTFail("Unexpected error \(error)")
                 }
             }
         }
 
-        _ = semaphore.wait(timeout: DispatchTime.now() + .seconds(1))
-        task.cancel()
+        _ = await task.result
 
         XCTAssertEqual(counter.load(ordering: .relaxed), 2, "Expected to receive instances 2 times, got \(counter.load(ordering: .relaxed)) times")
-        #endif
     }
 }
diff --git a/Tests/ServiceDiscoveryTests/Helpers.swift b/Tests/ServiceDiscoveryTests/Helpers.swift
@@ -2,7 +2,7 @@
 //
 // This source file is part of the SwiftServiceDiscovery open source project
 //
-// Copyright (c) 2021 Apple Inc. and the SwiftServiceDiscovery project authors
+// Copyright (c) 2021-2024 Apple Inc. and the SwiftServiceDiscovery project authors
 // Licensed under Apache License v2.0
 //
 // See LICENSE.txt for license information
@@ -58,18 +58,3 @@ func ensureResult<SD: ServiceDiscovery>(serviceDiscovery: SD, service: SD.Servic
 
     return _result
 }
-
-#if compiler(>=5.5) && canImport(_Concurrency)
-extension XCTestCase {
-    // TODO: remove once XCTest supports async functions
-    @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
-    func runAsyncAndWaitFor(_ closure: @escaping @Sendable () async throws -> Void, _ timeout: TimeInterval = 1.0) {
-        let finished = expectation(description: "finished")
-        Task.detached {
-            try await closure()
-            finished.fulfill()
-        }
-        wait(for: [finished], timeout: timeout)
-    }
-}
-#endif
diff --git a/Tests/ServiceDiscoveryTests/InMemoryServiceDiscoveryTests.swift b/Tests/ServiceDiscoveryTests/InMemoryServiceDiscoveryTests.swift
@@ -2,7 +2,7 @@
 //
 // This source file is part of the SwiftServiceDiscovery open source project
 //
-// Copyright (c) 2019-2023 Apple Inc. and the SwiftServiceDiscovery project authors
+// Copyright (c) 2019-2024 Apple Inc. and the SwiftServiceDiscovery project authors
 // Licensed under Apache License v2.0
 //
 // See LICENSE.txt for license information
@@ -245,63 +245,44 @@ class InMemoryServiceDiscoveryTests: XCTestCase {
 
     // MARK: - async/await API tests
 
-    @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
-    func test_async_lookup() throws {
-        #if !(compiler(>=5.5) && canImport(_Concurrency))
-        try XCTSkipIf(true)
-        #else
+    func test_async_lookup() async throws {
         var configuration = InMemoryServiceDiscovery<Service, Instance>.Configuration(serviceInstances: [Self.fooService: Self.fooInstances])
         configuration.register(service: Self.barService, instances: Self.barInstances)
 
         let serviceDiscovery = InMemoryServiceDiscovery(configuration: configuration)
 
-        runAsyncAndWaitFor {
-            let _fooInstances = try await serviceDiscovery.lookup(Self.fooService)
-            XCTAssertEqual(_fooInstances.count, 1, "Expected service[\(Self.fooService)] to have 1 instance, got \(_fooInstances.count)")
-            XCTAssertEqual(_fooInstances, Self.fooInstances, "Expected service[\(Self.fooService)] to have instances \(Self.fooInstances), got \(_fooInstances)")
+        let _fooInstances = try await serviceDiscovery.lookup(Self.fooService)
+        XCTAssertEqual(_fooInstances.count, 1, "Expected service[\(Self.fooService)] to have 1 instance, got \(_fooInstances.count)")
+        XCTAssertEqual(_fooInstances, Self.fooInstances, "Expected service[\(Self.fooService)] to have instances \(Self.fooInstances), got \(_fooInstances)")
 
-            let _barInstances = try await serviceDiscovery.lookup(Self.barService)
-            XCTAssertEqual(_barInstances.count, 2, "Expected service[\(Self.barService)] to have 2 instances, got \(_barInstances.count)")
-            XCTAssertEqual(_barInstances, Self.barInstances, "Expected service[\(Self.barService)] to have instances \(Self.barInstances), got \(_barInstances)")
-        }
-        #endif
+        let _barInstances = try await serviceDiscovery.lookup(Self.barService)
+        XCTAssertEqual(_barInstances.count, 2, "Expected service[\(Self.barService)] to have 2 instances, got \(_barInstances.count)")
+        XCTAssertEqual(_barInstances, Self.barInstances, "Expected service[\(Self.barService)] to have instances \(Self.barInstances), got \(_barInstances)")
     }
 
-    @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
-    func test_async_lookup_errorIfServiceUnknown() throws {
-        #if !(compiler(>=5.5) && canImport(_Concurrency))
-        try XCTSkipIf(true)
-        #else
+    func test_async_lookup_errorIfServiceUnknown() async throws {
         let unknownService = "unknown-service"
 
         let configuration = InMemoryServiceDiscovery<Service, Instance>.Configuration(serviceInstances: ["foo-service": []])
         let serviceDiscovery = InMemoryServiceDiscovery(configuration: configuration)
 
-        runAsyncAndWaitFor {
-            do {
-                _ = try await serviceDiscovery.lookup(unknownService)
-                return XCTFail("Lookup instances for service[\(unknownService)] should return an error")
-            } catch {
-                guard let lookupError = error as? LookupError, lookupError == .unknownService else {
-                    return XCTFail("Expected LookupError.unknownService, got \(error)")
-                }
+        do {
+            _ = try await serviceDiscovery.lookup(unknownService)
+            return XCTFail("Lookup instances for service[\(unknownService)] should return an error")
+        } catch {
+            guard let lookupError = error as? LookupError, lookupError == .unknownService else {
+                return XCTFail("Expected LookupError.unknownService, got \(error)")
             }
         }
-        #endif
     }
 
-    @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
-    func test_async_subscribe() throws {
-        #if !(compiler(>=5.5) && canImport(_Concurrency))
-        try XCTSkipIf(true)
-        #else
+    func test_async_subscribe() async throws {
         let configuration = InMemoryServiceDiscovery<Service, Instance>.Configuration(serviceInstances: [Self.fooService: Self.fooInstances])
         let serviceDiscovery = InMemoryServiceDiscovery(configuration: configuration)
 
-        let semaphore = DispatchSemaphore(value: 0)
         let counter = ManagedAtomic<Int>(0)
 
-        Task.detached {
+        Task {
             // Allow time for subscription to start
             usleep(100_000)
             // Update #1
@@ -311,7 +292,7 @@ class InMemoryServiceDiscoveryTests: XCTestCase {
             serviceDiscovery.register(Self.barService, instances: Self.barInstances)
         }
 
-        let task = Task.detached { () -> Void in
+        let task = Task<Void, Error> { () in
             do {
                 for try await instances in serviceDiscovery.subscribe(to: Self.barService) {
                     switch counter.wrappingIncrementThenLoad(ordering: .relaxed) {
@@ -331,18 +312,15 @@ class InMemoryServiceDiscoveryTests: XCTestCase {
                     guard let serviceDiscoveryError = error as? ServiceDiscoveryError, serviceDiscoveryError == .unavailable else {
                         return XCTFail("Expected ServiceDiscoveryError.unavailable, got \(error)")
                     }
-                    // Test is complete at this point
-                    semaphore.signal()
+                // Test is complete at this point
                 default:
                     XCTFail("Unexpected error \(error)")
                 }
             }
         }
 
-        _ = semaphore.wait(timeout: DispatchTime.now() + .seconds(1))
-        task.cancel()
+        _ = await task.result
 
         XCTAssertEqual(counter.load(ordering: .relaxed), 2, "Expected to receive instances 2 times, got \(counter.load(ordering: .relaxed)) times")
-        #endif
     }
 }