test: deflaking yet another test#19775
Merged
alyssawilk merged 4 commits intoenvoyproxy:mainfrom Feb 3, 2022
Merged
Conversation
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
adisuissa
reviewed
Feb 2, 2022
Contributor
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
Thanks for the cleanup.
Left a couple of minor comments and a question.
/wait
| @@ -464,6 +464,7 @@ name: matcher | |||
| auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); | |||
Contributor
There was a problem hiding this comment.
The call to makeHttpConnection in line 461 should be moved here (as is done in the other test that was updated).
test/integration/integration_test.cc
Outdated
|
|
||
| ASSERT_TRUE(response->waitForEndStream()); | ||
| EXPECT_THAT(response->headers(), HttpStatusIs("200")); | ||
| codec_client_->close(); |
Contributor
There was a problem hiding this comment.
nit: the codec_client_->close() on line 493 is moot.
Question: What is the purpose of the second codec, after the codec_client_ is already closed? will the rest of this test just be the same as using codec_client_ instead of second_codec?
Contributor
Author
There was a problem hiding this comment.
I'm not sure. maybe it was intentional to have 2 connections? I'll leave the final close hre and remove the prior just in case
joshperry
pushed a commit
to joshperry/envoy
that referenced
this pull request
Feb 13, 2022
As detailed in the linked issue, the lack of delay-close in integration tests was causing windows to react to FINs before reading error bodies, so turning up delay close. Fixes envoyproxy#19430 Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: Josh Perry <josh.perry@mx.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
As detailed in the linked issue, the lack of delay-close in integration tests was causing windows to react to FINs before reading error bodies, so turning up delay close.
Fixes #19430