[HTTP2] More Integration Tests #471
Conversation
| XCTAssertThrowsError(try task.futureResult.timeout(after: .seconds(2)).wait(), "Should fail") { error in | ||
| guard case let error = error as? HTTPClientError, error == .cancelled else { | ||
| return XCTFail("Should fail with cancelled") | ||
| } | ||
| } |
There was a problem hiding this comment.
| XCTAssertThrowsError(try task.futureResult.timeout(after: .seconds(2)).wait(), "Should fail") { error in | |
| guard case let error = error as? HTTPClientError, error == .cancelled else { | |
| return XCTFail("Should fail with cancelled") | |
| } | |
| } | |
| XCTAssertThrowsError(try task.futureResult.timeout(after: .seconds(2)).wait()) { | |
| XCTAssertEqual($0 as? HTTPClientError, . cancelled) | |
| } |
| if let clientError = error as? HTTPClientError, clientError == .cancelled { | ||
| continue | ||
| } else { | ||
| XCTFail("Unexpected error: \(error)") | ||
| } |
There was a problem hiding this comment.
| if let clientError = error as? HTTPClientError, clientError == .cancelled { | |
| continue | |
| } else { | |
| XCTFail("Unexpected error: \(error)") | |
| } | |
| XCTAssert(error as? HTTPClientError, .cancelled) |
| XCTAssertThrowsError(try task.futureResult.timeout(after: .seconds(2)).wait(), "Should fail") { error in | ||
| guard case let error = error as? HTTPClientError, error == .cancelled else { | ||
| return XCTFail("Should fail with cancelled") | ||
| } | ||
| } |
There was a problem hiding this comment.
| XCTAssertThrowsError(try task.futureResult.timeout(after: .seconds(2)).wait(), "Should fail") { error in | |
| guard case let error = error as? HTTPClientError, error == .cancelled else { | |
| return XCTFail("Should fail with cancelled") | |
| } | |
| } | |
| XCTAssertThrowsError(try task.futureResult.timeout(after: .seconds(2)).wait()) { | |
| XCTAssertEqual($0 as? HTTPClientError, .cancelled) | |
| } |
| case "/sendheaderandwait": | ||
| // sends some headers and waits indefinitely afterwards | ||
| context.writeAndFlush(wrapOutboundOut(.head(HTTPResponseHead(version: HTTPVersion(major: 1, minor: 1), status: .ok))), promise: nil) | ||
| return |
There was a problem hiding this comment.
Please create a small ChannelInboundHandler for this in the specific test case. There is so much stuff in this single handler and all is mixed.
| } | ||
|
|
||
| var backpressureEventLoop: EventLoop? | ||
| var stateDidChangeCallback: ((State) -> Void)? |
There was a problem hiding this comment.
this should be at least a let. however in your tests your mainly using this to get a callback once the head was received. I think, a specialized delegate only for this one job, might be a better idea.
dnadoba
force-pushed
the
dn-http2-intergration-tests
branch
2 times, most recently
from
02d3a90 to
1323450
Compare
| @@ -67,7 +82,7 @@ class HTTP2ClientTests: XCTestCase { | |||
| func testConcurrentRequestsFromDifferentThreads() { | |||
| let bin = HTTPBin(.http2(compress: false)) | |||
| defer { XCTAssertNoThrow(try bin.shutdown()) } | |||
| let client = self.makeDefaultHTTPClient() | |||
| let client = self.makeClientWithActiveHTTP2Connection(to: bin) | |||
There was a problem hiding this comment.
why do we need a client with an active connection?
There was a problem hiding this comment.
good catch, we don't.
| context.writeAndFlush(wrapOutboundOut(.head(HTTPResponseHead( | ||
| version: HTTPVersion(major: 1, minor: 1), | ||
| status: .ok | ||
| )) | ||
| ), promise: nil) |
There was a problem hiding this comment.
This will send at least two response headers for each request... you'll need to unwrap the incoming requests for .head and .end
There was a problem hiding this comment.
could we also use channelActive instead?
There was a problem hiding this comment.
nope... you need to wait for the request. you can not assume to get a request as soon as the channel become active.
There was a problem hiding this comment.
okay thanks, we now only send it after we received .end.
| init( | ||
| backpressureEventLoop: EventLoop? = nil, | ||
| stateDidChangeCallback: ((State) -> Void)? = nil | ||
| ) { |
There was a problem hiding this comment.
I think all changes in this file can now be reverted, can't they?
| } | ||
|
|
||
| /// sends some headers and waits indefinitely afterwards | ||
| private final class SendHeaderAndWaitChannelHandler: ChannelInboundHandler { |
There was a problem hiding this comment.
oh we should use this one to test idleReadTimeout as well. can be another pr.
we needed an event loop group which outlives the http client
dnadoba
force-pushed
the
dn-http2-intergration-tests
branch
from
c67e336 to
c82a3a5
Compare
No description provided.