Conversation
Tests/NIOHTTPServerTests/Utilities/NIOClient/NIOClient+HTTP1.swift
Outdated
Show resolved
Hide resolved
| /// With the ``NIOAsyncChannel`` the `setUpClient` methods vend, one can write `HTTPRequestPart`s to the channel | ||
| /// and observe `HTTPResponsePart`s from the inbound stream of the async channel. | ||
| @available(macOS 26.0, iOS 26.0, watchOS 26.0, tvOS 26.0, visionOS 26.0, *) | ||
| struct NIOHTTP1Client { |
There was a problem hiding this comment.
I find it a bit weird that we have only static methods on this type.
I think we should either change it to an actual wrapper of a channel (and initialise it accordingly in the init), or perhaps extend Channel and ClientBootstrap instead with these methods (and rename them to something like setUpTestHTTP1ClientHandlers() and setUpTestHTTP1Client(at:)).
There was a problem hiding this comment.
I've managed to structure these methods under Channel and ClientBootstrap now.
The end result is that NIOHTTP1Client (and NIOSecureUpgradeClient) are no longer required.
From tests, we can now simply just do this:
let client = try await ClientBootstrap(group: .singletonMultiThreadedEventLoopGroup)
.connectToTestHTTP1Server(at: serverAddress)
Tests/NIOHTTPServerTests/Utilities/NIOClient/NIOClient+HTTP1.swift
Outdated
Show resolved
Hide resolved
|
|
||
| @available(macOS 26.0, iOS 26.0, watchOS 26.0, tvOS 26.0, visionOS 26.0, *) | ||
| /// Provides a HTTP client with ALPN negotiation. | ||
| struct NIOSecureUpgradeClient { |
There was a problem hiding this comment.
Similar comments to the H1 client.
| extension NIOAsyncTestingChannel { | ||
| /// Forwards all of our outbound writes to `other` and vice-versa. | ||
| func glueTo(_ other: NIOAsyncTestingChannel) async throws { | ||
| await withThrowingTaskGroup { group in |
There was a problem hiding this comment.
This should be withThrowingDiscardingTaskGroup
| @available(macOS 26.0, iOS 26.0, watchOS 26.0, tvOS 26.0, visionOS 26.0, *) | ||
| extension TestingChannelHTTP1Server { | ||
| /// A plaintext HTTP/1.1 client backed by a `NIOAsyncTestingChannel`. | ||
| struct Client { |
There was a problem hiding this comment.
I think it's a bit weird we have a client nested inside a server. Can't this live in the NIOHTTP1Client?
There was a problem hiding this comment.
Following from #56 (comment), I've removed the dedicated Client type here.
I've added a simpler withConnectedClient method in TestingChannelHTTP1Server that sets up a NIOAsyncTestingChannel and calls the new configureTestHTTP1ClientPipeline() method (under Channel) on it.
| @available(macOS 26.0, iOS 26.0, watchOS 26.0, tvOS 26.0, visionOS 26.0, *) | ||
| extension TestingChannelSecureUpgradeServer { | ||
| // A Secure Upgrade HTTP client backed by a `NIOAsyncTestingChannel`. | ||
| struct Client { |
There was a problem hiding this comment.
Same comment here as in the H1 client
| ).get() | ||
|
|
||
| let connectionPromise = clientTestChannel.eventLoop.makePromise(of: Void.self) | ||
| clientTestChannel.connect(to: try .init(ipAddress: "127.0.0.1", port: 8000), promise: connectionPromise) |
There was a problem hiding this comment.
Does this port matter at all or is it needed just to activate the channel?
There was a problem hiding this comment.
It's needed just to activate the channel. I've changed this to call the createActiveChannel() method instead, which documents the need for calling connect().
…r `Channel` and `ClientBootstrap`
2 participants
Motivation:
We currently have two mechanisms for testing
NIOHTTPServerin an end-to-end manner. The first mechanism involves starting upNIOHTTPServeron localhost, and setting up a NIO client (backed by NIO'sClientBootstrap) to send requests / observe responses. The second mechanism involves spoofing aNIOAsyncTestingChannelas the channelNIOHTTPServerruns on, wiring a client channel (also backed by aNIOAsyncTestingChannel) to the server channel, and writing/reading to/from the client channel.Currently, both mechanisms share some duplicated code, have no documentation, and in general are poorly named/structured. We should improve this.
Modifications:
Clienttype broken down intoNIOHTTP1ClientandNIOSecureUpgradeClient.NIOSecureUpgradeClient'ssetUpChannelmethod now returns aNegotiatedClientConnectiontype instead of just opening one stream and returning the stream channel. For HTTP/2,NegotiatedClientConnectionhas anopenStreammethod; this enables multiple streams to be created in tests.NIOAsyncTestingChannel-based mechanism is now structured intoTestingChannelHTTP1ServerandTestingChannelSecureUpgradeServer. Both contain awithClientstatic function that vend aTestingChannelHTTP1Client/TestingChannelSecureUpgradeClientrespectively.Result:
Easier to write more involving tests.