From 448264c31be38217e4582cca7f4a11b5408495f4 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 14 Aug 2023 14:56:38 +0100 Subject: [PATCH 1/4] Fix flaky test `TransactionTests.testCancelAsyncRequest` Resolves #706. The continuation is resumed with the cancelation error before we cancel the scheduled request. https://github.com/swift-server/async-http-client/blob/62c06d47c8d21c91335e9f8998589e4ce31411e6/Sources/AsyncHTTPClient/AsyncAwait/Transaction.swift#L290C10-L294 Therefore this test is inherently flaky. The fix waits up to one second for the scheduled request to be canceled. --- Tests/AsyncHTTPClientTests/RequestBagTests.swift | 8 ++++++-- Tests/AsyncHTTPClientTests/TransactionTests.swift | 10 +++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/RequestBagTests.swift b/Tests/AsyncHTTPClientTests/RequestBagTests.swift index 75d57ba26..c5d99f19f 100644 --- a/Tests/AsyncHTTPClientTests/RequestBagTests.swift +++ b/Tests/AsyncHTTPClientTests/RequestBagTests.swift @@ -961,10 +961,14 @@ class UploadCountingDelegate: HTTPClientResponseDelegate { } } -class MockTaskQueuer: HTTPRequestScheduler { +final class MockTaskQueuer: HTTPRequestScheduler { private(set) var hitCancelCount = 0 - init() {} + let onCancelRequest: (@Sendable (HTTPSchedulableRequest) -> Void)? + + init(onCancelRequest: (@Sendable (HTTPSchedulableRequest) -> Void)? = nil) { + self.onCancelRequest = onCancelRequest + } func cancelRequest(_: HTTPSchedulableRequest) { self.hitCancelCount += 1 diff --git a/Tests/AsyncHTTPClientTests/TransactionTests.swift b/Tests/AsyncHTTPClientTests/TransactionTests.swift index 19454681c..7409fdd9b 100644 --- a/Tests/AsyncHTTPClientTests/TransactionTests.swift +++ b/Tests/AsyncHTTPClientTests/TransactionTests.swift @@ -43,19 +43,23 @@ final class TransactionTests: XCTestCase { preferredEventLoop: embeddedEventLoop ) - let queuer = MockTaskQueuer() + let scheduledRequestCanceled = self.expectation(description: "scheduled request canceled") + let queuer = MockTaskQueuer { _ in + scheduledRequestCanceled.fulfill() + } transaction.requestWasQueued(queuer) + XCTAssertEqual(queuer.hitCancelCount, 0) Task.detached { try await Task.sleep(nanoseconds: 5 * 1000 * 1000) transaction.cancel() } - XCTAssertEqual(queuer.hitCancelCount, 0) await XCTAssertThrowsError(try await responseTask.value) { error in XCTAssertTrue(error is CancellationError, "unexpected error \(error)") } - XCTAssertEqual(queuer.hitCancelCount, 1) + + await self.fulfillment(of: [scheduledRequestCanceled]) } } From a6d892142fb52a2a4a51c5e5c3d97d832fd395eb Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 14 Aug 2023 18:06:06 +0100 Subject: [PATCH 2/4] self.fulfillment(of:) is not available on Linux --- Tests/AsyncHTTPClientTests/RequestBagTests.swift | 3 ++- Tests/AsyncHTTPClientTests/TransactionTests.swift | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/RequestBagTests.swift b/Tests/AsyncHTTPClientTests/RequestBagTests.swift index c5d99f19f..e2a959589 100644 --- a/Tests/AsyncHTTPClientTests/RequestBagTests.swift +++ b/Tests/AsyncHTTPClientTests/RequestBagTests.swift @@ -970,8 +970,9 @@ final class MockTaskQueuer: HTTPRequestScheduler { self.onCancelRequest = onCancelRequest } - func cancelRequest(_: HTTPSchedulableRequest) { + func cancelRequest(_ request: HTTPSchedulableRequest) { self.hitCancelCount += 1 + self.onCancelRequest?(request) } } diff --git a/Tests/AsyncHTTPClientTests/TransactionTests.swift b/Tests/AsyncHTTPClientTests/TransactionTests.swift index 7409fdd9b..9e639157f 100644 --- a/Tests/AsyncHTTPClientTests/TransactionTests.swift +++ b/Tests/AsyncHTTPClientTests/TransactionTests.swift @@ -58,8 +58,11 @@ final class TransactionTests: XCTestCase { await XCTAssertThrowsError(try await responseTask.value) { error in XCTAssertTrue(error is CancellationError, "unexpected error \(error)") } - - await self.fulfillment(of: [scheduledRequestCanceled]) + + // self.fulfillment(of:) is not available on Linux + _ = { + self.wait(for: [scheduledRequestCanceled], timeout: 1) + }() } } From 44c8bfb948253c58a3c1357952a48734931dee43 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 14 Aug 2023 20:17:47 +0100 Subject: [PATCH 3/4] Fix formatting --- Tests/AsyncHTTPClientTests/TransactionTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/AsyncHTTPClientTests/TransactionTests.swift b/Tests/AsyncHTTPClientTests/TransactionTests.swift index 9e639157f..af8d475dd 100644 --- a/Tests/AsyncHTTPClientTests/TransactionTests.swift +++ b/Tests/AsyncHTTPClientTests/TransactionTests.swift @@ -58,7 +58,7 @@ final class TransactionTests: XCTestCase { await XCTAssertThrowsError(try await responseTask.value) { error in XCTAssertTrue(error is CancellationError, "unexpected error \(error)") } - + // self.fulfillment(of:) is not available on Linux _ = { self.wait(for: [scheduledRequestCanceled], timeout: 1) From a99d4bfcd8a9203d5bfed83058bb60a8d688e826 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Tue, 15 Aug 2023 11:29:43 +0100 Subject: [PATCH 4/4] Fix 5.6 --- Tests/AsyncHTTPClientTests/TransactionTests.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tests/AsyncHTTPClientTests/TransactionTests.swift b/Tests/AsyncHTTPClientTests/TransactionTests.swift index af8d475dd..13cb87eb5 100644 --- a/Tests/AsyncHTTPClientTests/TransactionTests.swift +++ b/Tests/AsyncHTTPClientTests/TransactionTests.swift @@ -27,6 +27,9 @@ typealias PreparedRequest = HTTPClientRequest.Prepared final class TransactionTests: XCTestCase { func testCancelAsyncRequest() { guard #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) else { return } + // creating the `XCTestExpectation` off the main thread crashes on Linux with Swift 5.6 + // therefore we create it here as a workaround which works fine + let scheduledRequestCanceled = self.expectation(description: "scheduled request canceled") XCTAsyncTest { let embeddedEventLoop = EmbeddedEventLoop() defer { XCTAssertNoThrow(try embeddedEventLoop.syncShutdownGracefully()) } @@ -43,7 +46,6 @@ final class TransactionTests: XCTestCase { preferredEventLoop: embeddedEventLoop ) - let scheduledRequestCanceled = self.expectation(description: "scheduled request canceled") let queuer = MockTaskQueuer { _ in scheduledRequestCanceled.fulfill() }