Multiplexer tests for payload-based stream channels #225
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Motivation: As part of apple#214 we added a stream channel which deals with `HTTP2Frame.FramePayload` as well as the relevant entry points in `HTTP2StreamMultiplexer` to create these streams. We didn't add a corresponding set of tests. Modifications: - Duplicate `HTTP2StreamMultiplexerTests` - Refactor to make use of new stream creation function and new inbound stream initializer - Fix up a couple of bugs in `HTTP2StreamMultiplexer`: - activate pending streams on `channelActive` - deactivate pending streams on `channelInactive` - change stream writability for pending streams on `channelWritabilityChanged` - add implementation for `childChannelClosed(channelID:)` Result: - Tests for payload-based stream creation via `HTTP2StreamMultiplexer`
glbrntt
commented
Aug 3, 2020
Comment on lines
+1046
to
+1051
| // Write some headers: the flush will trigger a stream ID to be assigned to the channel. | ||
| channel.writeAndFlush(HTTP2Frame.FramePayload.headers(.init(headers: [:]))).whenSuccess { | ||
| channel.getOption(HTTP2StreamChannelOptions.streamID).whenSuccess { streamID in | ||
| streamIDs.append(streamID) | ||
| } | ||
| } |
There was a problem hiding this comment.
There are some changes here.
Comment on lines
+1000
to
+1005
| // Write some headers: the flush will trigger a stream ID to be assigned to the channel. | ||
| channel.writeAndFlush(HTTP2Frame.FramePayload.headers(.init(headers: [:]))).whenSuccess { | ||
| channel.getOption(HTTP2StreamChannelOptions.streamID).whenSuccess { streamID in | ||
| streamIDs.append(streamID) | ||
| } | ||
| } |
There was a problem hiding this comment.
There are some changes here.
Comment on lines
+1575
to
+1578
| // We need to write (and flush) some data so that the streams get stream IDs. | ||
| for childChannel in channels { | ||
| XCTAssertNoThrow(try childChannel.writeAndFlush(HTTP2Frame.FramePayload.headers(.init(headers: [:]))).wait()) | ||
| } |
Comment on lines
+1622
to
+1623
| // We need to write (and flush) some data so that the streams get stream IDs. | ||
| XCTAssertNoThrow(try childChannel.writeAndFlush(HTTP2Frame.FramePayload.headers(.init(headers: [:]))).wait()) |
Lukasa
approved these changes
Aug 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
As part of #214 we added a stream channel which deals with
HTTP2Frame.FramePayloadas well as the relevant entry points inHTTP2StreamMultiplexerto create these streams. We didn't add acorresponding set of tests.
Modifications:
HTTP2StreamMultiplexerTestsstream initializer
HTTP2StreamMultiplexer:channelActivechannelInactivechannelWritabilityChangedchildChannelClosed(channelID:)Result:
HTTP2StreamMultiplexer