From e744e5b3ffb68adf047440719d1bd80cd4edabef Mon Sep 17 00:00:00 2001 From: antonio Date: Tue, 29 Sep 2020 19:12:29 -0400 Subject: [PATCH] backport to v1.13: test flake: fix flake in ApiListener integration test (#9808) (#13316) Description: previously the test was not waiting for the expectation on the server's thread to complete. Therefore, there was a use after free race condition with the ApiListener's TlsCachingDateProvider. This PR makes it so that the test waits for the expectation to be fulfilled and thus prevents the race. Risk Level: low Testing: ran integration test a few thousand times locally on a linux machine. Signed-off-by: Jose Nino Signed-off-by: Antonio Vicente --- VERSION | 2 +- docs/root/intro/version_history.rst | 6 ++++++ test/integration/api_listener_integration_test.cc | 13 +++++++------ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/VERSION b/VERSION index 43ded906259f..2e3a551febd8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.13.5 +1.13.6 diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index e3190e972992..5e7a542b054a 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -1,6 +1,12 @@ Version history --------------- +1.13.6 (September 29, 2020) +=========================== +Changes +------- +* test: fix flaky test. + 1.13.5 (September 29, 2020) =========================== Changes diff --git a/test/integration/api_listener_integration_test.cc b/test/integration/api_listener_integration_test.cc index 0fa47eb8275c..7328d87ea3b9 100644 --- a/test/integration/api_listener_integration_test.cc +++ b/test/integration/api_listener_integration_test.cc @@ -4,6 +4,7 @@ #include "test/test_common/environment.h" #include "test/test_common/utility.h" +#include "absl/synchronization/notification.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -76,13 +77,15 @@ name: api_listener NiceMock stream_encoder_; }; +ACTION_P(Notify, notification) { notification->Notify(); } + INSTANTIATE_TEST_SUITE_P(IpVersions, ApiListenerIntegrationTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); TEST_P(ApiListenerIntegrationTest, Basic) { - ConditionalInitializer test_ran; - test_server_->server().dispatcher().post([this, &test_ran]() -> void { + absl::Notification done; + test_server_->server().dispatcher().post([this, &done]() -> void { ASSERT_TRUE(test_server_->server().listenerManager().apiListener().has_value()); ASSERT_EQ("api_listener", test_server_->server().listenerManager().apiListener()->get().name()); ASSERT_TRUE(test_server_->server().listenerManager().apiListener()->get().http().has_value()); @@ -97,17 +100,15 @@ TEST_P(ApiListenerIntegrationTest, Basic) { Http::TestHeaderMapImpl expected_response_headers{{":status", "200"}}; EXPECT_CALL(stream_encoder_, encodeHeaders(_, false)); EXPECT_CALL(stream_encoder_, encodeData(_, false)); - EXPECT_CALL(stream_encoder_, encodeData(BufferStringEqual(""), true)); + EXPECT_CALL(stream_encoder_, encodeData(BufferStringEqual(""), true)).WillOnce(Notify(&done)); // Send a headers-only request stream_decoder.decodeHeaders( Http::HeaderMapPtr(new Http::TestHeaderMapImpl{ {":method", "GET"}, {":path", "/api"}, {":scheme", "http"}, {":authority", "host"}}), true); - - test_ran.setReady(); }); - test_ran.waitReady(); + ASSERT_TRUE(done.WaitForNotificationWithTimeout(absl::Seconds(1))); } } // namespace