add test case for stream reset during delay injection#354
add test case for stream reset during delay injection#354mattklein123 merged 2 commits intoenvoyproxy:masterfrom
Conversation
|
@mattklein123 LMK if this covers the test case you talked about. |
|
|
||
| // Prep up with a 5s delay | ||
| EXPECT_CALL(runtime_.snapshot_, featureEnabled("fault.http.delay.fixed_delay_percent", 100)) | ||
| .Times(1) |
There was a problem hiding this comment.
nit: Times(1) is implied when you do .WillOnce() or just a plain expect_call, so can typically be removed. There are multiple cases in here where this applies.
| EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); | ||
| EXPECT_EQ(0UL, config_->stats().aborts_injected_.value()); | ||
|
|
||
| EXPECT_EQ(FilterDataStatus::Continue, filter_->decodeData(data_, true)); |
There was a problem hiding this comment.
Though it works from a "test this thing" perspective, this is not a totally realistic test case. What you are doing here is calling decodeData with end_stream = true. This means that the request is over, as such it's impossible for decodeTrailers() to be called, so I would delete line 453. FYI, it is possible for resetStream callback to get called even when request is complete, since downstream can reset waiting for a response.
There was a problem hiding this comment.
Good to know. fixed the code and removed the silly .Times(1) from all tests. Thanks for the feedback!
…lter.rst (envoyproxy#479) * envoyproxy#354 translate dynamodb_filter * envoyproxy#354 zh-translation: docs/root/configuration/http/http_filters/dynamodb_filter.rst, fix * envoyproxy#354 zh-translation: docs/root/configuration/http/http_filters/dynamodb_filter.rst, fix review comment Co-authored-by: niushaohan <shaohan.niu@dfgroup.pro>
Per the discussion [here](envoyproxy/envoy-mobile#312 (comment)), updating the unary and streaming interfaces. - Moved the non-streaming convenience function into an extension on the protocol type so that it's available to all consumers - Added a test to validate the default behavior of this extension function - Added a `CancelableStream` protocol which includes a subset of functionality from the `StreamEmitter`, allowing consumers of the unary function to cancel requests without having the ability to send additional data into the stream Signed-off-by: Michael Rebello <me@michaelrebello.com> Signed-off-by: JP Simard <jp@jpsim.com>
Per the discussion [here](envoyproxy/envoy-mobile#312 (comment)), updating the unary and streaming interfaces. - Moved the non-streaming convenience function into an extension on the protocol type so that it's available to all consumers - Added a test to validate the default behavior of this extension function - Added a `CancelableStream` protocol which includes a subset of functionality from the `StreamEmitter`, allowing consumers of the unary function to cancel requests without having the ability to send additional data into the stream Signed-off-by: Michael Rebello <me@michaelrebello.com> Signed-off-by: JP Simard <jp@jpsim.com>
fixes #342