From 05b9d9b123afeacd1ae9e296410141f75c3069a4 Mon Sep 17 00:00:00 2001
From: Fabian Fett <fabianfett@apple.com>
Date: Sat, 4 Dec 2021 08:42:47 +0100
Subject: [PATCH] Make tests faster, by reducing timeout margins

---
 .../HTTPClient+SOCKSTests.swift               |  47 +++++--
 .../HTTPClientTests.swift                     | 124 +++++++++---------
 .../HTTPConnectionPool+FactoryTests.swift     |   5 +-
 3 files changed, 104 insertions(+), 72 deletions(-)

diff --git a/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift
index 5fdc5ac61..3a7c2a9f5 100644
--- a/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPClient+SOCKSTests.swift
@@ -75,8 +75,13 @@ class HTTPClientSOCKSTests: XCTestCase {
 
     func testProxySOCKS() throws {
         let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!")
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
-                                     configuration: .init(proxy: .socksServer(host: "localhost", port: socksBin.port)))
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(
+                timeout: .init(connect: .seconds(2), read: nil),
+                proxy: .socksServer(host: "localhost", port: socksBin.port)
+            )
+        )
 
         defer {
             XCTAssertNoThrow(try localClient.syncShutdown())
@@ -90,8 +95,13 @@ class HTTPClientSOCKSTests: XCTestCase {
     }
 
     func testProxySOCKSBogusAddress() throws {
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
-                                     configuration: .init(proxy: .socksServer(host: "127.0..")))
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(
+                timeout: .init(connect: .seconds(2), read: nil),
+                proxy: .socksServer(host: "127.0..")
+            )
+        )
 
         defer {
             XCTAssertNoThrow(try localClient.syncShutdown())
@@ -102,8 +112,14 @@ class HTTPClientSOCKSTests: XCTestCase {
     // there is no socks server, so we should fail
     func testProxySOCKSFailureNoServer() throws {
         let localHTTPBin = HTTPBin()
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
-                                     configuration: .init(proxy: .socksServer(host: "localhost", port: localHTTPBin.port)))
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(
+                timeout: .init(connect: .seconds(2), read: nil),
+                proxy: .socksServer(host: "localhost", port: localHTTPBin.port)
+            )
+        )
+
         defer {
             XCTAssertNoThrow(try localClient.syncShutdown())
             XCTAssertNoThrow(try localHTTPBin.shutdown())
@@ -113,8 +129,14 @@ class HTTPClientSOCKSTests: XCTestCase {
 
     // speak to a server that doesn't speak SOCKS
     func testProxySOCKSFailureInvalidServer() throws {
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
-                                     configuration: .init(proxy: .socksServer(host: "localhost")))
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(
+                timeout: .init(connect: .seconds(2), read: nil),
+                proxy: .socksServer(host: "localhost")
+            )
+        )
+
         defer {
             XCTAssertNoThrow(try localClient.syncShutdown())
         }
@@ -124,8 +146,13 @@ class HTTPClientSOCKSTests: XCTestCase {
     // test a handshake failure with a misbehaving server
     func testProxySOCKSMisbehavingServer() throws {
         let socksBin = try MockSOCKSServer(expectedURL: "/socks/test", expectedResponse: "it works!", misbehave: true)
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
-                                     configuration: .init(proxy: .socksServer(host: "localhost", port: socksBin.port)))
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(
+                timeout: .init(connect: .seconds(2), read: nil),
+                proxy: .socksServer(host: "localhost", port: socksBin.port)
+            )
+        )
 
         defer {
             XCTAssertNoThrow(try localClient.syncShutdown())
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift
index e7ba9d510..a9cb0522c 100644
--- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift
@@ -646,10 +646,8 @@ class HTTPClientTests: XCTestCase {
             task.cancel()
         }
 
-        XCTAssertThrowsError(try task.wait(), "Should fail") { error in
-            guard case let error = error as? HTTPClientError, error == .cancelled else {
-                return XCTFail("Should fail with cancelled")
-            }
+        XCTAssertThrowsError(try task.wait()) {
+            XCTAssertEqual($0 as? HTTPClientError, .cancelled)
         }
     }
 
@@ -730,19 +728,23 @@ class HTTPClientTests: XCTestCase {
 
     func testProxyPlaintextWithIncorrectlyAuthorization() throws {
         let localHTTPBin = HTTPBin(proxy: .simulate(authorization: "Basic YWxhZGRpbjpvcGVuc2VzYW1l"))
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
-                                     configuration: .init(proxy: .server(host: "localhost",
-                                                                         port: localHTTPBin.port,
-                                                                         authorization: .basic(username: "aladdin",
-                                                                                               password: "opensesamefoo"))))
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(
+                timeout: .init(connect: .seconds(2), read: nil),
+                proxy: .server(
+                    host: "localhost",
+                    port: localHTTPBin.port,
+                    authorization: .basic(username: "aladdin", password: "opensesamefoo")
+                )
+            )
+        )
         defer {
             XCTAssertNoThrow(try localClient.syncShutdown())
             XCTAssertNoThrow(try localHTTPBin.shutdown())
         }
-        XCTAssertThrowsError(try localClient.get(url: "http://test/ok").wait(), "Should fail") { error in
-            guard case let error = error as? HTTPClientError, error == .proxyAuthenticationRequired else {
-                return XCTFail("Should fail with HTTPClientError.proxyAuthenticationRequired")
-            }
+        XCTAssertThrowsError(try localClient.get(url: "http://test/ok").wait()) {
+            XCTAssertEqual($0 as? HTTPClientError, .proxyAuthenticationRequired)
         }
     }
 
@@ -859,31 +861,41 @@ class HTTPClientTests: XCTestCase {
 
     func testLoopDetectionRedirectLimit() throws {
         let localHTTPBin = HTTPBin(.http1_1(ssl: true))
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
-                                     configuration: HTTPClient.Configuration(certificateVerification: .none, redirectConfiguration: .follow(max: 5, allowCycles: false)))
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(
+                certificateVerification: .none,
+                redirectConfiguration: .follow(max: 5, allowCycles: false)
+            )
+        )
 
         defer {
             XCTAssertNoThrow(try localClient.syncShutdown())
             XCTAssertNoThrow(try localHTTPBin.shutdown())
         }
 
-        XCTAssertThrowsError(try localClient.get(url: "https://localhost:\(localHTTPBin.port)/redirect/infinite1").wait(), "Should fail with redirect limit") { error in
-            XCTAssertEqual(error as? HTTPClientError, HTTPClientError.redirectCycleDetected)
+        XCTAssertThrowsError(try localClient.get(url: "https://localhost:\(localHTTPBin.port)/redirect/infinite1").wait()) {
+            XCTAssertEqual($0 as? HTTPClientError, HTTPClientError.redirectCycleDetected)
         }
     }
 
     func testCountRedirectLimit() throws {
         let localHTTPBin = HTTPBin(.http1_1(ssl: true))
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
-                                     configuration: HTTPClient.Configuration(certificateVerification: .none, redirectConfiguration: .follow(max: 10, allowCycles: true)))
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(
+                certificateVerification: .none,
+                redirectConfiguration: .follow(max: 10, allowCycles: true)
+            )
+        )
 
         defer {
             XCTAssertNoThrow(try localClient.syncShutdown())
             XCTAssertNoThrow(try localHTTPBin.shutdown())
         }
 
-        XCTAssertThrowsError(try localClient.get(url: "https://localhost:\(localHTTPBin.port)/redirect/infinite1").wait(), "Should fail with redirect limit") { error in
-            XCTAssertEqual(error as? HTTPClientError, HTTPClientError.redirectLimitReached)
+        XCTAssertThrowsError(try localClient.get(url: "https://localhost:\(localHTTPBin.port)/redirect/infinite1").wait()) {
+            XCTAssertEqual($0 as? HTTPClientError, HTTPClientError.redirectLimitReached)
         }
     }
 
@@ -1105,9 +1117,15 @@ class HTTPClientTests: XCTestCase {
     }
 
     func testStressGetHttpsSSLError() throws {
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .createNew,
+            configuration: .init(timeout: .init(connect: .seconds(2), read: nil))
+        )
+        defer { XCTAssertNoThrow(try localClient.syncShutdown()) }
+
         let request = try Request(url: "https://localhost:\(self.defaultHTTPBin.port)/wait", method: .GET)
         let tasks = (1...100).map { _ -> HTTPClient.Task<TestHTTPDelegate.Response> in
-            self.defaultClient.execute(request: request, delegate: TestHTTPDelegate())
+            localClient.execute(request: request, delegate: TestHTTPDelegate())
         }
 
         let results = try EventLoopFuture<TestHTTPDelegate.Response>.whenAllComplete(tasks.map { $0.futureResult }, on: self.defaultClient.eventLoopGroup.next()).wait()
@@ -1148,14 +1166,10 @@ class HTTPClientTests: XCTestCase {
             XCTAssertNoThrow(try localClient.syncShutdown())
             XCTAssertNoThrow(try localHTTPBin.shutdown())
         }
-        do {
-            _ = try localClient.get(url: "http://localhost:\(localHTTPBin.port)/get").timeout(after: .seconds(5)).wait()
-            XCTFail("Shouldn't succeed")
-        } catch {
-            guard !(error is EventLoopFutureTimeoutError) else {
-                XCTFail("Timed out but should have failed immediately")
-                return
-            }
+
+        let future = localClient.get(url: "http://localhost:\(localHTTPBin.port)/get").timeout(after: .seconds(5))
+        XCTAssertThrowsError(try future.wait()) {
+            XCTAssertFalse($0 is EventLoopFutureTimeoutError, "Timed out but should have failed immediately")
         }
     }
 
@@ -1296,11 +1310,7 @@ class HTTPClientTests: XCTestCase {
             case .success:
                 XCTFail("Shouldn't succeed")
             case .failure(let error):
-                if let clientError = error as? HTTPClientError, clientError == .cancelled {
-                    continue
-                } else {
-                    XCTFail("Unexpected error: \(error)")
-                }
+                XCTAssertEqual(error as? HTTPClientError, .cancelled)
             }
         }
     }
@@ -1308,29 +1318,18 @@ class HTTPClientTests: XCTestCase {
     func testDoubleShutdown() {
         let client = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup))
         XCTAssertNoThrow(try client.syncShutdown())
-        do {
-            try client.syncShutdown()
-            XCTFail("Shutdown should fail with \(HTTPClientError.alreadyShutdown)")
-        } catch {
-            guard let clientError = error as? HTTPClientError, clientError == .alreadyShutdown else {
-                XCTFail("Unexpected error: \(error) instead of \(HTTPClientError.alreadyShutdown)")
-                return
-            }
+
+        XCTAssertThrowsError(try client.syncShutdown()) {
+            XCTAssertEqual($0 as? HTTPClientError, .alreadyShutdown)
         }
     }
 
     func testTaskFailsWhenClientIsShutdown() {
         let client = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup))
         XCTAssertNoThrow(try client.syncShutdown())
-        do {
-            _ = try client.get(url: "http://localhost/").wait()
-            XCTFail("Request shouldn't succeed")
-        } catch {
-            if let error = error as? HTTPClientError, error == .alreadyShutdown {
-                return
-            } else {
-                XCTFail("Unexpected error: \(error)")
-            }
+
+        XCTAssertThrowsError(try client.get(url: "http://localhost/").wait()) {
+            XCTAssertEqual($0 as? HTTPClientError, .alreadyShutdown)
         }
     }
 
@@ -1712,11 +1711,12 @@ class HTTPClientTests: XCTestCase {
     }
 
     func testRacePoolIdleConnectionsAndGet() {
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
-                                     configuration: .init(connectionPool: .init(idleTimeout: .milliseconds(10))))
-        defer {
-            XCTAssertNoThrow(try localClient.syncShutdown())
-        }
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(connectionPool: .init(idleTimeout: .milliseconds(10)))
+        )
+
+        defer { XCTAssertNoThrow(try localClient.syncShutdown()) }
         for _ in 1...500 {
             XCTAssertNoThrow(try localClient.get(url: self.defaultHTTPBinURLPrefix + "get").wait())
             Thread.sleep(forTimeInterval: 0.01 + .random(in: -0.05...0.05))
@@ -1724,13 +1724,15 @@ class HTTPClientTests: XCTestCase {
     }
 
     func testAvoidLeakingTLSHandshakeCompletionPromise() {
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), configuration: .init(timeout: .init(connect: .milliseconds(100))))
         let localHTTPBin = HTTPBin()
         let port = localHTTPBin.port
         XCTAssertNoThrow(try localHTTPBin.shutdown())
-        defer {
-            XCTAssertNoThrow(try localClient.syncShutdown())
-        }
+
+        let localClient = HTTPClient(
+            eventLoopGroupProvider: .shared(self.clientGroup),
+            configuration: .init(timeout: .init(connect: .milliseconds(100)))
+        )
+        defer { XCTAssertNoThrow(try localClient.syncShutdown()) }
 
         XCTAssertThrowsError(try localClient.get(url: "http://localhost:\(port)").wait()) { error in
             if isTestingNIOTS() {
diff --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+FactoryTests.swift b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+FactoryTests.swift
index b13ff3d18..4e986e039 100644
--- a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+FactoryTests.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+FactoryTests.swift
@@ -76,7 +76,10 @@ class HTTPConnectionPool_FactoryTests: XCTestCase {
         let factory = HTTPConnectionPool.ConnectionFactory(
             key: .init(request),
             tlsConfiguration: nil,
-            clientConfiguration: .init(proxy: .socksServer(host: "127.0.0.1", port: server!.localAddress!.port!)),
+            clientConfiguration: .init(
+                timeout: .init(connect: .seconds(2), read: nil),
+                proxy: .socksServer(host: "127.0.0.1", port: server!.localAddress!.port!)
+            ),
             sslContextCache: .init()
         )