Skip to content

Commit

Permalink
test: changing waitForEndStream to fail if it takes 30s (envoyproxy#1…
Browse files Browse the repository at this point in the history
…5812)

Turns out the envoy default timeout of 30s means we had 2 tests waiting for a response that were just hanging 30s.
Fixing both tests (1 overload, 1 quic) and futureproofing.

Risk Level: n/a (test only)
Testing: yes
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <[email protected]>
  • Loading branch information
alyssawilk authored Apr 7, 2021
1 parent 7d49926 commit d6acb14
Show file tree
Hide file tree
Showing 58 changed files with 396 additions and 363 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ TEST_P(AggregateIntegrationTest, PreviousPrioritiesRetryPredicate) {
waitForNextUpstreamRequest(SecondUpstreamIndex);
upstream_request_->encodeHeaders(default_response_headers_, true);

response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
EXPECT_TRUE(upstream_request_->complete());

EXPECT_TRUE(response->complete());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void AdaptiveConcurrencyIntegrationTest::respondToRequest(bool expect_forwarded)
upstream_requests_.front()->encodeData(1, true);
}

responses_.front()->waitForEndStream();
ASSERT_TRUE(responses_.front()->waitForEndStream());

if (expect_forwarded) {
EXPECT_TRUE(upstream_requests_.front()->complete());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class AdmissionControlIntegrationTest : public Event::TestUsingSimulatedTime,
au->setResponseTrailers(std::move(trailers));

auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_);
response->waitForEndStream();
RELEASE_ASSERT(response->waitForEndStream(), "unexpected timeout");
codec_client_->close();
return response;
}
Expand All @@ -84,7 +84,7 @@ class AdmissionControlIntegrationTest : public Event::TestUsingSimulatedTime,
Http::TestResponseHeaderMapImpl({{":status", code}})));

auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_);
response->waitForEndStream();
RELEASE_ASSERT(response->waitForEndStream(), "unexpected timeout");
codec_client_->close();
return response;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class AwsLambdaFilterIntegrationTest : public testing::TestWithParam<Network::Ad
upstream_request_->encodeData(buffer, true);
}

response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
EXPECT_TRUE(response->complete());

// verify headers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ TEST_P(BufferIntegrationTest, RouterRequestPopulateContentLength) {
ASSERT_NE(content_length, nullptr);
EXPECT_EQ(content_length->value().getStringView(), "9");

response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
ASSERT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().getStatusValue());
}
Expand Down Expand Up @@ -90,7 +90,7 @@ TEST_P(BufferIntegrationTest, RouterRequestPopulateContentLengthOnTrailers) {
ASSERT_NE(content_length, nullptr);
EXPECT_EQ(content_length->value().getStringView(), "10");

response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
ASSERT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().getStatusValue());
}
Expand All @@ -117,7 +117,7 @@ TEST_P(BufferIntegrationTest, RouterRequestBufferLimitExceeded) {
{"x-envoy-retry-on", "5xx"}},
1024 * 65);

response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
ASSERT_TRUE(response->complete());
EXPECT_EQ("413", response->headers().getStatusValue());
}
Expand Down Expand Up @@ -159,7 +159,7 @@ TEST_P(BufferIntegrationTest, RouteDisabled) {
waitForNextUpstreamRequest();
upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true);

response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
ASSERT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().getStatusValue());
}
Expand All @@ -185,7 +185,7 @@ TEST_P(BufferIntegrationTest, RouteOverride) {
waitForNextUpstreamRequest();
upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true);

response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
ASSERT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().getStatusValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ TEST_P(CacheIntegrationTest, MissInsertHit) {
// send 42 'a's
upstream_request_->encodeData(response_body, /*end_stream=*/true);
// Wait for the response to be read by the codec client.
response_decoder->waitForEndStream();
ASSERT_TRUE(response_decoder->waitForEndStream());
EXPECT_TRUE(response_decoder->complete());
EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers));
EXPECT_TRUE(response_decoder->headers().get(Http::CustomHeaders::get().Age).empty());
Expand All @@ -84,7 +84,7 @@ TEST_P(CacheIntegrationTest, MissInsertHit) {
{
IntegrationStreamDecoderPtr response_decoder =
codec_client_->makeHeaderOnlyRequest(request_headers);
response_decoder->waitForEndStream();
ASSERT_TRUE(response_decoder->waitForEndStream());
EXPECT_TRUE(response_decoder->complete());
EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers));
EXPECT_EQ(response_decoder->body(), response_body);
Expand Down Expand Up @@ -127,7 +127,7 @@ TEST_P(CacheIntegrationTest, ExpiredValidated) {
// send 42 'a's
upstream_request_->encodeData(response_body, true);
// Wait for the response to be read by the codec client.
response_decoder->waitForEndStream();
ASSERT_TRUE(response_decoder->waitForEndStream());
EXPECT_TRUE(response_decoder->complete());
EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers));
EXPECT_TRUE(response_decoder->headers().get(Http::CustomHeaders::get().Age).empty());
Expand Down Expand Up @@ -159,7 +159,7 @@ TEST_P(CacheIntegrationTest, ExpiredValidated) {
response_headers.setDate(not_modified_date);

// Wait for the response to be read by the codec client.
response_decoder->waitForEndStream();
ASSERT_TRUE(response_decoder->waitForEndStream());

// Check that the served response is the cached response
EXPECT_TRUE(response_decoder->complete());
Expand Down Expand Up @@ -208,7 +208,7 @@ TEST_P(CacheIntegrationTest, ExpiredFetchedNewResponse) {
// send 10 'a's
upstream_request_->encodeData(response_body, /*end_stream=*/true);
// Wait for the response to be read by the codec client.
response_decoder->waitForEndStream();
ASSERT_TRUE(response_decoder->waitForEndStream());
EXPECT_TRUE(response_decoder->complete());
EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers));
EXPECT_TRUE(response_decoder->headers().get(Http::CustomHeaders::get().Age).empty());
Expand Down Expand Up @@ -244,7 +244,7 @@ TEST_P(CacheIntegrationTest, ExpiredFetchedNewResponse) {
upstream_request_->encodeData(response_body, /*end_stream=*/true);

// Wait for the response to be read by the codec client.
response_decoder->waitForEndStream();
ASSERT_TRUE(response_decoder->waitForEndStream());
// Check that the served response is the updated response
EXPECT_TRUE(response_decoder->complete());
EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers));
Expand Down Expand Up @@ -289,7 +289,7 @@ TEST_P(CacheIntegrationTest, GetRequestWithBodyAndTrailers) {
// send 42 'a's
upstream_request_->encodeData(42, true);
// Wait for the response to be read by the codec client.
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
EXPECT_TRUE(response->complete());
EXPECT_THAT(response->headers(), IsSupersetOfHeaders(response_headers));
EXPECT_TRUE(response->headers().get(Http::CustomHeaders::get().Age).empty());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ TEST_P(CdnLoopFilterIntegrationTest, CdnLoop0Allowed1Seen) {
{"CDN-Loop", "cdn"}};

auto response = codec_client_->makeHeaderOnlyRequest(request_headers);
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
ASSERT_TRUE(response->complete());
EXPECT_EQ("502", response->headers().getStatusValue());
}
Expand All @@ -114,7 +114,7 @@ TEST_P(CdnLoopFilterIntegrationTest, UnparseableHeader) {
{"CDN-Loop", "[bad-header"}};

auto response = codec_client_->makeHeaderOnlyRequest(request_headers);
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
ASSERT_TRUE(response->complete());
EXPECT_EQ("400", response->headers().getStatusValue());
}
Expand Down Expand Up @@ -171,7 +171,7 @@ TEST_P(CdnLoopFilterIntegrationTest, CdnLoop2Allowed3Seen) {
{"CDN-Loop", "cdn, cdn, cdn"}};

auto response = codec_client_->makeHeaderOnlyRequest(request_headers);
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
ASSERT_TRUE(response->complete());
EXPECT_EQ("502", response->headers().getStatusValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class CorsFilterIntegrationTest : public testing::TestWithParam<Network::Address
initialize();
codec_client_ = makeHttpConnection(lookupPort("http"));
auto response = codec_client_->makeHeaderOnlyRequest(request_headers);
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
EXPECT_TRUE(response->complete());
compareHeaders(response->headers(), expected_response_headers);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class CsrfFilterIntegrationTest : public HttpProtocolIntegrationTest {
auto response = codec_client_->makeRequestWithBody(request_headers, 1024);
waitForNextUpstreamRequest();
upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true);
response->waitForEndStream();
RELEASE_ASSERT(response->waitForEndStream(), "unexpected timeout");

return response;
}
Expand All @@ -68,7 +68,7 @@ class CsrfFilterIntegrationTest : public HttpProtocolIntegrationTest {
initialize();
codec_client_ = makeHttpConnection(lookupPort("http"));
auto response = codec_client_->makeRequestWithBody(request_headers, 1024);
response->waitForEndStream();
RELEASE_ASSERT(response->waitForEndStream(), "unexpected timeout");

return response;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ TEST_P(DecompressorIntegrationTest, BidirectionalDecompression) {
upstream_request_->encodeData(response_data2, true);

// Wait for frames to arrive downstream.
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());

// Assert that the total bytes received downstream equal the sum of the uncompressed byte buffers
// sent.
Expand Down Expand Up @@ -264,7 +264,7 @@ TEST_P(DecompressorIntegrationTest, BidirectionalDecompressionError) {
upstream_request_->encodeData(response_data2, true);

// Wait for frames to arrive downstream.
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());

EXPECT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().Status()->value().getStringView());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ TEST_P(ProxyFilterIntegrationTest, DNSCacheHostOverflow) {
{":scheme", "http"},
{":authority", fmt::format("localhost2", fake_upstreams_[0]->localAddress()->ip()->port())}};
response = codec_client_->makeHeaderOnlyRequest(request_headers2);
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
EXPECT_EQ("503", response->headers().getStatusValue());
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.host_overflow")->value());
}
Expand All @@ -248,7 +248,7 @@ TEST_P(ProxyFilterIntegrationTest, UpstreamTls) {
EXPECT_STREQ("localhost", SSL_get_servername(ssl_socket->ssl(), TLSEXT_NAMETYPE_host_name));

upstream_request_->encodeHeaders(default_response_headers_, true);
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
checkSimpleRequestSuccess(0, 0, response.get());
}

Expand All @@ -272,7 +272,7 @@ TEST_P(ProxyFilterIntegrationTest, UpstreamTlsWithIpHost) {
EXPECT_STREQ(nullptr, SSL_get_servername(ssl_socket->ssl(), TLSEXT_NAMETYPE_host_name));

upstream_request_->encodeHeaders(default_response_headers_, true);
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
checkSimpleRequestSuccess(0, 0, response.get());
}

Expand All @@ -292,7 +292,7 @@ TEST_P(ProxyFilterIntegrationTest, UpstreamTlsInvalidSAN) {
fmt::format("localhost:{}", fake_upstreams_[0]->localAddress()->ip()->port())}};

auto response = codec_client_->makeHeaderOnlyRequest(request_headers);
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
EXPECT_EQ("503", response->headers().getStatusValue());

EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.ssl.fail_verify_san")->value());
Expand All @@ -310,7 +310,7 @@ TEST_P(ProxyFilterIntegrationTest, DnsCacheCircuitBreakersInvoked) {
fmt::format("localhost:{}", fake_upstreams_[0]->localAddress()->ip()->port())}};

auto response = codec_client_->makeRequestWithBody(request_headers, 1024);
response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.dns_rq_pending_overflow")->value());

EXPECT_TRUE(response->complete());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ class ExtAuthzGrpcIntegrationTest : public Grpc::VersionedGrpcClientIntegrationP
upstream_request_->headers().get(Http::LowerCaseString{header_to_remove.first}).empty());
}

response_->waitForEndStream();
ASSERT_TRUE(response_->waitForEndStream());

EXPECT_TRUE(upstream_request_->complete());
EXPECT_EQ(request_body_.length(), upstream_request_->bodyLength());
Expand Down Expand Up @@ -531,7 +531,7 @@ class ExtAuthzHttpIntegrationTest : public HttpIntegrationTest,
.empty());

upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true);
response_->waitForEndStream();
ASSERT_TRUE(response_->waitForEndStream());
EXPECT_TRUE(response_->complete());
EXPECT_EQ("200", response_->headers().getStatusValue());

Expand Down Expand Up @@ -777,7 +777,7 @@ TEST_P(ExtAuthzLocalReplyIntegrationTest, DeniedHeaderTest) {
};
ext_authz_request->encodeHeaders(ext_authz_response_headers, true);

response->waitForEndStream();
ASSERT_TRUE(response->waitForEndStream());
EXPECT_TRUE(response->complete());

EXPECT_EQ("401", response->headers().Status()->value().getStringView());
Expand Down Expand Up @@ -853,7 +853,7 @@ TEST_P(ExtAuthzGrpcIntegrationTest, GoogleAsyncClientCreation) {
upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, false);
upstream_request_->encodeData(response_size_, true);

response_->waitForEndStream();
ASSERT_TRUE(response_->waitForEndStream());

EXPECT_TRUE(upstream_request_->complete());
EXPECT_EQ(request_body_.length(), upstream_request_->bodyLength());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class ExtProcIntegrationTest : public HttpIntegrationTest,
}

void verifyDownstreamResponse(IntegrationStreamDecoder& response, int status_code) {
response.waitForEndStream();
ASSERT_TRUE(response.waitForEndStream());
EXPECT_TRUE(response.complete());
EXPECT_EQ(std::to_string(status_code), response.headers().getStatusValue());
}
Expand Down
Loading

0 comments on commit d6acb14

Please sign in to comment.