From 0032344e7a1cc02c5a6413415f7eedc4723d8b74 Mon Sep 17 00:00:00 2001 From: Ilya Teterin Date: Fri, 25 Sep 2020 17:43:59 +0100 Subject: [PATCH 1/4] Introduce helper methods for test of ConnectionsState Motivation: Issue #234 highlights that we directly manipulate ConnectionsState and this commit prepares tests to be refactored to manipulate the state by exposed APIs instead. Modifications: * introduce helper methods in ConnectionPoolTestsSupport.swift Result: * no observable changes --- .../ConnectionPoolTests.swift | 2 - .../ConnectionPoolTestsSupport.swift | 119 ++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 Tests/AsyncHTTPClientTests/ConnectionPoolTestsSupport.swift diff --git a/Tests/AsyncHTTPClientTests/ConnectionPoolTests.swift b/Tests/AsyncHTTPClientTests/ConnectionPoolTests.swift index aee407b28..e46e9d841 100644 --- a/Tests/AsyncHTTPClientTests/ConnectionPoolTests.swift +++ b/Tests/AsyncHTTPClientTests/ConnectionPoolTests.swift @@ -27,8 +27,6 @@ class ConnectionPoolTests: XCTestCase { var eventLoop: EmbeddedEventLoop! var http1ConnectionProvider: HTTP1ConnectionProvider! - struct TempError: Error {} - func testPending() { var state = HTTP1ConnectionProvider.ConnectionsState(eventLoop: self.eventLoop) diff --git a/Tests/AsyncHTTPClientTests/ConnectionPoolTestsSupport.swift b/Tests/AsyncHTTPClientTests/ConnectionPoolTestsSupport.swift new file mode 100644 index 000000000..dc5281e60 --- /dev/null +++ b/Tests/AsyncHTTPClientTests/ConnectionPoolTestsSupport.swift @@ -0,0 +1,119 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2018-2019 Apple Inc. and the AsyncHTTPClient project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +@testable import AsyncHTTPClient +import NIO +import XCTest + +extension ConnectionPoolTests { + func buildState(count: Int, release: Bool = true, eventLoop: EventLoop? = nil) -> (HTTP1ConnectionProvider.ConnectionsState, [Connection]) { + let eventLoop = eventLoop ?? self.eventLoop! + + var state = HTTP1ConnectionProvider.ConnectionsState(eventLoop: eventLoop) + var items: [Connection] = [] + + if count == 0 { + return (state, items) + } + + let channel = ActiveChannel(eventLoop: self.eventLoop) + + for _ in 1...count { + // Set up connection pool to have one available connection + do { + let connection = Connection(channel: channel, provider: self.http1ConnectionProvider) + items.append(connection) + // First, we ask the empty pool for a connection, triggering connection creation + XCTAssertTrue(state.enqueue()) + let action = state.acquire(waiter: .init(promise: eventLoop.makePromise(), setupComplete: eventLoop.makeSucceededFuture(()), preference: .indifferent)) + + switch action { + case .create(let waiter): + waiter.promise.succeed(connection) + default: + XCTFail("Unexpected action: \(action)") + } + + // We offer the connection to the pool so that it can be tracked + _ = state.offer(connection: connection) + } + } + + if release { + for item in items { + // No we release the connection, making it available for the next caller + _ = state.release(connection: item, closing: false) + } + } + return (state, items) + } +} + +func XCTAssertState(_ state: HTTP1ConnectionProvider.ConnectionsState, available: Int, leased: Int, waiters: Int, pending: Int, opened: Int) { + let snapshot = state.testsOnly_getInternalState() + XCTAssertEqual(available, snapshot.availableConnections.count) + XCTAssertEqual(leased, snapshot.leasedConnections.count) + XCTAssertEqual(waiters, snapshot.waiters.count) + XCTAssertEqual(pending, snapshot.pending) + XCTAssertEqual(opened, snapshot.openedConnectionsCount) +} + +func XCTAssertState(_ state: HTTP1ConnectionProvider.ConnectionsState, available: Int, leased: Int, waiters: Int, pending: Int, opened: Int, isLeased connection: Connection) { + let snapshot = state.testsOnly_getInternalState() + XCTAssertEqual(available, snapshot.availableConnections.count) + XCTAssertEqual(leased, snapshot.leasedConnections.count) + XCTAssertEqual(waiters, snapshot.waiters.count) + XCTAssertEqual(pending, snapshot.pending) + XCTAssertEqual(opened, snapshot.openedConnectionsCount) + XCTAssertTrue(snapshot.leasedConnections.contains(ConnectionKey(connection))) +} + +func XCTAssertState(_ state: HTTP1ConnectionProvider.ConnectionsState, available: Int, leased: Int, waiters: Int, pending: Int, opened: Int, isNotLeased connection: Connection) { + let snapshot = state.testsOnly_getInternalState() + XCTAssertEqual(available, snapshot.availableConnections.count) + XCTAssertEqual(leased, snapshot.leasedConnections.count) + XCTAssertEqual(waiters, snapshot.waiters.count) + XCTAssertEqual(pending, snapshot.pending) + XCTAssertEqual(opened, snapshot.openedConnectionsCount) + XCTAssertFalse(snapshot.leasedConnections.contains(ConnectionKey(connection))) +} + +struct XCTEmptyError: Error {} + +func XCTUnwrap(_ value: T?) throws -> T { + if let unwrapped = value { + return unwrapped + } + throw XCTEmptyError() +} + +struct TempError: Error {} + +func XCTAssertStateClose(_ state: HTTP1ConnectionProvider.ConnectionsState, available: Int, leased: Int, waiters: Int, clean: Bool) throws { + var state = state + + let (foundWaiters, foundAvailable, foundLeased, foundClean) = try XCTUnwrap(state.close()) + XCTAssertEqual(waiters, foundWaiters.count) + XCTAssertEqual(available, foundAvailable.count) + XCTAssertEqual(leased, foundLeased.count) + XCTAssertEqual(clean, foundClean) + + for waiter in foundWaiters { + waiter.promise.fail(TempError()) + } + + for lease in foundLeased { + try lease.cancel().wait() + } +} From e04891716d733d31e12d7b2f33aee85ca80f7535 Mon Sep 17 00:00:00 2001 From: Ilya Teterin Date: Fri, 25 Sep 2020 18:04:11 +0100 Subject: [PATCH 2/4] Move Connection tests out of ConnectionsState tests into separate file. Motivation: Clean up of code to address issue #234 - here we move away connection tests to separate files outside of ConnectionsState tests so we will be able to work on the ConnectionsState in focussed mode. Modifications: Connection tests moved to separate files. Result: No observable changes. --- .../ConnectionPoolTests+XCTest.swift | 5 - .../ConnectionPoolTests.swift | 246 ------------------ .../ConnectionTests+XCTest.swift | 35 +++ .../ConnectionTests.swift | 229 ++++++++++++++++ Tests/LinuxMain.swift | 1 + 5 files changed, 265 insertions(+), 251 deletions(-) create mode 100644 Tests/AsyncHTTPClientTests/ConnectionTests+XCTest.swift create mode 100644 Tests/AsyncHTTPClientTests/ConnectionTests.swift diff --git a/Tests/AsyncHTTPClientTests/ConnectionPoolTests+XCTest.swift b/Tests/AsyncHTTPClientTests/ConnectionPoolTests+XCTest.swift index 04cfa6421..0fea77411 100644 --- a/Tests/AsyncHTTPClientTests/ConnectionPoolTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/ConnectionPoolTests+XCTest.swift @@ -57,11 +57,6 @@ extension ConnectionPoolTests { ("testTimeoutAvailableConnection", testTimeoutAvailableConnection), ("testRemoteClosedLeasedConnection", testRemoteClosedLeasedConnection), ("testRemoteClosedAvailableConnection", testRemoteClosedAvailableConnection), - ("testConnectionReleaseActive", testConnectionReleaseActive), - ("testConnectionReleaseInactive", testConnectionReleaseInactive), - ("testConnectionRemoteCloseRelease", testConnectionRemoteCloseRelease), - ("testConnectionTimeoutRelease", testConnectionTimeoutRelease), - ("testAcquireAvailableBecomesUnavailable", testAcquireAvailableBecomesUnavailable), ("testShutdownOnPendingAndSuccess", testShutdownOnPendingAndSuccess), ("testShutdownOnPendingAndError", testShutdownOnPendingAndError), ("testShutdownTimeout", testShutdownTimeout), diff --git a/Tests/AsyncHTTPClientTests/ConnectionPoolTests.swift b/Tests/AsyncHTTPClientTests/ConnectionPoolTests.swift index e46e9d841..a42d6198e 100644 --- a/Tests/AsyncHTTPClientTests/ConnectionPoolTests.swift +++ b/Tests/AsyncHTTPClientTests/ConnectionPoolTests.swift @@ -1219,176 +1219,6 @@ class ConnectionPoolTests: XCTestCase { self.http1ConnectionProvider.execute(action, logger: HTTPClient.loggingDisabled) } - // MARK: - Connection Tests - - func testConnectionReleaseActive() throws { - let channel = ActiveChannel(eventLoop: self.eventLoop) - var snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - - let connection = Connection(channel: channel, provider: self.http1ConnectionProvider) - snapshot.openedConnectionsCount = 1 - snapshot.leasedConnections.insert(ConnectionKey(connection)) - - self.http1ConnectionProvider.state.testsOnly_setInternalState(snapshot) - - XCTAssertEqual(0, snapshot.availableConnections.count) - XCTAssertEqual(1, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(1, snapshot.openedConnectionsCount) - - connection.release(closing: false, logger: HTTPClient.loggingDisabled) - - // XCTAssertFalse(connection.isInUse) - snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - XCTAssertEqual(1, snapshot.availableConnections.count) - XCTAssertEqual(0, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(1, snapshot.openedConnectionsCount) - - // cleanup - // this cleanup code needs to go and use HTTP1ConnectionProvider's API instead - // (https://github.com/swift-server/async-http-client/issues/234) - connection.remoteClosed(logger: HTTPClient.loggingDisabled) - } - - func testConnectionReleaseInactive() throws { - let channel = EmbeddedChannel() - - var snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - - let connection = Connection(channel: channel, provider: self.http1ConnectionProvider) - snapshot.openedConnectionsCount = 1 - snapshot.leasedConnections.insert(ConnectionKey(connection)) - - self.http1ConnectionProvider.state.testsOnly_setInternalState(snapshot) - - XCTAssertEqual(0, snapshot.availableConnections.count) - XCTAssertEqual(1, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(1, snapshot.openedConnectionsCount) - - connection.release(closing: true, logger: HTTPClient.loggingDisabled) - - snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - XCTAssertEqual(0, snapshot.availableConnections.count) - XCTAssertEqual(0, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(0, snapshot.openedConnectionsCount) - } - - func testConnectionRemoteCloseRelease() throws { - let channel = EmbeddedChannel() - - var snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - - let connection = Connection(channel: channel, provider: self.http1ConnectionProvider) - snapshot.availableConnections.append(connection) - snapshot.openedConnectionsCount = 1 - - self.http1ConnectionProvider.state.testsOnly_setInternalState(snapshot) - - XCTAssertEqual(1, snapshot.availableConnections.count) - XCTAssertEqual(0, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(1, snapshot.openedConnectionsCount) - - connection.remoteClosed(logger: HTTPClient.loggingDisabled) - - snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - XCTAssertEqual(0, snapshot.availableConnections.count) - XCTAssertEqual(0, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(0, snapshot.openedConnectionsCount) - } - - func testConnectionTimeoutRelease() throws { - let channel = EmbeddedChannel() - - var snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - - let connection = Connection(channel: channel, provider: self.http1ConnectionProvider) - snapshot.availableConnections.append(connection) - snapshot.openedConnectionsCount = 1 - - self.http1ConnectionProvider.state.testsOnly_setInternalState(snapshot) - - XCTAssertEqual(1, snapshot.availableConnections.count) - XCTAssertEqual(0, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(1, snapshot.openedConnectionsCount) - - connection.timeout(logger: HTTPClient.loggingDisabled) - - snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - XCTAssertEqual(0, snapshot.availableConnections.count) - XCTAssertEqual(0, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(0, snapshot.openedConnectionsCount) - } - - func testAcquireAvailableBecomesUnavailable() throws { - let channel = ActiveChannel(eventLoop: self.eventLoop) - var snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - - let connection = Connection(channel: channel, provider: self.http1ConnectionProvider) - snapshot.availableConnections.append(connection) - snapshot.openedConnectionsCount = 1 - - self.http1ConnectionProvider.state.testsOnly_setInternalState(snapshot) - - XCTAssertEqual(1, snapshot.availableConnections.count) - XCTAssertEqual(0, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(1, snapshot.openedConnectionsCount) - - XCTAssertTrue(self.http1ConnectionProvider.enqueue()) - - let action = self.http1ConnectionProvider.state.acquire(waiter: .init(promise: self.eventLoop.makePromise(), setupComplete: self.eventLoop.makeSucceededFuture(()), preference: .indifferent)) - switch action { - case .lease(let connection, let waiter): - // Since this connection is already in use, this should be a no-op and state should not have changed from normal lease - connection.timeout(logger: HTTPClient.loggingDisabled) - - snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - XCTAssertTrue(connection.isActiveEstimation) - XCTAssertTrue(snapshot.leasedConnections.contains(ConnectionKey(connection))) - XCTAssertEqual(0, snapshot.availableConnections.count) - XCTAssertEqual(1, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(1, snapshot.openedConnectionsCount) - - // This is unrecoverable, but in this case we create a new connection, so state again should not change, even though release will be called - // This is important to preventself.http1ConnectionProvider deletion since connection is released and there could be 0 waiters - connection.remoteClosed(logger: HTTPClient.loggingDisabled) - - snapshot = self.http1ConnectionProvider.state.testsOnly_getInternalState() - XCTAssertTrue(snapshot.leasedConnections.contains(ConnectionKey(connection))) - XCTAssertEqual(0, snapshot.availableConnections.count) - XCTAssertEqual(1, snapshot.leasedConnections.count) - XCTAssertEqual(0, snapshot.waiters.count) - XCTAssertEqual(0, snapshot.pending) - XCTAssertEqual(1, snapshot.openedConnectionsCount) - - // cleanup - // this cleanup code needs to go and use HTTP1ConnectionProvider's API instead - // (https://github.com/swift-server/async-http-client/issues/234) - waiter.promise.fail(TempError()) - self.http1ConnectionProvider.release(connection: connection, closing: true, logger: HTTPClient.loggingDisabled) - default: - XCTFail("Unexpected action: \(action)") - } - } - // MARK: - Shutdown tests func testShutdownOnPendingAndSuccess() { @@ -1555,79 +1385,3 @@ class ConnectionPoolTests: XCTestCase { self.http1ConnectionProvider = nil } } - -class ActiveChannel: Channel, ChannelCore { - struct NotImplementedError: Error {} - - func localAddress0() throws -> SocketAddress { - throw NotImplementedError() - } - - func remoteAddress0() throws -> SocketAddress { - throw NotImplementedError() - } - - func register0(promise: EventLoopPromise?) { - promise?.fail(NotImplementedError()) - } - - func bind0(to: SocketAddress, promise: EventLoopPromise?) { - promise?.fail(NotImplementedError()) - } - - func connect0(to: SocketAddress, promise: EventLoopPromise?) { - promise?.fail(NotImplementedError()) - } - - func write0(_ data: NIOAny, promise: EventLoopPromise?) { - promise?.fail(NotImplementedError()) - } - - func flush0() {} - - func read0() {} - - func close0(error: Error, mode: CloseMode, promise: EventLoopPromise?) { - promise?.succeed(()) - } - - func triggerUserOutboundEvent0(_ event: Any, promise: EventLoopPromise?) { - promise?.fail(NotImplementedError()) - } - - func channelRead0(_: NIOAny) {} - - func errorCaught0(error: Error) {} - - var allocator: ByteBufferAllocator - var closeFuture: EventLoopFuture - var eventLoop: EventLoop - - var localAddress: SocketAddress? - var remoteAddress: SocketAddress? - var parent: Channel? - var isWritable: Bool = true - var isActive: Bool = true - - init(eventLoop: EmbeddedEventLoop) { - self.allocator = ByteBufferAllocator() - self.eventLoop = eventLoop - self.closeFuture = self.eventLoop.makeSucceededFuture(()) - } - - var _channelCore: ChannelCore { - return self - } - - var pipeline: ChannelPipeline { - return ChannelPipeline(channel: self) - } - - func setOption