From 9b1039410c4dc2cf9a3095253d09e17ad908d070 Mon Sep 17 00:00:00 2001 From: Yiming Jin Date: Sat, 8 May 2021 17:42:34 -0700 Subject: [PATCH] Address review comment for unit test Signed-off-by: Yiming Jin --- test/integration/BUILD | 2 +- test/integration/filters/BUILD | 4 ++-- ...cc => local_reply_with_metadata_filter.cc} | 16 ++++++++-------- test/integration/protocol_integration_test.cc | 19 ++++++------------- 4 files changed, 17 insertions(+), 24 deletions(-) rename test/integration/filters/{continue_after_local_reply_with_metadata_filter.cc => local_reply_with_metadata_filter.cc} (63%) diff --git a/test/integration/BUILD b/test/integration/BUILD index 77a2cb0af2ac..644bb1127a5f 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -447,12 +447,12 @@ envoy_cc_test_library( "//source/extensions/filters/http/health_check:config", "//test/common/http/http2:http2_frame", "//test/integration/filters:continue_after_local_reply_filter_lib", - "//test/integration/filters:continue_after_local_reply_filter_with_metadata_lib", "//test/integration/filters:continue_headers_only_inject_body", "//test/integration/filters:encoder_decoder_buffer_filter_lib", "//test/integration/filters:invalid_header_filter_lib", "//test/integration/filters:local_reply_during_encoding_data_filter_lib", "//test/integration/filters:local_reply_during_encoding_filter_lib", + "//test/integration/filters:local_reply_with_metadata_filter_lib", "//test/integration/filters:random_pause_filter_lib", "//test/test_common:logging_lib", "//test/test_common:utility_lib", diff --git a/test/integration/filters/BUILD b/test/integration/filters/BUILD index 6aaaf8415dd5..04e8b0abd67c 100644 --- a/test/integration/filters/BUILD +++ b/test/integration/filters/BUILD @@ -70,9 +70,9 @@ envoy_cc_test_library( ) envoy_cc_test_library( - name = "continue_after_local_reply_filter_with_metadata_lib", + name = "local_reply_with_metadata_filter_lib", srcs = [ - "continue_after_local_reply_with_metadata_filter.cc", + "local_reply_with_metadata_filter.cc", ], deps = [ ":common_lib", diff --git a/test/integration/filters/continue_after_local_reply_with_metadata_filter.cc b/test/integration/filters/local_reply_with_metadata_filter.cc similarity index 63% rename from test/integration/filters/continue_after_local_reply_with_metadata_filter.cc rename to test/integration/filters/local_reply_with_metadata_filter.cc index 7ce9da964ab4..07580be29c6a 100644 --- a/test/integration/filters/continue_after_local_reply_with_metadata_filter.cc +++ b/test/integration/filters/local_reply_with_metadata_filter.cc @@ -10,30 +10,30 @@ namespace Envoy { -// A filter that calls Http::FilterHeadersStatus::Continue and set metadata after a local reply. -class ContinueAfterLocalReplyWithMetadataFilter : public Http::PassThroughFilter { +// A filter that calls Http::FilterHeadersStatus::StopAll and set metadata after a local reply. +class LocalReplyWithMetadataFilter : public Http::PassThroughFilter { public: - constexpr static char name[] = "continue-after-local-reply-with-metadata-filter"; + constexpr static char name[] = "local-reply-with-metadata-filter"; Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap&, bool) override { Http::MetadataMap metadata_map = {{"headers", "headers"}, {"duplicate", "duplicate"}}; Http::MetadataMapPtr metadata_map_ptr = std::make_unique(metadata_map); decoder_callbacks_->addDecodedMetadata().emplace_back(std::move(metadata_map_ptr)); decoder_callbacks_->sendLocalReply(Envoy::Http::Code::OK, "", nullptr, absl::nullopt, - "ContinueAfterLocalReplyFilter is ready"); - return Http::FilterHeadersStatus::Continue; + "LocalReplyWithMetadataFilter is ready"); + return Http::FilterHeadersStatus::StopIteration; } // Adds new metadata to metadata_map directly Http::FilterMetadataStatus decodeMetadata(Http::MetadataMap& metadata_map) override { metadata_map["data"] = "data"; - throw EnvoyException("Should not call decode metadata after local reply."); + ASSERT(false); return Http::FilterMetadataStatus::Continue; } }; -constexpr char ContinueAfterLocalReplyWithMetadataFilter::name[]; -static Registry::RegisterFactory, +constexpr char LocalReplyWithMetadataFilter::name[]; +static Registry::RegisterFactory, Server::Configuration::NamedHttpFilterConfigFactory> register_; diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 58a787bd71a5..3c799a0566a5 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -2316,9 +2316,9 @@ TEST_P(DownstreamProtocolIntegrationTest, HeaderNormalizationRejection) { // Tests a filter that returns a FilterHeadersStatus::Continue after a local reply without // processing new metadata generated in decodeHeader -TEST_P(DownstreamProtocolIntegrationTest, ContinueAfterLocalReplyWithMetadata) { +TEST_P(DownstreamProtocolIntegrationTest, LocalReplyWithMetadata) { config_helper_.addFilter(R"EOF( - name: continue-after-local-reply-with-metadata-filter + name: local-reply-with-metadata-filter typed_config: "@type": type.googleapis.com/google.protobuf.Empty )EOF"); @@ -2326,17 +2326,10 @@ TEST_P(DownstreamProtocolIntegrationTest, ContinueAfterLocalReplyWithMetadata) { codec_client_ = makeHttpConnection(lookupPort("http")); // Send a headers only request. - IntegrationStreamDecoderPtr response; - EXPECT_ENVOY_BUG( - { - response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); - ASSERT_TRUE(response->waitForEndStream()); - ASSERT_TRUE(response->complete()); - ASSERT_EQ("200", response->headers().getStatusValue()); - ENVOY_LOG(info, "test upstream request"); - }, - "envoy bug failure: !continue_iteration || !state_.local_complete_. " - "Details: Filter did not return StopAll or StopIteration after sending a local reply."); + auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + ASSERT_EQ("200", response->headers().getStatusValue()); } } // namespace Envoy