From 1ee408c2232673616f435d806f112ab681b9f574 Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Tue, 10 Jul 2018 16:30:39 -0400 Subject: [PATCH 1/6] Add timeouts to methods in fake_upstream.h that could wait forever. Signed-off-by: Michael Behr --- test/integration/ads_integration_test.cc | 16 +- test/integration/fake_upstream.cc | 235 ++++++++++++------ test/integration/fake_upstream.h | 199 ++++++++++----- test/integration/h1_capture_fuzz_test.cc | 11 +- test/integration/hds_integration_test.cc | 16 +- test/integration/header_integration_test.cc | 12 +- test/integration/http2_integration_test.cc | 51 ++-- .../http2_upstream_integration_test.cc | 41 +-- test/integration/http_integration.cc | 116 ++++----- .../idle_timeout_integration_test.cc | 6 +- test/integration/integration_test.cc | 26 +- .../load_stats_integration_test.cc | 18 +- .../integration/ratelimit_integration_test.cc | 24 +- .../tcp_conn_pool_integration_test.cc | 24 +- .../integration/tcp_proxy_integration_test.cc | 142 ++++++----- .../integration/websocket_integration_test.cc | 124 ++++----- test/integration/xfcc_integration_test.cc | 6 +- test/test_common/utility.h | 8 + 18 files changed, 647 insertions(+), 428 deletions(-) diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index d0a72ed610d54..02a319b321991 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -68,7 +68,7 @@ class AdsIntegrationBaseTest : public HttpIntegrationTest { void createAdsConnection(FakeUpstream& upstream) { ads_upstream_ = &upstream; - ads_connection_ = ads_upstream_->waitForHttpConnection(*dispatcher_); + ASSERT(ads_upstream_->waitForHttpConnection(*dispatcher_, &ads_connection_)); } void cleanUpAdsConnection() { @@ -76,8 +76,8 @@ class AdsIntegrationBaseTest : public HttpIntegrationTest { // Don't ASSERT fail if an ADS reconnect ends up unparented. ads_upstream_->set_allow_unexpected_disconnects(true); - ads_connection_->close(); - ads_connection_->waitForDisconnect(); + ASSERT(ads_connection_->close()); + ASSERT(ads_connection_->waitForDisconnect()); ads_connection_.reset(); } @@ -126,7 +126,7 @@ class AdsIntegrationTest : public AdsIntegrationBaseTest, const Protobuf::int32 expected_error_code = Grpc::Status::GrpcStatus::Ok, const std::string& expected_error_message = "") { envoy::api::v2::DiscoveryRequest discovery_request; - ads_stream_->waitForGrpcMessage(*dispatcher_, discovery_request); + VERIFY_ASSERTION(ads_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); // TODO(PiotrSikora): Remove this hack once fixed internally. if (!(expected_type_url == discovery_request.type_url())) { @@ -266,7 +266,7 @@ class AdsIntegrationTest : public AdsIntegrationBaseTest, AdsIntegrationBaseTest::initialize(); if (ads_stream_ == nullptr) { createAdsConnection(*(fake_upstreams_[1])); - ads_stream_ = ads_connection_->waitForNewStream(*dispatcher_); + ASSERT(ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_)); ads_stream_->startGrpcStream(); } } @@ -511,7 +511,7 @@ INSTANTIATE_TEST_CASE_P(IpVersionsClientType, AdsFailIntegrationTest, TEST_P(AdsFailIntegrationTest, ConnectDisconnect) { initialize(); createAdsConnection(*fake_upstreams_[1]); - ads_stream_ = ads_connection_->waitForNewStream(*dispatcher_); + ASSERT_TRUE(ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_)); ads_stream_->startGrpcStream(); ads_stream_->finishGrpcStream(Grpc::Status::Internal); } @@ -564,7 +564,7 @@ INSTANTIATE_TEST_CASE_P(IpVersionsClientType, AdsConfigIntegrationTest, TEST_P(AdsConfigIntegrationTest, EdsClusterWithAdsConfigSource) { initialize(); createAdsConnection(*fake_upstreams_[1]); - ads_stream_ = ads_connection_->waitForNewStream(*dispatcher_); + ASSERT_TRUE(ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_)); ads_stream_->startGrpcStream(); ads_stream_->finishGrpcStream(Grpc::Status::Ok); } @@ -585,7 +585,7 @@ TEST_P(AdsIntegrationTest, XdsBatching) { pre_worker_start_test_steps_ = [this]() { createAdsConnection(*fake_upstreams_.back()); - ads_stream_ = ads_connection_->waitForNewStream(*dispatcher_); + ASSERT_TRUE(ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_)); ads_stream_->startGrpcStream(); EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "", diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 4ec4ae40aafbf..8651906979d2d 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -23,6 +23,15 @@ #include "test/test_common/printers.h" #include "test/test_common/utility.h" +#include "absl/strings/str_cat.h" + +using namespace std::chrono_literals; + +using std::chrono::milliseconds; +using testing::AssertionFailure; +using testing::AssertionResult; +using testing::AssertionSuccess; + namespace Envoy { FakeStream::FakeStream(FakeHttpConnection& parent, Http::StreamEncoder& encoder) : parent_(parent), encoder_(encoder) { @@ -100,42 +109,63 @@ void FakeStream::onResetStream(Http::StreamResetReason) { decoder_event_.notifyOne(); } -void FakeStream::waitForHeadersComplete() { +AssertionResult FakeStream::waitForHeadersComplete(milliseconds timeout) { Thread::LockGuard lock(lock_); + auto end_time = std::chrono::steady_clock::now() + timeout; while (!headers_) { - decoder_event_.wait(lock_); + if (std::chrono::steady_clock::now() >= end_time) { + return AssertionFailure() << "Timed out waiting for headers."; + } + decoder_event_.waitFor(lock_, 5ms); } + return AssertionSuccess(); } -void FakeStream::waitForData(Event::Dispatcher& client_dispatcher, uint64_t body_length) { +AssertionResult FakeStream::waitForData(Event::Dispatcher& client_dispatcher, uint64_t body_length, + milliseconds timeout) { Thread::LockGuard lock(lock_); + auto start_time = std::chrono::steady_clock::now(); while (bodyLength() < body_length) { - decoder_event_.waitFor(lock_, std::chrono::milliseconds(5)); + if (std::chrono::steady_clock::now() >= start_time + timeout) { + return AssertionFailure() << "Timed out waiting for data."; + } + decoder_event_.waitFor(lock_, 5ms); if (bodyLength() < body_length) { // Run the client dispatcher since we may need to process window updates, etc. client_dispatcher.run(Event::Dispatcher::RunType::NonBlock); } } + return AssertionSuccess(); } -void FakeStream::waitForEndStream(Event::Dispatcher& client_dispatcher) { - ENVOY_LOG(trace, "FakeStream waiting for end stream"); +AssertionResult FakeStream::waitForEndStream(Event::Dispatcher& client_dispatcher, + milliseconds timeout) { Thread::LockGuard lock(lock_); + auto start_time = std::chrono::steady_clock::now(); while (!end_stream_) { - decoder_event_.waitFor(lock_, std::chrono::milliseconds(5)); + if (std::chrono::steady_clock::now() >= start_time + timeout) { + return AssertionFailure() << "Timed out waiting for end of stream."; + } + decoder_event_.waitFor(lock_, 5ms); if (!end_stream_) { // Run the client dispatcher since we may need to process window updates, etc. client_dispatcher.run(Event::Dispatcher::RunType::NonBlock); } } - ENVOY_LOG(trace, "FakeStream done waiting for end stream"); + return AssertionSuccess(); } -void FakeStream::waitForReset() { +AssertionResult FakeStream::waitForReset(milliseconds timeout) { Thread::LockGuard lock(lock_); + auto start_time = std::chrono::steady_clock::now(); while (!saw_reset_) { - decoder_event_.wait(lock_); // Safe since CondVar::wait won't throw. + if (std::chrono::steady_clock::now() >= start_time + timeout) { + return AssertionFailure() << "Timed out waiting for reset."; + } + // Safe since CondVar::waitFor won't throw. + decoder_event_.waitFor(lock_, 5ms); } + return AssertionSuccess(); } void FakeStream::startGrpcStream() { @@ -163,20 +193,23 @@ FakeHttpConnection::FakeHttpConnection(SharedConnectionWrapper& shared_connectio Network::ReadFilterSharedPtr{new ReadFilter(*this)}); } -void FakeConnectionBase::close() { - shared_connection_.executeOnDispatcher([](Network::Connection& connection) { - connection.close(Network::ConnectionCloseType::FlushWrite); - }); +AssertionResult FakeConnectionBase::close(std::chrono::milliseconds timeout) { + return shared_connection_.executeOnDispatcher( + [](Network::Connection& connection) { + connection.close(Network::ConnectionCloseType::FlushWrite); + }, + timeout); } -void FakeConnectionBase::readDisable(bool disable) { - shared_connection_.executeOnDispatcher( - [disable](Network::Connection& connection) { connection.readDisable(disable); }); +AssertionResult FakeConnectionBase::readDisable(bool disable, std::chrono::milliseconds timeout) { + return shared_connection_.executeOnDispatcher( + [disable](Network::Connection& connection) { connection.readDisable(disable); }, timeout); } -void FakeConnectionBase::enableHalfClose(bool enable) { - shared_connection_.executeOnDispatcher( - [enable](Network::Connection& connection) { connection.enableHalfClose(enable); }); +AssertionResult FakeConnectionBase::enableHalfClose(bool enable, + std::chrono::milliseconds timeout) { + return shared_connection_.executeOnDispatcher( + [enable](Network::Connection& connection) { connection.enableHalfClose(enable); }, timeout); } Http::StreamDecoder& FakeHttpConnection::newStream(Http::StreamEncoder& encoder) { @@ -186,29 +219,42 @@ Http::StreamDecoder& FakeHttpConnection::newStream(Http::StreamEncoder& encoder) return *new_streams_.back(); } -void FakeConnectionBase::waitForDisconnect(bool ignore_spurious_events) { +AssertionResult FakeConnectionBase::waitForDisconnect(bool ignore_spurious_events, + milliseconds timeout) { ENVOY_LOG(trace, "FakeConnectionBase waiting for disconnect"); + auto end_time = std::chrono::steady_clock::now() + timeout; Thread::LockGuard lock(lock_); while (shared_connection_.connected()) { - connection_event_.wait(lock_); // Safe since CondVar::wait won't throw. + if (std::chrono::steady_clock::now() >= end_time) { + return AssertionResult("Timed out waiting for disconnect."); + } + Thread::CondVar::WaitStatus status = connection_event_.waitFor(lock_, 5ms); // The default behavior of waitForDisconnect is to assume the test cleanly // calls waitForData, waitForNewStream, etc. to handle all events on the // connection. If the caller explicitly notes that other events should be // ignored, continue looping until a disconnect is detected. Otherwise fall // through and hit the assert below. - if (!ignore_spurious_events) { + if ((status == Thread::CondVar::WaitStatus::NoTimeout) && !ignore_spurious_events) { break; } } - ASSERT(!shared_connection_.connected()); + if (shared_connection_.connected()) { + return AssertionFailure() << "Expected disconnect, but got a different event."; + } ENVOY_LOG(trace, "FakeConnectionBase done waiting for disconnect"); + return AssertionSuccess(); } -void FakeConnectionBase::waitForHalfClose(bool ignore_spurious_events) { +AssertionResult FakeConnectionBase::waitForHalfClose(bool ignore_spurious_events, + milliseconds timeout) { + auto end_time = std::chrono::steady_clock::now() + timeout; Thread::LockGuard lock(lock_); while (!half_closed_) { - connection_event_.wait(lock_); // Safe since CondVar::wait won't throw. + if (std::chrono::steady_clock::now() >= end_time) { + return AssertionFailure() << "Timed out waiting for half close."; + } + connection_event_.waitFor(lock_, 5ms); // Safe since CondVar::waitFor won't throw. // The default behavior of waitForHalfClose is to assume the test cleanly // calls waitForData, waitForNewStream, etc. to handle all events on the // connection. If the caller explicitly notes that other events should be @@ -219,15 +265,22 @@ void FakeConnectionBase::waitForHalfClose(bool ignore_spurious_events) { } } - ASSERT(half_closed_); + return half_closed_ + ? AssertionSuccess() + : (AssertionFailure() << "Expected half close event, but got a different event."); } -FakeStreamPtr FakeHttpConnection::waitForNewStream(Event::Dispatcher& client_dispatcher, - bool ignore_spurious_events) { +AssertionResult FakeHttpConnection::waitForNewStream(Event::Dispatcher& client_dispatcher, + FakeStreamPtr* stream, + bool ignore_spurious_events, + milliseconds timeout) { + auto end_time = std::chrono::steady_clock::now() + timeout; Thread::LockGuard lock(lock_); while (new_streams_.empty()) { - Thread::CondVar::WaitStatus status = - connection_event_.waitFor(lock_, std::chrono::milliseconds(5)); + if (std::chrono::steady_clock::now() >= end_time) { + return AssertionResult("Timed out waiting for new stream."); + } + Thread::CondVar::WaitStatus status = connection_event_.waitFor(lock_, 5ms); // As with waitForDisconnect, by default, waitForNewStream returns after the next event. // If the caller explicitly notes other events should be ignored, it will instead actually // wait for the next new stream, ignoring other events such as onData() @@ -240,10 +293,12 @@ FakeStreamPtr FakeHttpConnection::waitForNewStream(Event::Dispatcher& client_dis } } - ASSERT(!new_streams_.empty()); - FakeStreamPtr stream = std::move(new_streams_.front()); + if (new_streams_.empty()) { + return AssertionFailure() << "Expected new stream event, but got a different event."; + } + *stream = std::move(new_streams_.front()); new_streams_.pop_front(); - return stream; + return AssertionSuccess(); } FakeUpstream::FakeUpstream(const std::string& uds_path, FakeHttpConnection::Type type) @@ -281,8 +336,7 @@ FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket_factory, Network::SocketPtr&& listen_socket, FakeHttpConnection::Type type, bool enable_half_close) - : http_type_(type), socket_(std::move(listen_socket)), - api_(new Api::Impl(std::chrono::milliseconds(10000))), + : http_type_(type), socket_(std::move(listen_socket)), api_(new Api::Impl(milliseconds(10000))), dispatcher_(api_->allocateDispatcher()), handler_(new Server::ConnectionHandlerImpl(ENVOY_LOGGER(), *dispatcher_)), allow_unexpected_disconnects_(false), enable_half_close_(enable_half_close), listener_(*this), @@ -327,71 +381,81 @@ void FakeUpstream::threadRoutine() { } } -FakeHttpConnectionPtr FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher) { - FakeHttpConnectionPtr connection; +AssertionResult FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher, + FakeHttpConnectionPtr* connection, + milliseconds timeout) { + auto end_time = std::chrono::steady_clock::now() + timeout; { Thread::LockGuard lock(lock_); while (new_connections_.empty()) { - new_connection_event_.waitFor(lock_, std::chrono::milliseconds(5)); + if (std::chrono::steady_clock::now() >= end_time) { + return AssertionFailure() << "Timed out waiting for new connection."; + } + new_connection_event_.waitFor(lock_, 5ms); if (new_connections_.empty()) { // Run the client dispatcher since we may need to process window updates, etc. client_dispatcher.run(Event::Dispatcher::RunType::NonBlock); } } - ASSERT(!new_connections_.empty()); - connection = + if (new_connections_.empty()) { + return AssertionFailure() << "Got a new connection event, but didn't create a connection."; + } + *connection = std::make_unique(consumeConnection(), stats_store_, http_type_); } - connection->initialize(); - connection->readDisable(false); - return connection; + (*connection)->initialize(); + VERIFY_ASSERTION((*connection)->readDisable(false)); + return AssertionSuccess(); } -FakeHttpConnectionPtr +AssertionResult FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher, - std::vector>& upstreams) { - for (;;) { + std::vector>& upstreams, + FakeHttpConnectionPtr* connection, milliseconds timeout) { + auto end_time = std::chrono::steady_clock::now() + timeout; + while (std::chrono::steady_clock::now() < end_time) { for (auto it = upstreams.begin(); it != upstreams.end(); ++it) { FakeUpstream& upstream = **it; Thread::ReleasableLockGuard lock(upstream.lock_); if (upstream.new_connections_.empty()) { - upstream.new_connection_event_.waitFor(upstream.lock_, std::chrono::milliseconds(5)); + upstream.new_connection_event_.waitFor(upstream.lock_, 5ms); } if (upstream.new_connections_.empty()) { // Run the client dispatcher since we may need to process window updates, etc. client_dispatcher.run(Event::Dispatcher::RunType::NonBlock); } else { - FakeHttpConnectionPtr connection(new FakeHttpConnection( - upstream.consumeConnection(), upstream.stats_store_, upstream.http_type_)); + *connection = std::make_unique( + upstream.consumeConnection(), upstream.stats_store_, upstream.http_type_); lock.release(); - connection->initialize(); - connection->readDisable(false); - return connection; + (*connection)->initialize(); + VERIFY_ASSERTION((*connection)->readDisable(false)); + return AssertionSuccess(); } } } + return AssertionFailure() << "Timed out waiting for HTTP connection."; } -FakeRawConnectionPtr FakeUpstream::waitForRawConnection(std::chrono::milliseconds wait_for_ms) { - FakeRawConnectionPtr connection; +AssertionResult FakeUpstream::waitForRawConnection(FakeRawConnectionPtr* connection, + milliseconds timeout) { { Thread::LockGuard lock(lock_); if (new_connections_.empty()) { ENVOY_LOG(debug, "waiting for raw connection"); - new_connection_event_.waitFor(lock_, wait_for_ms); // Safe since CondVar::wait won't throw. + new_connection_event_.waitFor(lock_, timeout); // Safe since CondVar::waitFor won't throw. } if (new_connections_.empty()) { - return nullptr; + return AssertionFailure() << "Timed out waiting for raw connection"; } - connection = std::make_unique(consumeConnection()); + *connection = std::make_unique(consumeConnection()); } - connection->initialize(); - connection->readDisable(false); - connection->enableHalfClose(enable_half_close_); - return connection; + (*connection)->initialize(); + VERIFY_ASSERTION((*connection)->readDisable(false)); + VERIFY_ASSERTION((*connection)->enableHalfClose(enable_half_close_)); + return AssertionSuccess(); } SharedConnectionWrapper& FakeUpstream::consumeConnection() { @@ -402,30 +466,49 @@ SharedConnectionWrapper& FakeUpstream::consumeConnection() { return connection_wrapper->shared_connection(); } -std::string FakeRawConnection::waitForData(uint64_t num_bytes) { +AssertionResult FakeRawConnection::waitForData(uint64_t num_bytes, std::string* data, + milliseconds timeout) { Thread::LockGuard lock(lock_); + ENVOY_LOG(debug, "waiting for {} bytes of data", num_bytes); + auto end_time = std::chrono::steady_clock::now() + timeout; while (data_.size() != num_bytes) { - ENVOY_LOG(debug, "waiting for {} bytes of data", num_bytes); - connection_event_.wait(lock_); // Safe since CondVar::wait won't throw. + if (std::chrono::steady_clock::now() >= end_time) { + return AssertionFailure() << "Timed out waiting for data."; + } + connection_event_.waitFor(lock_, 5ms); // Safe since CondVar::waitFor won't throw. + } + if (data != nullptr) { + *data = data_; } - return data_; + return AssertionSuccess(); } -std::string -FakeRawConnection::waitForData(const std::function& data_validator) { +AssertionResult +FakeRawConnection::waitForData(const std::function& data_validator, + std::string* data, milliseconds timeout) { Thread::LockGuard lock(lock_); + ENVOY_LOG(debug, "waiting for data"); + auto end_time = std::chrono::steady_clock::now() + timeout; while (!data_validator(data_)) { - ENVOY_LOG(debug, "waiting for data"); - connection_event_.wait(lock_); // Safe since CondVar::wait won't throw. + if (std::chrono::steady_clock::now() >= end_time) { + return AssertionFailure() << "Timed out waiting for data."; + } + connection_event_.waitFor(lock_, 5ms); // Safe since CondVar::waitFor won't throw. } - return data_; + if (data != nullptr) { + *data = data_; + } + return AssertionSuccess(); } -void FakeRawConnection::write(const std::string& data, bool end_stream) { - shared_connection_.executeOnDispatcher([&data, end_stream](Network::Connection& connection) { - Buffer::OwnedImpl to_write(data); - connection.write(to_write, end_stream); - }); +AssertionResult FakeRawConnection::write(const std::string& data, bool end_stream, + milliseconds timeout) { + return shared_connection_.executeOnDispatcher( + [&data, end_stream](Network::Connection& connection) { + Buffer::OwnedImpl to_write(data); + connection.write(to_write, end_stream); + }, + timeout); } Network::FilterStatus FakeRawConnection::ReadFilter::onData(Buffer::Instance& data, diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index 87bc4cfe022f0..5a079d5933a9f 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -56,10 +56,20 @@ class FakeStream : public Http::StreamDecoder, const Http::HeaderMap& headers() { return *headers_; } void setAddServedByHeader(bool add_header) { add_served_by_header_ = add_header; } const Http::HeaderMapPtr& trailers() { return trailers_; } - void waitForHeadersComplete(); - void waitForData(Event::Dispatcher& client_dispatcher, uint64_t body_length); - void waitForEndStream(Event::Dispatcher& client_dispatcher); - void waitForReset(); + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForHeadersComplete(std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForData(Event::Dispatcher& client_dispatcher, uint64_t body_length, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForEndStream(Event::Dispatcher& client_dispatcher, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForReset(std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); // gRPC convenience methods. void startGrpcStream(); @@ -81,26 +91,43 @@ class FakeStream : public Http::StreamDecoder, ENVOY_LOG(debug, "Received gRPC message: {}", message.DebugString()); decoded_grpc_frames_.erase(decoded_grpc_frames_.begin()); } - template void waitForGrpcMessage(Event::Dispatcher& client_dispatcher, T& message) { + template + ABSL_MUST_USE_RESULT testing::AssertionResult + waitForGrpcMessage(Event::Dispatcher& client_dispatcher, T& message, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)) { + auto end_time = std::chrono::steady_clock::now() + timeout; ENVOY_LOG(debug, "Waiting for gRPC message..."); if (!decoded_grpc_frames_.empty()) { decodeGrpcFrame(message); - return; + return AssertionSuccess(); + } + if (!waitForData(client_dispatcher, 5, timeout)) { + return testing::AssertionFailure() << "Timed out waiting for start of gRPC message."; } - waitForData(client_dispatcher, 5); { Thread::LockGuard lock(lock_); - EXPECT_TRUE(grpc_decoder_.decode(body(), decoded_grpc_frames_)); + if (!grpc_decoder_.decode(body(), decoded_grpc_frames_)) { + return testing::AssertionFailure() + << "Couldn't decode gRPC data frame: " << body().toString(); + } } if (decoded_grpc_frames_.size() < 1) { - waitForData(client_dispatcher, grpc_decoder_.length()); + timeout = std::chrono::duration_cast( + end_time - std::chrono::steady_clock::now()); + if (!waitForData(client_dispatcher, grpc_decoder_.length(), timeout)) { + return testing::AssertionFailure() << "Timed out waiting for end of gRPC message."; + } { Thread::LockGuard lock(lock_); - EXPECT_TRUE(grpc_decoder_.decode(body(), decoded_grpc_frames_)); + if (!grpc_decoder_.decode(body(), decoded_grpc_frames_)) { + return testing::AssertionFailure() + << "Couldn't decode gRPC data frame: " << body().toString(); + } } } decodeGrpcFrame(message); ENVOY_LOG(debug, "Received gRPC message: {}", message.DebugString()); + return AssertionSuccess(); } // Http::StreamDecoder @@ -196,29 +223,41 @@ class SharedConnectionWrapper : public Network::ConnectionCallbacks { // wait-for-completion. If the connection is disconnected, either prior to post or when the // dispatcher schedules the callback, we silently ignore if allow_unexpected_disconnects_ // is set. - void executeOnDispatcher(std::function f) { + ABSL_MUST_USE_RESULT + testing::AssertionResult + executeOnDispatcher(std::function f, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)) { Thread::LockGuard lock(lock_); if (disconnected_) { - return; + return testing::AssertionSuccess(); } Thread::CondVar callback_ready_event; - connection_.dispatcher().post([this, f, &callback_ready_event]() -> void { - // The use of connected() here, vs. !disconnected_, is because we want to use the lock_ - // acquisition to briefly serialize. This avoids us entering this completion and issuing a - // notifyOne() until the wait() is ready to receive it below. - if (connected()) { - f(connection_); - } else { - RELEASE_ASSERT( - allow_unexpected_disconnects_, - "The connection disconnected unexpectedly, and allow_unexpected_disconnects_ is false." - "\n See " - "https://github.com/envoyproxy/envoy/blob/master/test/integration/README.md#" - "unexpected-disconnects"); - } - callback_ready_event.notifyOne(); - }); - callback_ready_event.wait(lock_); + bool unexpected_disconnect = false; + connection_.dispatcher().post( + [this, f, &callback_ready_event, &unexpected_disconnect]() -> void { + // The use of connected() here, vs. !disconnected_, is because we want to use the lock_ + // acquisition to briefly serialize. This avoids us entering this completion and issuing a + // notifyOne() until the wait() is ready to receive it below. + if (connected()) { + f(connection_); + } else { + unexpected_disconnect = true; + } + callback_ready_event.notifyOne(); + }); + Thread::CondVar::WaitStatus status = callback_ready_event.waitFor(lock_, timeout); + if (status == Thread::CondVar::WaitStatus::Timeout) { + return testing::AssertionFailure() << "Timed out while executing on dispatcher."; + } + if (unexpected_disconnect && !allow_unexpected_disconnects_) { + return testing::AssertionFailure() + << "The connection disconnected unexpectedly, and allow_unexpected_disconnects_ is " + "false." + "\n See " + "https://github.com/envoyproxy/envoy/blob/master/test/integration/README.md#" + "unexpected-disconnects"; + } + return testing::AssertionSuccess(); } private: @@ -284,20 +323,36 @@ class FakeConnectionBase : public Logger::Loggable { ASSERT(disconnect_callback_handle_ != nullptr); shared_connection_.removeDisconnectCallback(disconnect_callback_handle_); } - void close(); - void readDisable(bool disable); - // By default waitForDisconnect and waitForHalfClose assume the next event is a disconnect and - // fails an assert if an unexpected event occurs. If a caller truly wishes to wait until - // disconnect, set ignore_spurious_events = true. - void waitForDisconnect(bool ignore_spurious_events = false); - void waitForHalfClose(bool ignore_spurious_events = false); - - virtual void initialize() { + ABSL_MUST_USE_RESULT + testing::AssertionResult + close(std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + ABSL_MUST_USE_RESULT + testing::AssertionResult + readDisable(bool disable, std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + // By default waitForDisconnect and waitForHalfClose assume the next event is + // a disconnect and return an AssertionFailure if an unexpected event occurs. + // If a caller truly wishes to wait until disconnect, set + // ignore_spurious_events = true. + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForDisconnect(bool ignore_spurious_events = false, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForHalfClose(bool ignore_spurious_events = false, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + + ABSL_MUST_USE_RESULT + virtual testing::AssertionResult initialize() { initialized_ = true; disconnect_callback_handle_ = shared_connection_.addDisconnectCallback([this] { connection_event_.notifyOne(); }); + return testing::AssertionSuccess(); } - void enableHalfClose(bool enabled); + ABSL_MUST_USE_RESULT + testing::AssertionResult + enableHalfClose(bool enabled, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); SharedConnectionWrapper& shared_connection() { return shared_connection_; } // The same caveats apply here as in SharedConnectionWrapper::connection(). Network::Connection& connection() const { return shared_connection_.connection(); } @@ -324,10 +379,13 @@ class FakeHttpConnection : public Http::ServerConnectionCallbacks, public FakeCo FakeHttpConnection(SharedConnectionWrapper& shared_connection, Stats::Store& store, Type type); // By default waitForNewStream assumes the next event is a new stream and - // fails an assert if an unexpected event occurs. If a caller truly wishes to - // wait for a new stream, set ignore_spurious_events = true. - FakeStreamPtr waitForNewStream(Event::Dispatcher& client_dispatcher, - bool ignore_spurious_events = false); + // returns AssertionFaliure if an unexpected event occurs. If a caller truly + // wishes to wait for a new stream, set ignore_spurious_events = true. + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForNewStream(Event::Dispatcher& client_dispatcher, FakeStreamPtr* stream, + bool ignore_spurious_events = false, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); // Http::ServerConnectionCallbacks Http::StreamDecoder& newStream(Http::StreamEncoder& response_encoder) override; @@ -361,17 +419,34 @@ class FakeRawConnection : public FakeConnectionBase { : FakeConnectionBase(shared_connection) {} typedef const std::function ValidatorFunction; - std::string waitForData(uint64_t num_bytes); + // Writes to data. If data is nullptr, discards the received data. + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForData(uint64_t num_bytes, std::string* data = nullptr, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); // Wait until data_validator returns true. - // example usage: waitForData(FakeRawConnection::waitForInexactMatch("foo")); - std::string waitForData(const ValidatorFunction& data_validator); - void write(const std::string& data, bool end_stream = false); - - void initialize() override { - shared_connection_.executeOnDispatcher([this](Network::Connection& connection) { - connection.addReadFilter(Network::ReadFilterSharedPtr{new ReadFilter(*this)}); - }); - FakeConnectionBase::initialize(); + // example usage: + // std::string data; + // ASSERT_TRUE(waitForData(FakeRawConnection::waitForInexactMatch("foo"), &data)); + // EXPECT_EQ(data, "foobar"); + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForData(const ValidatorFunction& data_validator, std::string* data = nullptr, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + ABSL_MUST_USE_RESULT + testing::AssertionResult + write(const std::string& data, bool end_stream = false, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + + testing::AssertionResult initialize() override { + testing::AssertionResult result = + shared_connection_.executeOnDispatcher([this](Network::Connection& connection) { + connection.addReadFilter(Network::ReadFilterSharedPtr{new ReadFilter(*this)}); + }); + if (!result) { + return result; + } + return FakeConnectionBase::initialize(); } // Creates a ValidatorFunction which returns true when data_to_wait_for is @@ -413,15 +488,23 @@ class FakeUpstream : Logger::Loggable, ~FakeUpstream(); FakeHttpConnection::Type httpType() { return http_type_; } - FakeHttpConnectionPtr waitForHttpConnection(Event::Dispatcher& client_dispatcher); - FakeRawConnectionPtr - waitForRawConnection(std::chrono::milliseconds wait_for_ms = std::chrono::milliseconds{10000}); + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForHttpConnection(Event::Dispatcher& client_dispatcher, FakeHttpConnectionPtr* connection, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + ABSL_MUST_USE_RESULT + testing::AssertionResult + waitForRawConnection(FakeRawConnectionPtr* connection, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); Network::Address::InstanceConstSharedPtr localAddress() const { return socket_->localAddress(); } // Wait for one of the upstreams to receive a connection - static FakeHttpConnectionPtr + ABSL_MUST_USE_RESULT + static testing::AssertionResult waitForHttpConnection(Event::Dispatcher& client_dispatcher, - std::vector>& upstreams); + std::vector>& upstreams, + FakeHttpConnectionPtr* connection, + std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); // Network::FilterChainManager const Network::FilterChain* findFilterChain(const Network::ConnectionSocket&) const override { diff --git a/test/integration/h1_capture_fuzz_test.cc b/test/integration/h1_capture_fuzz_test.cc index 2d8b5168ed6ab..4de4e3de70735 100644 --- a/test/integration/h1_capture_fuzz_test.cc +++ b/test/integration/h1_capture_fuzz_test.cc @@ -37,9 +37,8 @@ class H1FuzzIntegrationTest : public HttpIntegrationTest { break; case test::integration::Event::kUpstreamSendBytes: if (fake_upstream_connection == nullptr) { - fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(max_wait_ms_); - // If we timed out, we fail out. - if (fake_upstream_connection == nullptr) { + if (!fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection, max_wait_ms_)) { + // If we timed out, we fail out. tcp_client->close(); return; } @@ -49,7 +48,7 @@ class H1FuzzIntegrationTest : public HttpIntegrationTest { tcp_client->close(); return; } - fake_upstream_connection->write(event.upstream_send_bytes()); + ASSERT(fake_upstream_connection->write(event.upstream_send_bytes())); break; case test::integration::Event::kUpstreamRecvBytes: // TODO(htuch): Should we wait for some data? @@ -61,9 +60,9 @@ class H1FuzzIntegrationTest : public HttpIntegrationTest { } if (fake_upstream_connection != nullptr) { if (fake_upstream_connection->connected()) { - fake_upstream_connection->close(); + ASSERT(fake_upstream_connection->close()); } - fake_upstream_connection->waitForDisconnect(true); + ASSERT(fake_upstream_connection->waitForDisconnect(true)); } tcp_client->close(); } diff --git a/test/integration/hds_integration_test.cc b/test/integration/hds_integration_test.cc index e81322cbb9f63..8e8f435d291ac 100644 --- a/test/integration/hds_integration_test.cc +++ b/test/integration/hds_integration_test.cc @@ -53,8 +53,8 @@ class HdsIntegrationTest : public HttpIntegrationTest, } void waitForHdsStream() { - fake_hds_connection_ = hds_upstream_->waitForHttpConnection(*dispatcher_); - hds_stream_ = fake_hds_connection_->waitForNewStream(*dispatcher_); + ASSERT(hds_upstream_->waitForHttpConnection(*dispatcher_, &fake_hds_connection_)); + ASSERT(fake_hds_connection_->waitForNewStream(*dispatcher_, &hds_stream_)); } void requestHealthCheckSpecifier() { @@ -68,8 +68,8 @@ class HdsIntegrationTest : public HttpIntegrationTest, void cleanupHdsConnection() { if (fake_hds_connection_ != nullptr) { - fake_hds_connection_->close(); - fake_hds_connection_->waitForDisconnect(); + ASSERT(fake_hds_connection_->close()); + ASSERT(fake_hds_connection_->waitForDisconnect()); } } @@ -98,9 +98,9 @@ TEST_P(HdsIntegrationTest, Simple) { server_health_check_specifier.mutable_interval()->set_nanos(500000000); // 500ms // Server <--> Envoy - fake_hds_connection_ = hds_upstream_->waitForHttpConnection(*dispatcher_); - hds_stream_ = fake_hds_connection_->waitForNewStream(*dispatcher_); - hds_stream_->waitForGrpcMessage(*dispatcher_, envoy_msg); + ASSERT_TRUE(hds_upstream_->waitForHttpConnection(*dispatcher_, &fake_hds_connection_)); + ASSERT_TRUE(fake_hds_connection_->waitForNewStream(*dispatcher_, &hds_stream_)); + ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, envoy_msg)); EXPECT_EQ(0, test_server_->counter("hds_delegate.requests")->value()); EXPECT_EQ(1, test_server_->counter("hds_delegate.responses")->value()); @@ -111,7 +111,7 @@ TEST_P(HdsIntegrationTest, Simple) { test_server_->waitForCounterGe("hds_delegate.requests", ++hds_requests_); // Wait for Envoy to reply - hds_stream_->waitForGrpcMessage(*dispatcher_, envoy_msg_2); + ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, envoy_msg_2)); EXPECT_EQ(1, test_server_->counter("hds_delegate.requests")->value()); EXPECT_EQ(2, test_server_->counter("hds_delegate.responses")->value()); diff --git a/test/integration/header_integration_test.cc b/test/integration/header_integration_test.cc index 33d8e5af21501..2e50e8fecff15 100644 --- a/test/integration/header_integration_test.cc +++ b/test/integration/header_integration_test.cc @@ -142,8 +142,8 @@ class HeaderIntegrationTest if (eds_connection_ != nullptr) { // Don't ASSERT fail if an EDS reconnect ends up unparented. fake_upstreams_[1]->set_allow_unexpected_disconnects(true); - eds_connection_->close(); - eds_connection_->waitForDisconnect(); + ASSERT(eds_connection_->close()); + ASSERT(eds_connection_->waitForDisconnect()); eds_connection_.reset(); } cleanupUpstreamAndDownstream(); @@ -309,12 +309,12 @@ class HeaderIntegrationTest void initialize() override { if (use_eds_) { pre_worker_start_test_steps_ = [this]() { - eds_connection_ = fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); - eds_stream_ = eds_connection_->waitForNewStream(*dispatcher_); + ASSERT(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &eds_connection_)); + ASSERT(eds_connection_->waitForNewStream(*dispatcher_, &eds_stream_)); eds_stream_->startGrpcStream(); envoy::api::v2::DiscoveryRequest discovery_request; - eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request); + ASSERT(eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); envoy::api::v2::DiscoveryResponse discovery_response; discovery_response.set_version_info("1"); @@ -343,7 +343,7 @@ class HeaderIntegrationTest eds_stream_->sendGrpcMessage(discovery_response); // Wait for the next request to make sure the first response was consumed. - eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request); + ASSERT(eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); }; } diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index ae04ba0db8291..394223e2e5ad3 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -253,8 +253,8 @@ TEST_P(Http2IntegrationTest, IdleTimeoutWithSimultaneousRequests) { encoder1 = &encoder_decoder.first; auto response1 = std::move(encoder_decoder.second); - fake_upstream_connection1 = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request1 = fake_upstream_connection1->waitForNewStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection1)); + ASSERT_TRUE(fake_upstream_connection1->waitForNewStream(*dispatcher_, &upstream_request1)); // Start request 2 auto encoder_decoder2 = @@ -264,16 +264,16 @@ TEST_P(Http2IntegrationTest, IdleTimeoutWithSimultaneousRequests) { {":authority", "host"}}); encoder2 = &encoder_decoder2.first; auto response2 = std::move(encoder_decoder2.second); - fake_upstream_connection2 = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request2 = fake_upstream_connection2->waitForNewStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection2)); + ASSERT_TRUE(fake_upstream_connection2->waitForNewStream(*dispatcher_, &upstream_request2)); // Finish request 1 codec_client_->sendData(*encoder1, request1_bytes, true); - upstream_request1->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request1->waitForEndStream(*dispatcher_)); // Finish request i2 codec_client_->sendData(*encoder2, request2_bytes, true); - upstream_request2->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request2->waitForEndStream(*dispatcher_)); // Respond to request 2 upstream_request2->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); @@ -300,8 +300,8 @@ TEST_P(Http2IntegrationTest, IdleTimeoutWithSimultaneousRequests) { EXPECT_EQ(request1_bytes, response1->body().size()); // Do not send any requests and validate idle timeout kicks in after both the requests are done. - fake_upstream_connection1->waitForDisconnect(); - fake_upstream_connection2->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection1->waitForDisconnect()); + ASSERT_TRUE(fake_upstream_connection2->waitForDisconnect()); test_server_->waitForCounterGe("cluster.cluster_0.upstream_cx_idle_timeout", 2); } @@ -325,8 +325,8 @@ void Http2IntegrationTest::simultaneousRequest(int32_t request1_bytes, int32_t r encoder1 = &encoder_decoder.first; auto response1 = std::move(encoder_decoder.second); - fake_upstream_connection1 = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request1 = fake_upstream_connection1->waitForNewStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection1)); + ASSERT_TRUE(fake_upstream_connection1->waitForNewStream(*dispatcher_, &upstream_request1)); // Start request 2 auto encoder_decoder2 = @@ -336,16 +336,16 @@ void Http2IntegrationTest::simultaneousRequest(int32_t request1_bytes, int32_t r {":authority", "host"}}); encoder2 = &encoder_decoder2.first; auto response2 = std::move(encoder_decoder2.second); - fake_upstream_connection2 = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request2 = fake_upstream_connection2->waitForNewStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection2)); + ASSERT_TRUE(fake_upstream_connection2->waitForNewStream(*dispatcher_, &upstream_request2)); // Finish request 1 codec_client_->sendData(*encoder1, request1_bytes, true); - upstream_request1->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request1->waitForEndStream(*dispatcher_)); // Finish request 2 codec_client_->sendData(*encoder2, request2_bytes, true); - upstream_request2->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request2->waitForEndStream(*dispatcher_)); // Respond to request 2 upstream_request2->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); @@ -368,10 +368,10 @@ void Http2IntegrationTest::simultaneousRequest(int32_t request1_bytes, int32_t r EXPECT_EQ(request2_bytes, response1->body().size()); // Cleanup both downstream and upstream - fake_upstream_connection1->close(); - fake_upstream_connection1->waitForDisconnect(); - fake_upstream_connection2->close(); - fake_upstream_connection2->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection1->close()); + ASSERT_TRUE(fake_upstream_connection1->waitForDisconnect()); + ASSERT_TRUE(fake_upstream_connection2->close()); + ASSERT_TRUE(fake_upstream_connection2->waitForDisconnect()); codec_client_->close(); } @@ -400,8 +400,8 @@ Http2RingHashIntegrationTest::~Http2RingHashIntegrationTest() { codec_client_ = nullptr; } for (auto it = fake_upstream_connections_.begin(); it != fake_upstream_connections_.end(); ++it) { - (*it)->close(); - (*it)->waitForDisconnect(); + ASSERT((*it)->close()); + ASSERT((*it)->waitForDisconnect()); } } @@ -435,17 +435,20 @@ void Http2RingHashIntegrationTest::sendMultipleRequests( } for (uint32_t i = 0; i < num_requests; ++i) { - auto fake_upstream_connection = - FakeUpstream::waitForHttpConnection(*dispatcher_, fake_upstreams_); + FakeHttpConnectionPtr fake_upstream_connection; + ASSERT_TRUE(FakeUpstream::waitForHttpConnection(*dispatcher_, fake_upstreams_, + &fake_upstream_connection)); // As data and streams are interwoven, make sure waitForNewStream() // ignores incoming data and waits for actual stream establishment. - upstream_requests.push_back(fake_upstream_connection->waitForNewStream(*dispatcher_, true)); + upstream_requests.emplace_back(); + ASSERT_TRUE( + fake_upstream_connection->waitForNewStream(*dispatcher_, &upstream_requests.back(), true)); upstream_requests.back()->setAddServedByHeader(true); fake_upstream_connections_.push_back(std::move(fake_upstream_connection)); } for (uint32_t i = 0; i < num_requests; ++i) { - upstream_requests[i]->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_requests[i]->waitForEndStream(*dispatcher_)); upstream_requests[i]->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); upstream_requests[i]->encodeData(rand.random() % (1024 * 2), true); } diff --git a/test/integration/http2_upstream_integration_test.cc b/test/integration/http2_upstream_integration_test.cc index 2d4b3be30a52e..2445c9fa04bae 100644 --- a/test/integration/http2_upstream_integration_test.cc +++ b/test/integration/http2_upstream_integration_test.cc @@ -97,12 +97,12 @@ void Http2UpstreamIntegrationTest::bidirectionalStreaming(uint32_t bytes) { {":authority", "host"}}); auto response = std::move(encoder_decoder.second); request_encoder_ = &encoder_decoder.first; - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); // Send part of the request body and ensure it is received upstream. codec_client_->sendData(*request_encoder_, bytes, false); - upstream_request_->waitForData(*dispatcher_, bytes); + ASSERT_TRUE(upstream_request_->waitForData(*dispatcher_, bytes)); // Start sending the response and ensure it is received downstream. upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); @@ -111,7 +111,7 @@ void Http2UpstreamIntegrationTest::bidirectionalStreaming(uint32_t bytes) { // Finish the request. codec_client_->sendTrailers(*request_encoder_, Http::TestHeaderMapImpl{{"trailer", "foo"}}); - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); // Finish the response. upstream_request_->encodeTrailers(Http::TestHeaderMapImpl{{"trailer", "bar"}}); @@ -138,12 +138,12 @@ TEST_P(Http2UpstreamIntegrationTest, BidirectionalStreamingReset) { {":authority", "host"}}); auto response = std::move(encoder_decoder.second); request_encoder_ = &encoder_decoder.first; - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); // Send some request data. codec_client_->sendData(*request_encoder_, 1024, false); - upstream_request_->waitForData(*dispatcher_, 1024); + ASSERT_TRUE(upstream_request_->waitForData(*dispatcher_, 1024)); // Start sending the response. upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); @@ -152,7 +152,7 @@ TEST_P(Http2UpstreamIntegrationTest, BidirectionalStreamingReset) { // Finish sending therequest. codec_client_->sendTrailers(*request_encoder_, Http::TestHeaderMapImpl{{"trailer", "foo"}}); - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); // Reset the stream. upstream_request_->encodeResetStream(); @@ -177,8 +177,8 @@ void Http2UpstreamIntegrationTest::simultaneousRequest(uint32_t request1_bytes, {":authority", "host"}}); Http::StreamEncoder* encoder1 = &encoder_decoder1.first; auto response1 = std::move(encoder_decoder1.second); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request1 = fake_upstream_connection_->waitForNewStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request1)); // Start request 2 auto encoder_decoder2 = @@ -188,15 +188,17 @@ void Http2UpstreamIntegrationTest::simultaneousRequest(uint32_t request1_bytes, {":authority", "host"}}); Http::StreamEncoder* encoder2 = &encoder_decoder2.first; auto response2 = std::move(encoder_decoder2.second); - upstream_request2 = fake_upstream_connection_->waitForNewStream(*dispatcher_); + // DO NOT SUBMIT replace other ASSERT_TRUE with ASSERT (?) + // (in places like this, ASSERT_TRUE is probably fine) + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request2)); // Finish request 1 codec_client_->sendData(*encoder1, request1_bytes, true); - upstream_request1->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request1->waitForEndStream(*dispatcher_)); // Finish request 2 codec_client_->sendData(*encoder2, request2_bytes, true); - upstream_request2->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request2->waitForEndStream(*dispatcher_)); // Respond to request 2 upstream_request2->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); @@ -302,20 +304,23 @@ TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) { codec_client_->sendData(*encoders[i], 0, true); } } - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); for (uint32_t i = 0; i < num_requests; ++i) { - upstream_requests.push_back(fake_upstream_connection_->waitForNewStream(*dispatcher_)); + FakeStreamPtr stream; + upstream_requests.emplace_back(); + ASSERT_TRUE( + fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_requests.back())); } for (uint32_t i = 0; i < num_requests; ++i) { if (i % 15 != 0) { - upstream_requests[i]->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_requests[i]->waitForEndStream(*dispatcher_)); upstream_requests[i]->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); upstream_requests[i]->encodeData(100, false); } } // Close the connection. - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); // Ensure the streams are all reset successfully. for (uint32_t i = 0; i < num_requests; ++i) { if (i % 15 != 0) { diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 5c3204dbdf8dc..c16f469e44995 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -220,8 +220,8 @@ void HttpIntegrationTest::cleanupUpstreamAndDownstream() { // will interpret that as an unexpected disconnect. The codec client is not // subject to the same failure mode. if (fake_upstream_connection_) { - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT(fake_upstream_connection_->close()); + ASSERT(fake_upstream_connection_->waitForDisconnect()); } if (codec_client_) { codec_client_->close(); @@ -231,13 +231,13 @@ void HttpIntegrationTest::cleanupUpstreamAndDownstream() { void HttpIntegrationTest::waitForNextUpstreamRequest(uint64_t upstream_index) { // If there is no upstream connection, wait for it to be established. if (!fake_upstream_connection_) { - fake_upstream_connection_ = - fake_upstreams_[upstream_index]->waitForHttpConnection(*dispatcher_); + ASSERT(fake_upstreams_[upstream_index]->waitForHttpConnection(*dispatcher_, + &fake_upstream_connection_)); } // Wait for the next stream on the upstream connection. - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); + ASSERT(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); // Wait for the stream to be completely received. - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT(upstream_request_->waitForEndStream(*dispatcher_)); } void HttpIntegrationTest::testRouterRequestAndResponseWithBody( @@ -450,12 +450,12 @@ void HttpIntegrationTest::testRouterUpstreamDisconnectBeforeRequestComplete() { {":authority", "host"}}); auto response = std::move(encoder_decoder.second); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); - upstream_request_->waitForHeadersComplete(); - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); response->waitForEndStream(); if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { @@ -484,8 +484,8 @@ void HttpIntegrationTest::testRouterUpstreamDisconnectBeforeResponseComplete( {":authority", "host"}}); waitForNextUpstreamRequest(); upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { codec_client_->waitForDisconnect(); @@ -514,17 +514,17 @@ void HttpIntegrationTest::testRouterDownstreamDisconnectBeforeRequestComplete( {":scheme", "http"}, {":authority", "host"}}); auto response = std::move(encoder_decoder.second); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); - upstream_request_->waitForHeadersComplete(); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); codec_client_->close(); if (upstreamProtocol() == FakeHttpConnection::Type::HTTP1) { - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); } else { - upstream_request_->waitForReset(); - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(upstream_request_->waitForReset()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); } EXPECT_FALSE(upstream_request_->complete()); @@ -550,11 +550,11 @@ void HttpIntegrationTest::testRouterDownstreamDisconnectBeforeResponseComplete( codec_client_->close(); if (upstreamProtocol() == FakeHttpConnection::Type::HTTP1) { - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); } else { - upstream_request_->waitForReset(); - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(upstream_request_->waitForReset()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); } EXPECT_TRUE(upstream_request_->complete()); @@ -574,19 +574,19 @@ void HttpIntegrationTest::testRouterUpstreamResponseBeforeRequestComplete() { {":scheme", "http"}, {":authority", "host"}}); auto response = std::move(encoder_decoder.second); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); - upstream_request_->waitForHeadersComplete(); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); upstream_request_->encodeData(512, true); response->waitForEndStream(); if (upstreamProtocol() == FakeHttpConnection::Type::HTTP1) { - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); } else { - upstream_request_->waitForReset(); - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(upstream_request_->waitForReset()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); } if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { @@ -618,10 +618,11 @@ void HttpIntegrationTest::testRetry() { upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, false); if (fake_upstreams_[0]->httpType() == FakeHttpConnection::Type::HTTP1) { - fake_upstream_connection_->waitForDisconnect(); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + ASSERT_TRUE( + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); } else { - upstream_request_->waitForReset(); + ASSERT_TRUE(upstream_request_->waitForReset()); } waitForNextUpstreamRequest(); upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); @@ -669,10 +670,11 @@ void HttpIntegrationTest::testGrpcRetry() { upstream_request_->encodeHeaders( Http::TestHeaderMapImpl{{":status", "200"}, {"grpc-status", "1"}}, false); if (fake_upstreams_[0]->httpType() == FakeHttpConnection::Type::HTTP1) { - fake_upstream_connection_->waitForDisconnect(); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + ASSERT_TRUE( + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); } else { - upstream_request_->waitForReset(); + ASSERT_TRUE(upstream_request_->waitForReset()); } waitForNextUpstreamRequest(); @@ -780,7 +782,7 @@ void HttpIntegrationTest::testHittingEncoderFilterLimit() { // be buffered, the stream will be reset, and the connection will disconnect. fake_upstreams_[0]->set_allow_unexpected_disconnects(true); upstream_request_->encodeData(1024 * 65, false); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); response->waitForEndStream(); EXPECT_TRUE(response->complete()); @@ -800,14 +802,14 @@ void HttpIntegrationTest::testEnvoyHandling100Continue(bool additional_continue_ {"expect", "100-continue"}}); request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); // The continue headers should arrive immediately. response->waitForContinueHeaders(); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); // Send the rest of the request. codec_client_->sendData(*request_encoder_, 10, true); - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); // Verify the Expect header is stripped. EXPECT_EQ(nullptr, upstream_request_->headers().get(Http::Headers::get().Expect)); if (via.empty()) { @@ -872,8 +874,8 @@ void HttpIntegrationTest::testEnvoyProxying100Continue(bool continue_before_upst auto response = std::move(encoder_decoder.second); // Wait for the request headers to be received upstream. - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); if (continue_before_upstream_complete) { // This case tests sending on 100-Continue headers before the client has sent all the @@ -883,7 +885,7 @@ void HttpIntegrationTest::testEnvoyProxying100Continue(bool continue_before_upst } // Send all of the request data and wait for it to be received upstream. codec_client_->sendData(*request_encoder_, 10, true); - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); if (!continue_before_upstream_complete) { // This case tests forwarding 100-Continue after the client has sent all data. @@ -931,7 +933,7 @@ void HttpIntegrationTest::testIdleTimeoutBasic() { test_server_->waitForCounterGe("cluster.cluster_0.upstream_rq_200", 1); // Do not send any requests and validate if idle time out kicks in. - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); test_server_->waitForCounterGe("cluster.cluster_0.upstream_cx_idle_timeout", 1); } @@ -985,7 +987,7 @@ void HttpIntegrationTest::testIdleTimeoutWithTwoRequests() { test_server_->waitForCounterGe("cluster.cluster_0.upstream_rq_200", 2); // Do not send any requests and validate if idle time out kicks in. - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); test_server_->waitForCounterGe("cluster.cluster_0.upstream_cx_idle_timeout", 1); } @@ -1016,7 +1018,7 @@ void HttpIntegrationTest::testUpstreamDisconnectWithTwoRequests() { // Response 1. upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); upstream_request_->encodeData(512, true); - fake_upstream_connection_->close(); + ASSERT_TRUE(fake_upstream_connection_->close()); response->waitForEndStream(); EXPECT_TRUE(upstream_request_->complete()); @@ -1026,7 +1028,7 @@ void HttpIntegrationTest::testUpstreamDisconnectWithTwoRequests() { test_server_->waitForCounterGe("cluster.cluster_0.upstream_rq_200", 1); // Response 2. - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); fake_upstream_connection_.reset(); waitForNextUpstreamRequest(); upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); @@ -1405,12 +1407,14 @@ void HttpIntegrationTest::testUpstreamProtocolError() { {":method", "GET"}, {":path", "/test/long/url"}, {":authority", "host"}}); auto response = std::move(encoder_decoder.second); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); // TODO(mattklein123): Waiting for exact amount of data is a hack. This needs to // be fixed. - fake_upstream_connection->waitForData(187); - fake_upstream_connection->write("bad protocol data!"); - fake_upstream_connection->waitForDisconnect(); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(187, &data)); + ASSERT_TRUE(fake_upstream_connection->write("bad protocol data!")); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); codec_client_->waitForDisconnect(); EXPECT_TRUE(response->complete()); @@ -1442,11 +1446,11 @@ void HttpIntegrationTest::testDownstreamResetBeforeResponseComplete() { codec_client_->sendReset(*request_encoder_); if (upstreamProtocol() == FakeHttpConnection::Type::HTTP1) { - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); } else { - upstream_request_->waitForReset(); - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(upstream_request_->waitForReset()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); } codec_client_->close(); diff --git a/test/integration/idle_timeout_integration_test.cc b/test/integration/idle_timeout_integration_test.cc index 6aa5fa3b82cdf..9a62cb46ac221 100644 --- a/test/integration/idle_timeout_integration_test.cc +++ b/test/integration/idle_timeout_integration_test.cc @@ -37,9 +37,9 @@ class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { {":authority", "host"}}); request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); - upstream_request_->waitForHeadersComplete(); + ASSERT(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT(upstream_request_->waitForHeadersComplete()); return response; } diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index e4fa9feb4ae38..163cc3f894df2 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -159,7 +159,7 @@ TEST_P(IntegrationTest, HittingGrpcFilterLimitBufferingHeaders) { upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); fake_upstreams_[0]->set_allow_unexpected_disconnects(true); upstream_request_->encodeData(1024 * 65, false); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); response->waitForEndStream(); EXPECT_TRUE(response->complete()); @@ -251,10 +251,14 @@ TEST_P(IntegrationTest, TestHeadWithExplicitTE) { auto tcp_client = makeTcpConnection(lookupPort("http")); tcp_client->write("HEAD / HTTP/1.1\r\nHost: host\r\n\r\n"); - auto fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("\r\n\r\n")); - - fake_upstream_connection->write("HTTP/1.1 200 OK\r\nTransfer-encoding: chunked\r\n\r\n"); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData( + FakeRawConnection::waitForInexactMatch("\r\n\r\n"), &data)); + + ASSERT_TRUE( + fake_upstream_connection->write("HTTP/1.1 200 OK\r\nTransfer-encoding: chunked\r\n\r\n")); tcp_client->waitForData("\r\n\r\n", false); std::string response = tcp_client->data(); @@ -263,8 +267,8 @@ TEST_P(IntegrationTest, TestHeadWithExplicitTE) { EXPECT_THAT(response, HasSubstr("transfer-encoding: chunked\r\n")); EXPECT_THAT(response, EndsWith("\r\n\r\n")); - fake_upstream_connection->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->close()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); tcp_client->close(); } @@ -286,12 +290,14 @@ TEST_P(IntegrationTest, TestBind) { {":scheme", "http"}, {":authority", "host"}}, 1024); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_NE(fake_upstream_connection_, nullptr); std::string address = fake_upstream_connection_->connection().remoteAddress()->ip()->addressAsString(); EXPECT_EQ(address, address_string); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_NE(upstream_request_, nullptr); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); cleanupUpstreamAndDownstream(); } diff --git a/test/integration/load_stats_integration_test.cc b/test/integration/load_stats_integration_test.cc index b451bafcce20c..9829af77c8fb9 100644 --- a/test/integration/load_stats_integration_test.cc +++ b/test/integration/load_stats_integration_test.cc @@ -147,8 +147,8 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, } void waitForLoadStatsStream() { - fake_loadstats_connection_ = load_report_upstream_->waitForHttpConnection(*dispatcher_); - loadstats_stream_ = fake_loadstats_connection_->waitForNewStream(*dispatcher_); + ASSERT(load_report_upstream_->waitForHttpConnection(*dispatcher_, &fake_loadstats_connection_)); + ASSERT(fake_loadstats_connection_->waitForNewStream(*dispatcher_, &loadstats_stream_)); } void @@ -218,7 +218,7 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, // merge until all the expected load has been reported. do { envoy::service::load_stats::v2::LoadStatsRequest local_loadstats_request; - loadstats_stream_->waitForGrpcMessage(*dispatcher_, local_loadstats_request); + ASSERT(loadstats_stream_->waitForGrpcMessage(*dispatcher_, local_loadstats_request)); // Sanity check and clear the measured load report interval. for (auto& cluster_stats : *local_loadstats_request.mutable_cluster_stats()) { const uint32_t actual_load_report_interval_ms = @@ -244,10 +244,10 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, } void waitForUpstreamResponse(uint32_t endpoint_index, uint32_t response_code = 200) { - fake_upstream_connection_ = - service_upstream_[endpoint_index]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT(service_upstream_[endpoint_index]->waitForHttpConnection(*dispatcher_, + &fake_upstream_connection_)); + ASSERT(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT(upstream_request_->waitForEndStream(*dispatcher_)); upstream_request_->encodeHeaders( Http::TestHeaderMapImpl{{":status", std::to_string(response_code)}}, false); @@ -293,8 +293,8 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, void cleanupLoadStatsConnection() { if (fake_loadstats_connection_ != nullptr) { - fake_loadstats_connection_->close(); - fake_loadstats_connection_->waitForDisconnect(); + ASSERT(fake_loadstats_connection_->close()); + ASSERT(fake_loadstats_connection_->waitForDisconnect()); } } diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index 6a298d3b95c99..d44ac4bc7b473 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -74,11 +74,11 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, } void waitForRatelimitRequest() { - fake_ratelimit_connection_ = fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); - ratelimit_request_ = fake_ratelimit_connection_->waitForNewStream(*dispatcher_); + ASSERT(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_ratelimit_connection_)); + ASSERT(fake_ratelimit_connection_->waitForNewStream(*dispatcher_, &ratelimit_request_)); envoy::service::ratelimit::v2::RateLimitRequest request_msg; - ratelimit_request_->waitForGrpcMessage(*dispatcher_, request_msg); - ratelimit_request_->waitForEndStream(*dispatcher_); + ASSERT(ratelimit_request_->waitForGrpcMessage(*dispatcher_, request_msg)); + ASSERT(ratelimit_request_->waitForEndStream(*dispatcher_)); EXPECT_STREQ("POST", ratelimit_request_->headers().Method()->value().c_str()); if (useDataPlaneProto()) { EXPECT_STREQ("/envoy.service.ratelimit.v2.RateLimitService/ShouldRateLimit", @@ -98,9 +98,9 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, } void waitForSuccessfulUpstreamResponse() { - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT(upstream_request_->waitForEndStream(*dispatcher_)); upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); upstream_request_->encodeData(response_size_, true); @@ -133,9 +133,9 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, if (fake_ratelimit_connection_ != nullptr) { if (clientType() != Grpc::ClientType::GoogleGrpc) { // TODO(htuch) we should document the underlying cause of this difference and/or fix it. - fake_ratelimit_connection_->close(); + ASSERT(fake_ratelimit_connection_->close()); } - fake_ratelimit_connection_->waitForDisconnect(); + ASSERT(fake_ratelimit_connection_->waitForDisconnect()); } cleanupUpstreamAndDownstream(); } @@ -213,9 +213,9 @@ TEST_P(RatelimitIntegrationTest, Timeout) { TEST_P(RatelimitIntegrationTest, ConnectImmediateDisconnect) { initiateClientConnection(); - fake_ratelimit_connection_ = fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); - fake_ratelimit_connection_->close(); - fake_ratelimit_connection_->waitForDisconnect(true); + ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_ratelimit_connection_)); + ASSERT_TRUE(fake_ratelimit_connection_->close()); + ASSERT_TRUE(fake_ratelimit_connection_->waitForDisconnect(true)); fake_ratelimit_connection_ = nullptr; // Rate limiter fails open waitForSuccessfulUpstreamResponse(); diff --git a/test/integration/tcp_conn_pool_integration_test.cc b/test/integration/tcp_conn_pool_integration_test.cc index 4ae2b50eccfc7..01233273f9419 100644 --- a/test/integration/tcp_conn_pool_integration_test.cc +++ b/test/integration/tcp_conn_pool_integration_test.cc @@ -152,9 +152,10 @@ TEST_P(TcpConnPoolIntegrationTest, SingleRequest) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); tcp_client->write(request); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - fake_upstream_connection->waitForData(request.size()); - fake_upstream_connection->write(response); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstream_connection->waitForData(request.size())); + ASSERT_TRUE(fake_upstream_connection->write(response)); tcp_client->waitForData(response); tcp_client->close(); @@ -170,20 +171,25 @@ TEST_P(TcpConnPoolIntegrationTest, MultipleRequests) { // send request 1 tcp_client->write(request1); - FakeRawConnectionPtr fake_upstream_connection1 = fake_upstreams_[0]->waitForRawConnection(); - EXPECT_EQ(request1, fake_upstream_connection1->waitForData(request1.size())); + FakeRawConnectionPtr fake_upstream_connection1; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection1)); + std::string data; + ASSERT_TRUE(fake_upstream_connection1->waitForData(request1.size(), &data)); + EXPECT_EQ(request1, data); // send request 2 tcp_client->write(request2); - FakeRawConnectionPtr fake_upstream_connection2 = fake_upstreams_[0]->waitForRawConnection(); - EXPECT_EQ(request2, fake_upstream_connection2->waitForData(request2.size())); + FakeRawConnectionPtr fake_upstream_connection2; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection2)); + ASSERT_TRUE(fake_upstream_connection2->waitForData(request2.size(), &data)); + EXPECT_EQ(request2, data); // send response 2 - fake_upstream_connection2->write(response2); + ASSERT_TRUE(fake_upstream_connection2->write(response2)); tcp_client->waitForData(response2); // send response 1 - fake_upstream_connection1->write(response1); + ASSERT_TRUE(fake_upstream_connection1->write(response1)); tcp_client->waitForData(response1, false); tcp_client->close(); diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index 64779fb89ac20..c00b69690a703 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -33,19 +33,20 @@ void TcpProxyIntegrationTest::initialize() { TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamWritesFirst) { initialize(); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); - fake_upstream_connection->write("hello"); + ASSERT_TRUE(fake_upstream_connection->write("hello")); tcp_client->waitForData("hello"); tcp_client->write("hello"); - fake_upstream_connection->waitForData(5); + ASSERT_TRUE(fake_upstream_connection->waitForData(5)); - fake_upstream_connection->write("", true); + ASSERT_TRUE(fake_upstream_connection->write("", true)); tcp_client->waitForHalfClose(); tcp_client->write("", true); - fake_upstream_connection->waitForHalfClose(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForHalfClose()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); } // Test proxying data in both directions, and that all data is flushed properly @@ -54,11 +55,12 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamDisconnect) { initialize(); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); tcp_client->write("hello"); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - fake_upstream_connection->waitForData(5); - fake_upstream_connection->write("world"); - fake_upstream_connection->close(); - fake_upstream_connection->waitForDisconnect(); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstream_connection->waitForData(5)); + ASSERT_TRUE(fake_upstream_connection->write("world")); + ASSERT_TRUE(fake_upstream_connection->close()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); tcp_client->waitForHalfClose(); tcp_client->close(); @@ -71,15 +73,16 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyDownstreamDisconnect) { initialize(); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); tcp_client->write("hello"); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - fake_upstream_connection->waitForData(5); - fake_upstream_connection->write("world"); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstream_connection->waitForData(5)); + ASSERT_TRUE(fake_upstream_connection->write("world")); tcp_client->waitForData("world"); tcp_client->write("hello", true); - fake_upstream_connection->waitForData(10); - fake_upstream_connection->waitForHalfClose(); - fake_upstream_connection->write("", true); - fake_upstream_connection->waitForDisconnect(true); + ASSERT_TRUE(fake_upstream_connection->waitForData(10)); + ASSERT_TRUE(fake_upstream_connection->waitForHalfClose()); + ASSERT_TRUE(fake_upstream_connection->write("", true)); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect(true)); tcp_client->waitForDisconnect(); } @@ -90,14 +93,15 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyLargeWrite) { std::string data(1024 * 16, 'a'); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); tcp_client->write(data); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - fake_upstream_connection->waitForData(data.size()); - fake_upstream_connection->write(data); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstream_connection->waitForData(data.size())); + ASSERT_TRUE(fake_upstream_connection->write(data)); tcp_client->waitForData(data); tcp_client->close(); - fake_upstream_connection->waitForHalfClose(); - fake_upstream_connection->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForHalfClose()); + ASSERT_TRUE(fake_upstream_connection->close()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); uint32_t upstream_pauses = test_server_->counter("cluster.cluster_0.upstream_flow_control_paused_reading_total") @@ -123,15 +127,16 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyDownstreamFlush) { std::string data(size, 'a'); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); tcp_client->readDisable(true); tcp_client->write("", true); // This ensures that readDisable(true) has been run on it's thread // before tcp_client starts writing. - fake_upstream_connection->waitForHalfClose(); + ASSERT_TRUE(fake_upstream_connection->waitForHalfClose()); - fake_upstream_connection->write(data, true); + ASSERT_TRUE(fake_upstream_connection->write(data, true)); test_server_->waitForCounterGe("cluster.cluster_0.upstream_flow_control_paused_reading_total", 1); EXPECT_EQ(test_server_->counter("cluster.cluster_0.upstream_flow_control_resumed_reading_total") @@ -140,7 +145,7 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyDownstreamFlush) { tcp_client->readDisable(false); tcp_client->waitForData(data); tcp_client->waitForHalfClose(); - fake_upstream_connection->waitForHalfClose(); + ASSERT_TRUE(fake_upstream_connection->waitForHalfClose()); uint32_t upstream_pauses = test_server_->counter("cluster.cluster_0.upstream_flow_control_paused_reading_total") @@ -161,9 +166,10 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamFlush) { std::string data(size, 'a'); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - fake_upstream_connection->readDisable(true); - fake_upstream_connection->write("", true); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstream_connection->readDisable(true)); + ASSERT_TRUE(fake_upstream_connection->write("", true)); // This ensures that fake_upstream_connection->readDisable has been run on it's thread // before tcp_client starts writing. @@ -172,9 +178,9 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamFlush) { tcp_client->write(data, true); test_server_->waitForGaugeEq("tcp.tcp_stats.upstream_flush_active", 1); - fake_upstream_connection->readDisable(false); - fake_upstream_connection->waitForData(data.size()); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->readDisable(false)); + ASSERT_TRUE(fake_upstream_connection->waitForData(data.size())); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); tcp_client->waitForHalfClose(); EXPECT_EQ(test_server_->counter("tcp.tcp_stats.upstream_flush_total")->value(), 1); @@ -190,9 +196,10 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamFlushEnvoyExit) { std::string data(size, 'a'); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - fake_upstream_connection->readDisable(true); - fake_upstream_connection->write("", true); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstream_connection->readDisable(true)); + ASSERT_TRUE(fake_upstream_connection->write("", true)); // This ensures that fake_upstream_connection->readDisable has been run on it's thread // before tcp_client starts writing. @@ -202,8 +209,8 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamFlushEnvoyExit) { test_server_->waitForGaugeEq("tcp.tcp_stats.upstream_flush_active", 1); test_server_.reset(); - fake_upstream_connection->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->close()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); // Success criteria is that no ASSERTs fire and there are no leaks. } @@ -233,16 +240,17 @@ TEST_P(TcpProxyIntegrationTest, AccessLog) { initialize(); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); - fake_upstream_connection->write("hello"); + ASSERT_TRUE(fake_upstream_connection->write("hello")); tcp_client->waitForData("hello"); - fake_upstream_connection->write("", true); + ASSERT_TRUE(fake_upstream_connection->write("", true)); tcp_client->waitForHalfClose(); tcp_client->write("", true); - fake_upstream_connection->waitForHalfClose(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForHalfClose()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); std::string log_result; // Access logs only get flushed to disk periodically, so poll until the log is non-empty @@ -277,16 +285,17 @@ TEST_P(TcpProxyIntegrationTest, ShutdownWithOpenConnections) { initialize(); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); tcp_client->write("hello"); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - fake_upstream_connection->waitForData(5); - fake_upstream_connection->write("world"); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstream_connection->waitForData(5)); + ASSERT_TRUE(fake_upstream_connection->write("world")); tcp_client->waitForData("world"); tcp_client->write("hello", false); - fake_upstream_connection->waitForData(10); + ASSERT_TRUE(fake_upstream_connection->waitForData(10)); test_server_.reset(); - fake_upstream_connection->waitForHalfClose(); - fake_upstream_connection->close(); - fake_upstream_connection->waitForDisconnect(true); + ASSERT_TRUE(fake_upstream_connection->waitForHalfClose()); + ASSERT_TRUE(fake_upstream_connection->close()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect(true)); tcp_client->waitForHalfClose(); tcp_client->close(); @@ -333,14 +342,15 @@ TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithLargeOutstandingData) { initialize(); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); std::string data(1024 * 16, 'a'); tcp_client->write(data); - fake_upstream_connection->write(data); + ASSERT_TRUE(fake_upstream_connection->write(data)); tcp_client->waitForDisconnect(true); - fake_upstream_connection->waitForDisconnect(true); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect(true)); } void TcpProxySslIntegrationTest::initialize() { @@ -387,7 +397,7 @@ void TcpProxySslIntegrationTest::setupConnections() { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); } - fake_upstream_connection_ = fake_upstreams_[0]->waitForRawConnection(); + ASSERT(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection_)); } // Test proxying data in both directions with envoy doing TCP and TLS @@ -402,10 +412,10 @@ void TcpProxySslIntegrationTest::sendAndReceiveTlsData(const std::string& data_t } // Make sure the data makes it upstream. - fake_upstream_connection_->waitForData(data_to_send_upstream.size()); + ASSERT_TRUE(fake_upstream_connection_->waitForData(data_to_send_upstream.size())); // Now send data downstream and make sure it arrives. - fake_upstream_connection_->write(data_to_send_downstream); + ASSERT_TRUE(fake_upstream_connection_->write(data_to_send_downstream)); payload_reader_->set_data_to_wait_for(data_to_send_downstream); ssl_client_->dispatcher().run(Event::Dispatcher::RunType::Block); @@ -413,9 +423,9 @@ void TcpProxySslIntegrationTest::sendAndReceiveTlsData(const std::string& data_t Buffer::OwnedImpl empty_buffer; ssl_client_->write(empty_buffer, true); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - fake_upstream_connection_->waitForHalfClose(); - fake_upstream_connection_->write("", true); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->waitForHalfClose()); + ASSERT_TRUE(fake_upstream_connection_->write("", true)); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); ssl_client_->dispatcher().run(Event::Dispatcher::RunType::Block); EXPECT_TRUE(payload_reader_->readLastByte()); EXPECT_TRUE(connect_callbacks_.closed()); @@ -438,15 +448,15 @@ TEST_P(TcpProxySslIntegrationTest, DownstreamHalfClose) { Buffer::OwnedImpl empty_buffer; ssl_client_->write(empty_buffer, true); - fake_upstream_connection_->waitForHalfClose(); + ASSERT_TRUE(fake_upstream_connection_->waitForHalfClose()); const std::string data("data"); - fake_upstream_connection_->write(data, false); + ASSERT_TRUE(fake_upstream_connection_->write(data, false)); payload_reader_->set_data_to_wait_for(data); ssl_client_->dispatcher().run(Event::Dispatcher::RunType::Block); EXPECT_FALSE(payload_reader_->readLastByte()); - fake_upstream_connection_->write("", true); + ASSERT_TRUE(fake_upstream_connection_->write("", true)); ssl_client_->dispatcher().run(Event::Dispatcher::RunType::Block); EXPECT_TRUE(payload_reader_->readLastByte()); } @@ -455,7 +465,7 @@ TEST_P(TcpProxySslIntegrationTest, DownstreamHalfClose) { TEST_P(TcpProxySslIntegrationTest, UpstreamHalfClose) { setupConnections(); - fake_upstream_connection_->write("", true); + ASSERT_TRUE(fake_upstream_connection_->write("", true)); ssl_client_->dispatcher().run(Event::Dispatcher::RunType::Block); EXPECT_TRUE(payload_reader_->readLastByte()); EXPECT_FALSE(connect_callbacks_.closed()); @@ -466,14 +476,14 @@ TEST_P(TcpProxySslIntegrationTest, UpstreamHalfClose) { while (client_write_buffer_->bytes_drained() != val.size()) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); } - fake_upstream_connection_->waitForData(val.size()); + ASSERT_TRUE(fake_upstream_connection_->waitForData(val.size())); Buffer::OwnedImpl empty_buffer; ssl_client_->write(empty_buffer, true); while (!connect_callbacks_.closed()) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); } - fake_upstream_connection_->waitForHalfClose(); + ASSERT_TRUE(fake_upstream_connection_->waitForHalfClose()); } } // namespace diff --git a/test/integration/websocket_integration_test.cc b/test/integration/websocket_integration_test.cc index aa9f838e4572c..0248abd466167 100644 --- a/test/integration/websocket_integration_test.cc +++ b/test/integration/websocket_integration_test.cc @@ -124,28 +124,31 @@ TEST_P(WebsocketIntegrationTest, WebSocketConnectionDownstreamDisconnect) { if (old_style_websockets_) { test_server_->waitForCounterGe("tcp.websocket.downstream_cx_total", 1); } - fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - const std::string data = fake_upstream_connection->waitForData(&headersRead); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(&headersRead, &data)); validateInitialUpstreamData(data); // Accept websocket upgrade request - fake_upstream_connection->write(upgrade_resp_str_); + ASSERT_TRUE(fake_upstream_connection->write(upgrade_resp_str_)); tcp_client->waitForData("\r\n\r\n", false); validateInitialDownstreamData(tcp_client->data(), downstreamRespStr()); // Standard TCP proxy semantics post upgrade tcp_client->write("hello"); - fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("hello")); - fake_upstream_connection->write("world"); + ASSERT_TRUE( + fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("hello"))); + ASSERT_TRUE(fake_upstream_connection->write("world")); tcp_client->waitForData("world", false); tcp_client->write("bye!"); // downstream disconnect tcp_client->close(); - std::string final_data = - fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("bye")); - fake_upstream_connection->waitForDisconnect(); + std::string final_data; + ASSERT_TRUE(fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("bye"), + &final_data)); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); validateFinalDownstreamData(tcp_client->data(), downstreamRespStr() + "world"); @@ -174,25 +177,26 @@ TEST_P(WebsocketIntegrationTest, WebSocketConnectionUpstreamDisconnect) { tcp_client = makeTcpConnection(lookupPort("http")); // Send websocket upgrade request tcp_client->write(upgrade_req_str_); - fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - ASSERT_TRUE(fake_upstream_connection != nullptr); - const std::string data = fake_upstream_connection->waitForData(&headersRead); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(&headersRead, &data)); validateInitialUpstreamData(data); // Accept websocket upgrade request - fake_upstream_connection->write(upgrade_resp_str_); + ASSERT_TRUE(fake_upstream_connection->write(upgrade_resp_str_)); tcp_client->waitForData("\r\n\r\n", false); validateInitialDownstreamData(tcp_client->data(), downstreamRespStr()); // Standard TCP proxy semantics post upgrade tcp_client->write("hello"); - fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("hello")); + ASSERT_TRUE( + fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("hello"))); - fake_upstream_connection->write("world"); + ASSERT_TRUE(fake_upstream_connection->write("world")); // upstream disconnect - fake_upstream_connection->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->close()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); tcp_client->waitForData("world", false); tcp_client->waitForDisconnect(); ASSERT(!fake_upstream_connection->connected()); @@ -216,20 +220,21 @@ TEST_P(WebsocketIntegrationTest, EarlyData) { tcp_client = makeTcpConnection(lookupPort("http")); // Send early data alongside websocket upgrade request tcp_client->write(upgrade_req_str + early_data_req_str); - fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); // Wait for both the upgrade, and the early data. - const std::string data = fake_upstream_connection->waitForData( - FakeRawConnection::waitForInexactMatch(early_data_req_str.c_str())); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData( + FakeRawConnection::waitForInexactMatch(early_data_req_str.c_str()), &data)); // We expect to find the early data on the upstream side EXPECT_TRUE(StringUtil::endsWith(data, early_data_req_str)); // Accept websocket upgrade request - fake_upstream_connection->write(upgrade_resp_str_); + ASSERT_TRUE(fake_upstream_connection->write(upgrade_resp_str_)); // Reply also with early data - fake_upstream_connection->write(early_data_resp_str); + ASSERT_TRUE(fake_upstream_connection->write(early_data_resp_str)); // upstream disconnect - fake_upstream_connection->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->close()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); tcp_client->waitForData(early_data_resp_str, false); tcp_client->waitForDisconnect(); @@ -263,18 +268,19 @@ TEST_P(WebsocketIntegrationTest, WebSocketConnectionIdleTimeout) { // The request path gets rewritten from /websocket/test to /websocket. // The size of headers received by the destination is 228 bytes. tcp_client->write(upgrade_req_str_); - fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - const std::string data = fake_upstream_connection->waitForData(&headersRead); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(&headersRead, &data)); validateInitialUpstreamData(data); // Accept websocket upgrade request - fake_upstream_connection->write(upgrade_resp_str_); + ASSERT_TRUE(fake_upstream_connection->write(upgrade_resp_str_)); tcp_client->waitForData("\r\n\r\n", false); validateInitialDownstreamData(tcp_client->data(), downstreamRespStr()); // Standard TCP proxy semantics post upgrade tcp_client->write("hello"); tcp_client->write("hello"); - fake_upstream_connection->write("world"); + ASSERT_TRUE(fake_upstream_connection->write("world")); tcp_client->waitForData("world", false); if (old_style_websockets_) { @@ -283,7 +289,7 @@ TEST_P(WebsocketIntegrationTest, WebSocketConnectionIdleTimeout) { test_server_->waitForCounterGe("http.config_test.downstream_rq_idle_timeout", 1); } tcp_client->waitForDisconnect(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); } TEST_P(WebsocketIntegrationTest, WebSocketLogging) { @@ -335,20 +341,21 @@ TEST_P(WebsocketIntegrationTest, WebSocketLogging) { // The request path gets rewritten from /websocket/test to /websocket. // The size of headers received by the destination is 228 bytes. tcp_client->write(upgrade_req_str_); - fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - const std::string data = fake_upstream_connection->waitForData(228); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(228, &data)); // Accept websocket upgrade request - fake_upstream_connection->write(upgrade_resp_str_); + ASSERT_TRUE(fake_upstream_connection->write(upgrade_resp_str_)); tcp_client->waitForData(upgrade_resp_str_); // Standard TCP proxy semantics post upgrade tcp_client->write("hello"); // datalen = 228 + strlen(hello) - fake_upstream_connection->waitForData(233); - fake_upstream_connection->write("world"); + ASSERT_TRUE(fake_upstream_connection->waitForData(233)); + ASSERT_TRUE(fake_upstream_connection->write("world")); tcp_client->waitForData(upgrade_resp_str_ + "world"); - fake_upstream_connection->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->close()); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); tcp_client->waitForDisconnect(); tcp_client->close(); @@ -400,12 +407,13 @@ TEST_P(WebsocketIntegrationTest, NonWebsocketUpgrade) { if (old_style_websockets_) { test_server_->waitForCounterGe("tcp.websocket.downstream_cx_total", 1); } - fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - const std::string data = fake_upstream_connection->waitForData(&headersRead); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(&headersRead, &data)); validateInitialUpstreamData(data); // Accept websocket upgrade request - fake_upstream_connection->write(upgrade_resp_str); + ASSERT_TRUE(fake_upstream_connection->write(upgrade_resp_str)); tcp_client->waitForData("\r\n\r\n", false); if (old_style_websockets_) { ASSERT_EQ(tcp_client->data(), upgrade_resp_str); @@ -413,16 +421,18 @@ TEST_P(WebsocketIntegrationTest, NonWebsocketUpgrade) { // Standard TCP proxy semantics post upgrade tcp_client->write("hello"); - fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("hello")); - fake_upstream_connection->write("world"); + ASSERT_TRUE( + fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("hello"))); + ASSERT_TRUE(fake_upstream_connection->write("world")); tcp_client->waitForData("world", false); tcp_client->write("bye!"); // downstream disconnect tcp_client->close(); - std::string final_data = - fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("bye")); - fake_upstream_connection->waitForDisconnect(); + std::string final_data; + ASSERT_TRUE(fake_upstream_connection->waitForData(FakeRawConnection::waitForInexactMatch("bye"), + &final_data)); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); const std::string modified_upgrade_resp_str = "HTTP/1.1 101 Switching Protocols\r\nconnection: " "Upgrade\r\nupgrade: foo\r\ncontent-length: " @@ -500,14 +510,14 @@ TEST_P(WebsocketIntegrationTest, WebsocketCustomFilterChain) { early_data_req_str.length()); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("http")); tcp_client->write(upgrade_req_str + early_data_req_str); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - ASSERT_TRUE(fake_upstream_connection != nullptr); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); // Make sure the full payload arrives. - const std::string data = fake_upstream_connection->waitForData( - FakeRawConnection::waitForInexactMatch(early_data_req_str.c_str())); + ASSERT_TRUE(fake_upstream_connection->waitForData( + FakeRawConnection::waitForInexactMatch(early_data_req_str.c_str()))); // Tear down all the connections cleanly. tcp_client->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); } } @@ -523,9 +533,10 @@ TEST_P(WebsocketIntegrationTest, BidirectionalChunkedData) { // Upgrade, send initial data and wait for it to be received. IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("http")); tcp_client->write(upgrade_req_str); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - const std::string data = fake_upstream_connection->waitForData( - FakeRawConnection::waitForInexactMatch("SomeWebSocketPayload")); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstream_connection->waitForData( + FakeRawConnection::waitForInexactMatch("SomeWebSocketPayload"))); // Finish the upgrade. const std::string upgrade_resp_str = @@ -533,19 +544,20 @@ TEST_P(WebsocketIntegrationTest, BidirectionalChunkedData) { "transfer-encoding: chunked\r\n\r\n" "4\r\nabcd\r\n0\r\n\r\n" "SomeWebsocketResponsePayload"; - fake_upstream_connection->write(upgrade_resp_str); + ASSERT_TRUE(fake_upstream_connection->write(upgrade_resp_str)); tcp_client->waitForData("SomeWebsocketResponsePayload", false); // Verify bidirectional data still works. tcp_client->write("FinalClientPayload"); - std::string final_data = fake_upstream_connection->waitForData( - FakeRawConnection::waitForInexactMatch("FinalClientPayload")); - fake_upstream_connection->write("FinalServerPayload"); + std::string final_data; + ASSERT_TRUE(fake_upstream_connection->waitForData( + FakeRawConnection::waitForInexactMatch("FinalClientPayload"), &final_data)); + ASSERT_TRUE(fake_upstream_connection->write("FinalServerPayload")); tcp_client->waitForData("FinalServerPayload", false); // Clean up. tcp_client->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); const std::string modified_upstream_payload = "GET /websocket/test HTTP/1.1\r\n" diff --git a/test/integration/xfcc_integration_test.cc b/test/integration/xfcc_integration_test.cc index fa2116055374b..dfe53530944af 100644 --- a/test/integration/xfcc_integration_test.cc +++ b/test/integration/xfcc_integration_test.cc @@ -149,9 +149,9 @@ void XfccIntegrationTest::testRequestAndResponseWithXfccHeader(std::string previ codec_client_ = makeHttpConnection(std::move(conn)); auto response = codec_client_->makeHeaderOnlyRequest(header_map); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); if (expected_xfcc.empty()) { EXPECT_EQ(nullptr, upstream_request_->headers().ForwardedClientCert()); } else { diff --git a/test/test_common/utility.h b/test/test_common/utility.h index c972c9ad407c6..250492354e9dc 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -75,6 +75,14 @@ namespace Envoy { EXPECT_DEATH(statement, message); \ } while (false) +#define VERIFY_ASSERTION(statement) \ + { \ + ::testing::AssertionResult status = statement; \ + if (!status) { \ + return status; \ + } \ + } + // Random number generator which logs its seed to stderr. To repeat a test run with a non-zero seed // one can run the test with --test_arg=--gtest_random_seed=[seed] class TestRandomGenerator { From e4dcd85860116e43bcbfe8d9bdd27388886e5821 Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Mon, 23 Jul 2018 19:06:10 -0400 Subject: [PATCH 2/6] Fix tests outside of test/integration. Signed-off-by: Michael Behr --- .../grpc/grpc_client_integration_test.cc | 8 ++-- .../grpc_client_integration_test_harness.h | 22 +++++---- .../extensions/access_loggers/http_grpc/BUILD | 1 + .../grpc_access_log_integration_test.cc | 42 ++++++++++------- .../grpc_json_transcoder_integration_test.cc | 13 ++--- .../http/jwt_authn/filter_integration_test.cc | 14 +++--- .../filters/http/lua/lua_integration_test.cc | 20 ++++---- .../squash/squash_filter_integration_test.cc | 11 +++-- .../thrift_proxy/filter_integration_test.cc | 47 +++++++++++-------- ...le_based_metadata_grpc_credentials_test.cc | 2 +- .../stats_sinks/metrics_service/BUILD | 1 + .../metrics_service_integration_test.cc | 32 ++++++++----- 12 files changed, 123 insertions(+), 90 deletions(-) diff --git a/test/common/grpc/grpc_client_integration_test.cc b/test/common/grpc/grpc_client_integration_test.cc index 65046f2052d79..c60a4273cb2b0 100644 --- a/test/common/grpc/grpc_client_integration_test.cc +++ b/test/common/grpc/grpc_client_integration_test.cc @@ -308,10 +308,10 @@ TEST_P(GrpcClientIntegrationTest, ResetAfterCloseLocal) { initialize(); auto stream = createStream(empty_metadata_); stream->grpc_stream_->closeStream(); - stream->fake_stream_->waitForEndStream(dispatcher_helper_.dispatcher_); + ASSERT_TRUE(stream->fake_stream_->waitForEndStream(dispatcher_helper_.dispatcher_)); stream->grpc_stream_->resetStream(); dispatcher_helper_.dispatcher_.run(Event::Dispatcher::RunType::NonBlock); - stream->fake_stream_->waitForReset(); + ASSERT_TRUE(stream->fake_stream_->waitForReset()); } // Validate that request cancel() works. @@ -323,7 +323,7 @@ TEST_P(GrpcClientIntegrationTest, CancelRequest) { EXPECT_CALL(*request->child_span_, finishSpan()); request->grpc_request_->cancel(); dispatcher_helper_.dispatcher_.run(Event::Dispatcher::RunType::NonBlock); - request->fake_stream_->waitForReset(); + ASSERT_TRUE(request->fake_stream_->waitForReset()); } // Parameterize the loopback test server socket address and gRPC client type. @@ -352,7 +352,7 @@ TEST_P(GrpcSslClientIntegrationTest, BasicSslRequestWithClientCert) { class GrpcAccessTokenClientIntegrationTest : public GrpcSslClientIntegrationTest { public: void expectExtraHeaders(FakeStream& fake_stream) override { - fake_stream.waitForHeadersComplete(); + ASSERT(fake_stream.waitForHeadersComplete()); Http::TestHeaderMapImpl stream_headers(fake_stream.headers()); if (access_token_value_ != "") { if (access_token_value_2_ == "") { diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index 877c810497dea..72252c0bca109 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -82,7 +82,7 @@ class HelloworldStream : public MockAsyncStreamCallbacks grpc_stream_->sendMessage(request_msg, end_stream); helloworld::HelloRequest received_msg; - fake_stream_->waitForGrpcMessage(dispatcher_helper_.dispatcher_, received_msg); + ASSERT(fake_stream_->waitForGrpcMessage(dispatcher_helper_.dispatcher_, received_msg)); EXPECT_THAT(request_msg, ProtoEq(received_msg)); } @@ -162,7 +162,7 @@ class HelloworldStream : public MockAsyncStreamCallbacks void closeStream() { grpc_stream_->closeStream(); - fake_stream_->waitForEndStream(dispatcher_helper_.dispatcher_); + ASSERT(fake_stream_->waitForEndStream(dispatcher_helper_.dispatcher_)); } DispatcherHelper& dispatcher_helper_; @@ -224,8 +224,8 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { void TearDown() override { if (fake_connection_) { - fake_connection_->close(); - fake_connection_->waitForDisconnect(); + ASSERT(fake_connection_->close()); + ASSERT(fake_connection_->waitForDisconnect()); fake_connection_.reset(); } } @@ -291,7 +291,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { } void expectInitialHeaders(FakeStream& fake_stream, const TestMetadata& initial_metadata) { - fake_stream.waitForHeadersComplete(); + ASSERT(fake_stream.waitForHeadersComplete()); Http::TestHeaderMapImpl stream_headers(fake_stream.headers()); EXPECT_EQ("POST", stream_headers.get_(":method")); EXPECT_EQ("/helloworld.Greeter/SayHello", stream_headers.get_(":path")); @@ -333,9 +333,10 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { EXPECT_NE(request->grpc_request_, nullptr); if (!fake_connection_) { - fake_connection_ = fake_upstream_->waitForHttpConnection(dispatcher_); + ASSERT(fake_upstream_->waitForHttpConnection(dispatcher_, &fake_connection_)); } - fake_streams_.push_back(fake_connection_->waitForNewStream(dispatcher_)); + fake_streams_.emplace_back(); + ASSERT(fake_connection_->waitForNewStream(dispatcher_, &fake_streams_.back())); auto& fake_stream = *fake_streams_.back(); request->fake_stream_ = &fake_stream; @@ -343,7 +344,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { expectExtraHeaders(fake_stream); helloworld::HelloRequest received_msg; - fake_stream.waitForGrpcMessage(dispatcher_, received_msg); + ASSERT(fake_stream.waitForGrpcMessage(dispatcher_, received_msg)); EXPECT_THAT(request_msg, ProtoEq(received_msg)); return request; @@ -362,9 +363,10 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { EXPECT_NE(stream->grpc_stream_, nullptr); if (!fake_connection_) { - fake_connection_ = fake_upstream_->waitForHttpConnection(dispatcher_); + ASSERT(fake_upstream_->waitForHttpConnection(dispatcher_, &fake_connection_)); } - fake_streams_.push_back(fake_connection_->waitForNewStream(dispatcher_)); + fake_streams_.emplace_back(); + ASSERT(fake_connection_->waitForNewStream(dispatcher_, &fake_streams_.back())); auto& fake_stream = *fake_streams_.back(); stream->fake_stream_ = &fake_stream; diff --git a/test/extensions/access_loggers/http_grpc/BUILD b/test/extensions/access_loggers/http_grpc/BUILD index 1d5314a999fe2..2f2e07aab3ca9 100644 --- a/test/extensions/access_loggers/http_grpc/BUILD +++ b/test/extensions/access_loggers/http_grpc/BUILD @@ -46,5 +46,6 @@ envoy_extension_cc_test( "//source/extensions/access_loggers/http_grpc:config", "//test/common/grpc:grpc_client_integration_lib", "//test/integration:http_integration_lib", + "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc b/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc index f734f4c72d346..0bd519c3a0f4c 100644 --- a/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc +++ b/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc @@ -8,9 +8,12 @@ #include "test/common/grpc/grpc_client_integration.h" #include "test/integration/http_integration.h" +#include "test/test_common/utility.h" #include "gtest/gtest.h" +using testing::AssertionResult; + namespace Envoy { namespace { @@ -49,17 +52,20 @@ class AccessLogIntegrationTest : public HttpIntegrationTest, HttpIntegrationTest::initialize(); } - void waitForAccessLogConnection() { - fake_access_log_connection_ = fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); + ABSL_MUST_USE_RESULT + AssertionResult waitForAccessLogConnection() { + return fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_access_log_connection_); } - void waitForAccessLogStream() { - access_log_request_ = fake_access_log_connection_->waitForNewStream(*dispatcher_); + ABSL_MUST_USE_RESULT + AssertionResult waitForAccessLogStream() { + return fake_access_log_connection_->waitForNewStream(*dispatcher_, &access_log_request_); } - void waitForAccessLogRequest(const std::string& expected_request_msg_yaml) { + ABSL_MUST_USE_RESULT + AssertionResult waitForAccessLogRequest(const std::string& expected_request_msg_yaml) { envoy::service::accesslog::v2::StreamAccessLogsMessage request_msg; - access_log_request_->waitForGrpcMessage(*dispatcher_, request_msg); + VERIFY_ASSERTION(access_log_request_->waitForGrpcMessage(*dispatcher_, request_msg)); EXPECT_STREQ("POST", access_log_request_->headers().Method()->value().c_str()); EXPECT_STREQ("/envoy.service.accesslog.v2.AccessLogService/StreamAccessLogs", access_log_request_->headers().Path()->value().c_str()); @@ -78,12 +84,14 @@ class AccessLogIntegrationTest : public HttpIntegrationTest, log_entry->mutable_common_properties()->clear_time_to_last_downstream_tx_byte(); log_entry->mutable_request()->clear_request_id(); EXPECT_EQ(request_msg.DebugString(), expected_request_msg.DebugString()); + + return AssertionSuccess(); } void cleanup() { if (fake_access_log_connection_ != nullptr) { - fake_access_log_connection_->close(); - fake_access_log_connection_->waitForDisconnect(); + ASSERT(fake_access_log_connection_->close()); + ASSERT(fake_access_log_connection_->waitForDisconnect()); } } @@ -97,9 +105,9 @@ INSTANTIATE_TEST_CASE_P(IpVersionsCientType, AccessLogIntegrationTest, // Test a basic full access logging flow. TEST_P(AccessLogIntegrationTest, BasicAccessLogFlow) { testRouterNotFound(); - waitForAccessLogConnection(); - waitForAccessLogStream(); - waitForAccessLogRequest(fmt::format(R"EOF( + ASSERT_TRUE(waitForAccessLogConnection()); + ASSERT_TRUE(waitForAccessLogStream()); + ASSERT_TRUE(waitForAccessLogRequest(fmt::format(R"EOF( identifier: node: id: node_name @@ -124,13 +132,13 @@ TEST_P(AccessLogIntegrationTest, BasicAccessLogFlow) { value: 404 response_headers_bytes: 54 )EOF", - VersionInfo::version())); + VersionInfo::version()))); BufferingStreamDecoderPtr response = IntegrationUtil::makeSingleRequest( lookupPort("http"), "GET", "/notfound", "", downstream_protocol_, version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("404", response->headers().Status()->value().c_str()); - waitForAccessLogRequest(R"EOF( + ASSERT_TRUE(waitForAccessLogRequest(R"EOF( http_logs: log_entry: common_properties: @@ -146,7 +154,7 @@ TEST_P(AccessLogIntegrationTest, BasicAccessLogFlow) { response_code: value: 404 response_headers_bytes: 54 -)EOF"); +)EOF")); // Send an empty response and end the stream. This should never happen but make sure nothing // breaks and we make a new stream on a follow up request. @@ -168,8 +176,8 @@ TEST_P(AccessLogIntegrationTest, BasicAccessLogFlow) { downstream_protocol_, version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("404", response->headers().Status()->value().c_str()); - waitForAccessLogStream(); - waitForAccessLogRequest(fmt::format(R"EOF( + ASSERT_TRUE(waitForAccessLogStream()); + ASSERT_TRUE(waitForAccessLogRequest(fmt::format(R"EOF( identifier: node: id: node_name @@ -194,7 +202,7 @@ TEST_P(AccessLogIntegrationTest, BasicAccessLogFlow) { value: 404 response_headers_bytes: 54 )EOF", - VersionInfo::version())); + VersionInfo::version()))); cleanup(); } diff --git a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc index 2417a31794ded..92188de9f01fd 100644 --- a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc +++ b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc @@ -71,10 +71,11 @@ class GrpcJsonTranscoderIntegrationTest response = codec_client_->makeHeaderOnlyRequest(request_headers); } - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); + ASSERT_TRUE( + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); if (!grpc_request_messages.empty()) { - upstream_request_->waitForEndStream(*dispatcher_); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); Grpc::Decoder grpc_decoder; std::vector frames; @@ -114,7 +115,7 @@ class GrpcJsonTranscoderIntegrationTest } EXPECT_TRUE(upstream_request_->complete()); } else { - upstream_request_->waitForReset(); + ASSERT_TRUE(upstream_request_->waitForReset()); } response->waitForEndStream(); @@ -136,8 +137,8 @@ class GrpcJsonTranscoderIntegrationTest } codec_client_->close(); - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); } }; diff --git a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc index 283481bdd482b..d0178216b4bb4 100644 --- a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc @@ -109,9 +109,9 @@ class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest { } void waitForJwksResponse(const std::string& status, const std::string& jwks_body) { - fake_jwks_connection_ = fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); - jwks_request_ = fake_jwks_connection_->waitForNewStream(*dispatcher_); - jwks_request_->waitForEndStream(*dispatcher_); + ASSERT(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_jwks_connection_)); + ASSERT(fake_jwks_connection_->waitForNewStream(*dispatcher_, &jwks_request_)); + ASSERT(jwks_request_->waitForEndStream(*dispatcher_)); Http::TestHeaderMapImpl response_headers{{":status", status}}; jwks_request_->encodeHeaders(response_headers, false); @@ -122,12 +122,12 @@ class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest { void cleanup() { codec_client_->close(); if (fake_jwks_connection_ != nullptr) { - fake_jwks_connection_->close(); - fake_jwks_connection_->waitForDisconnect(); + ASSERT(fake_jwks_connection_->close()); + ASSERT(fake_jwks_connection_->waitForDisconnect()); } if (fake_upstream_connection_ != nullptr) { - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT(fake_upstream_connection_->close()); + ASSERT(fake_upstream_connection_->waitForDisconnect()); } } diff --git a/test/extensions/filters/http/lua/lua_integration_test.cc b/test/extensions/filters/http/lua/lua_integration_test.cc index b7e9060a44e4c..91485ff7efcb3 100644 --- a/test/extensions/filters/http/lua/lua_integration_test.cc +++ b/test/extensions/filters/http/lua/lua_integration_test.cc @@ -72,12 +72,12 @@ class LuaIntegrationTest : public HttpIntegrationTest, void cleanup() { codec_client_->close(); if (fake_lua_connection_ != nullptr) { - fake_lua_connection_->close(); - fake_lua_connection_->waitForDisconnect(); + ASSERT(fake_lua_connection_->close()); + ASSERT(fake_lua_connection_->waitForDisconnect()); } if (fake_upstream_connection_ != nullptr) { - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + ASSERT(fake_upstream_connection_->close()); + ASSERT(fake_upstream_connection_->waitForDisconnect()); } } @@ -234,9 +234,9 @@ name: envoy.lua {"x-forwarded-for", "10.0.0.1"}}; auto response = codec_client_->makeHeaderOnlyRequest(request_headers); - fake_lua_connection_ = fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); - lua_request_ = fake_lua_connection_->waitForNewStream(*dispatcher_); - lua_request_->waitForEndStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_lua_connection_)); + ASSERT_TRUE(fake_lua_connection_->waitForNewStream(*dispatcher_, &lua_request_)); + ASSERT_TRUE(lua_request_->waitForEndStream(*dispatcher_)); Http::TestHeaderMapImpl response_headers{{":status", "200"}, {"foo", "bar"}}; lua_request_->encodeHeaders(response_headers, false); Buffer::OwnedImpl response_data1("good"); @@ -292,9 +292,9 @@ name: envoy.lua {"x-forwarded-for", "10.0.0.1"}}; auto response = codec_client_->makeHeaderOnlyRequest(request_headers); - fake_lua_connection_ = fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); - lua_request_ = fake_lua_connection_->waitForNewStream(*dispatcher_); - lua_request_->waitForEndStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_lua_connection_)); + ASSERT_TRUE(fake_lua_connection_->waitForNewStream(*dispatcher_, &lua_request_)); + ASSERT_TRUE(lua_request_->waitForEndStream(*dispatcher_)); Http::TestHeaderMapImpl response_headers{{":status", "200"}, {"foo", "bar"}}; lua_request_->encodeHeaders(response_headers, true); diff --git a/test/extensions/filters/http/squash/squash_filter_integration_test.cc b/test/extensions/filters/http/squash/squash_filter_integration_test.cc index 52c37153ad7d7..1631364842e12 100644 --- a/test/extensions/filters/http/squash/squash_filter_integration_test.cc +++ b/test/extensions/filters/http/squash/squash_filter_integration_test.cc @@ -22,19 +22,20 @@ class SquashFilterIntegrationTest : public HttpIntegrationTest, ~SquashFilterIntegrationTest() { if (fake_squash_connection_) { - fake_squash_connection_->close(); - fake_squash_connection_->waitForDisconnect(); + ASSERT(fake_squash_connection_->close()); + ASSERT(fake_squash_connection_->waitForDisconnect()); } } FakeStreamPtr sendSquash(const std::string& status, const std::string& body) { if (!fake_squash_connection_) { - fake_squash_connection_ = fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); + ASSERT(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_squash_connection_)); } - FakeStreamPtr request_stream = fake_squash_connection_->waitForNewStream(*dispatcher_); - request_stream->waitForEndStream(*dispatcher_); + FakeStreamPtr request_stream; + ASSERT(fake_squash_connection_->waitForNewStream(*dispatcher_, &request_stream)); + ASSERT(request_stream->waitForEndStream(*dispatcher_)); if (body.empty()) { request_stream->encodeHeaders(Http::TestHeaderMapImpl{{":status", status}}, true); } else { diff --git a/test/extensions/filters/network/thrift_proxy/filter_integration_test.cc b/test/extensions/filters/network/thrift_proxy/filter_integration_test.cc index 8317a11d8a123..c321f7782e1bd 100644 --- a/test/extensions/filters/network/thrift_proxy/filter_integration_test.cc +++ b/test/extensions/filters/network/thrift_proxy/filter_integration_test.cc @@ -173,16 +173,18 @@ TEST_P(ThriftFilterIntegrationTest, Success) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); tcp_client->write(request_bytes_.toString()); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - Buffer::OwnedImpl upstream_request( - fake_upstream_connection->waitForData(request_bytes_.length())); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(request_bytes_.length(), &data)); + Buffer::OwnedImpl upstream_request(data); EXPECT_TRUE(TestUtility::buffersEqual(upstream_request, request_bytes_)); - fake_upstream_connection->write(response_bytes_.toString()); + ASSERT_TRUE(fake_upstream_connection->write(response_bytes_.toString())); tcp_client->waitForData(response_bytes_.toString()); tcp_client->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); EXPECT_TRUE(TestUtility::buffersEqual(Buffer::OwnedImpl(tcp_client->data()), response_bytes_)); @@ -198,16 +200,18 @@ TEST_P(ThriftFilterIntegrationTest, IDLException) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); tcp_client->write(request_bytes_.toString()); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - Buffer::OwnedImpl upstream_request( - fake_upstream_connection->waitForData(request_bytes_.length())); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(request_bytes_.length(), &data)); + Buffer::OwnedImpl upstream_request(data); EXPECT_TRUE(TestUtility::buffersEqual(upstream_request, request_bytes_)); - fake_upstream_connection->write(response_bytes_.toString()); + ASSERT_TRUE(fake_upstream_connection->write(response_bytes_.toString())); tcp_client->waitForData(response_bytes_.toString()); tcp_client->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); EXPECT_TRUE(TestUtility::buffersEqual(Buffer::OwnedImpl(tcp_client->data()), response_bytes_)); @@ -223,16 +227,19 @@ TEST_P(ThriftFilterIntegrationTest, Exception) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); tcp_client->write(request_bytes_.toString()); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - Buffer::OwnedImpl upstream_request( - fake_upstream_connection->waitForData(request_bytes_.length())); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(request_bytes_.length(), &data)); + Buffer::OwnedImpl upstream_request(data); + EXPECT_TRUE(TestUtility::buffersEqual(upstream_request, request_bytes_)); - fake_upstream_connection->write(response_bytes_.toString()); + ASSERT_TRUE(fake_upstream_connection->write(response_bytes_.toString())); tcp_client->waitForData(response_bytes_.toString()); tcp_client->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); EXPECT_TRUE(TestUtility::buffersEqual(Buffer::OwnedImpl(tcp_client->data()), response_bytes_)); @@ -248,13 +255,15 @@ TEST_P(ThriftFilterIntegrationTest, Oneway) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); tcp_client->write(request_bytes_.toString()); - FakeRawConnectionPtr fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - Buffer::OwnedImpl upstream_request( - fake_upstream_connection->waitForData(request_bytes_.length())); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + std::string data; + ASSERT_TRUE(fake_upstream_connection->waitForData(request_bytes_.length(), &data)); + Buffer::OwnedImpl upstream_request(data); EXPECT_TRUE(TestUtility::buffersEqual(upstream_request, request_bytes_)); tcp_client->close(); - fake_upstream_connection->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); Stats::CounterSharedPtr counter = test_server_->counter("thrift.thrift_stats.request_oneway"); EXPECT_EQ(1U, counter->value()); diff --git a/test/extensions/grpc_credentials/file_based_metadata/file_based_metadata_grpc_credentials_test.cc b/test/extensions/grpc_credentials/file_based_metadata/file_based_metadata_grpc_credentials_test.cc index 40e10ab1b5e22..4a41fb7ced654 100644 --- a/test/extensions/grpc_credentials/file_based_metadata/file_based_metadata_grpc_credentials_test.cc +++ b/test/extensions/grpc_credentials/file_based_metadata/file_based_metadata_grpc_credentials_test.cc @@ -19,7 +19,7 @@ namespace { class GrpcFileBasedMetadataClientIntegrationTest : public GrpcSslClientIntegrationTest { public: void expectExtraHeaders(FakeStream& fake_stream) override { - fake_stream.waitForHeadersComplete(); + ASSERT(fake_stream.waitForHeadersComplete()); Http::TestHeaderMapImpl stream_headers(fake_stream.headers()); if (!header_value_1_.empty()) { EXPECT_EQ(header_prefix_1_ + header_value_1_, stream_headers.get_(header_key_1_)); diff --git a/test/extensions/stats_sinks/metrics_service/BUILD b/test/extensions/stats_sinks/metrics_service/BUILD index 3dba545540d45..e9a79fae08128 100644 --- a/test/extensions/stats_sinks/metrics_service/BUILD +++ b/test/extensions/stats_sinks/metrics_service/BUILD @@ -39,6 +39,7 @@ envoy_extension_cc_test( "//source/extensions/stat_sinks/metrics_service:config", "//test/common/grpc:grpc_client_integration_lib", "//test/integration:http_integration_lib", + "//test/test_common:utility_lib", "@envoy_api//envoy/service/metrics/v2:metrics_service_cc", ], ) diff --git a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc index 35e325223889b..d0a79bee9c6f8 100644 --- a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc +++ b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc @@ -7,9 +7,12 @@ #include "test/common/grpc/grpc_client_integration.h" #include "test/integration/http_integration.h" +#include "test/test_common/utility.h" #include "gtest/gtest.h" +using testing::AssertionResult; + namespace Envoy { namespace { @@ -47,15 +50,20 @@ class MetricsServiceIntegrationTest : public HttpIntegrationTest, HttpIntegrationTest::initialize(); } - void waitForMetricsServiceConnection() { - fake_metrics_service_connection_ = fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); + ABSL_MUST_USE_RESULT + AssertionResult waitForMetricsServiceConnection() { + return fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, + &fake_metrics_service_connection_); } - void waitForMetricsStream() { - metrics_service_request_ = fake_metrics_service_connection_->waitForNewStream(*dispatcher_); + ABSL_MUST_USE_RESULT + AssertionResult waitForMetricsStream() { + return fake_metrics_service_connection_->waitForNewStream(*dispatcher_, + &metrics_service_request_); } - void waitForMetricsRequest() { + ABSL_MUST_USE_RESULT + AssertionResult waitForMetricsRequest() { bool known_histogram_exists = false; bool known_counter_exists = false; bool known_gauge_exists = false; @@ -65,7 +73,7 @@ class MetricsServiceIntegrationTest : public HttpIntegrationTest, // flushed. while (!(known_counter_exists && known_gauge_exists && known_histogram_exists)) { envoy::service::metrics::v2::StreamMetricsMessage request_msg; - metrics_service_request_->waitForGrpcMessage(*dispatcher_, request_msg); + VERIFY_ASSERTION(metrics_service_request_->waitForGrpcMessage(*dispatcher_, request_msg)); EXPECT_STREQ("POST", metrics_service_request_->headers().Method()->value().c_str()); EXPECT_STREQ("/envoy.service.metrics.v2.MetricsService/StreamMetrics", metrics_service_request_->headers().Path()->value().c_str()); @@ -102,12 +110,14 @@ class MetricsServiceIntegrationTest : public HttpIntegrationTest, EXPECT_TRUE(known_counter_exists); EXPECT_TRUE(known_gauge_exists); EXPECT_TRUE(known_histogram_exists); + + return AssertionSuccess(); } void cleanup() { if (fake_metrics_service_connection_ != nullptr) { - fake_metrics_service_connection_->close(); - fake_metrics_service_connection_->waitForDisconnect(); + ASSERT(fake_metrics_service_connection_->close()); + ASSERT(fake_metrics_service_connection_->waitForDisconnect()); } } @@ -130,9 +140,9 @@ TEST_P(MetricsServiceIntegrationTest, BasicFlow) { {"x-lyft-user-id", "123"}}; sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0); - waitForMetricsServiceConnection(); - waitForMetricsStream(); - waitForMetricsRequest(); + ASSERT_TRUE(waitForMetricsServiceConnection()); + ASSERT_TRUE(waitForMetricsStream()); + ASSERT_TRUE(waitForMetricsRequest()); // Send an empty response and end the stream. This should never happen but make sure nothing // breaks and we make a new stream on a follow up request. From d7cb98ed22e29562415b3f3fd0bd7026bcad2cb2 Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Tue, 24 Jul 2018 13:19:21 -0400 Subject: [PATCH 3/6] Use RELEASE_ASSERT in tests that aren't ready to propagate timeout errors. Signed-off-by: Michael Behr --- .../grpc/grpc_client_integration_test.cc | 3 +- .../grpc_client_integration_test_harness.h | 33 +++++++++++++------ .../grpc_access_log_integration_test.cc | 6 ++-- .../http/jwt_authn/filter_integration_test.cc | 22 +++++++++---- .../filters/http/lua/lua_integration_test.cc | 12 ++++--- .../squash/squash_filter_integration_test.cc | 17 +++++++--- ...le_based_metadata_grpc_credentials_test.cc | 3 +- .../metrics_service_integration_test.cc | 6 ++-- test/integration/ads_integration_test.cc | 12 ++++--- test/integration/h1_capture_fuzz_test.cc | 11 +++++-- test/integration/hds_integration_test.cc | 13 +++++--- test/integration/header_integration_test.cc | 19 +++++++---- test/integration/http2_integration_test.cc | 6 ++-- test/integration/http_integration.cc | 18 ++++++---- .../idle_timeout_integration_test.cc | 10 ++++-- .../load_stats_integration_test.cc | 28 +++++++++++----- .../integration/ratelimit_integration_test.cc | 29 +++++++++++----- .../integration/tcp_proxy_integration_test.cc | 3 +- 18 files changed, 172 insertions(+), 79 deletions(-) diff --git a/test/common/grpc/grpc_client_integration_test.cc b/test/common/grpc/grpc_client_integration_test.cc index c60a4273cb2b0..30d92a2abe12a 100644 --- a/test/common/grpc/grpc_client_integration_test.cc +++ b/test/common/grpc/grpc_client_integration_test.cc @@ -352,7 +352,8 @@ TEST_P(GrpcSslClientIntegrationTest, BasicSslRequestWithClientCert) { class GrpcAccessTokenClientIntegrationTest : public GrpcSslClientIntegrationTest { public: void expectExtraHeaders(FakeStream& fake_stream) override { - ASSERT(fake_stream.waitForHeadersComplete()); + AssertionResult result = fake_stream.waitForHeadersComplete(); + RELEASE_ASSERT(result, result.message()); Http::TestHeaderMapImpl stream_headers(fake_stream.headers()); if (access_token_value_ != "") { if (access_token_value_2_ == "") { diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index 72252c0bca109..3dd69fd2d8373 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -82,7 +82,9 @@ class HelloworldStream : public MockAsyncStreamCallbacks grpc_stream_->sendMessage(request_msg, end_stream); helloworld::HelloRequest received_msg; - ASSERT(fake_stream_->waitForGrpcMessage(dispatcher_helper_.dispatcher_, received_msg)); + AssertionResult result = + fake_stream_->waitForGrpcMessage(dispatcher_helper_.dispatcher_, received_msg); + RELEASE_ASSERT(result, result.message()); EXPECT_THAT(request_msg, ProtoEq(received_msg)); } @@ -162,7 +164,8 @@ class HelloworldStream : public MockAsyncStreamCallbacks void closeStream() { grpc_stream_->closeStream(); - ASSERT(fake_stream_->waitForEndStream(dispatcher_helper_.dispatcher_)); + AssertionResult result = fake_stream_->waitForEndStream(dispatcher_helper_.dispatcher_); + RELEASE_ASSERT(result, result.message()); } DispatcherHelper& dispatcher_helper_; @@ -224,8 +227,10 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { void TearDown() override { if (fake_connection_) { - ASSERT(fake_connection_->close()); - ASSERT(fake_connection_->waitForDisconnect()); + AssertionResult result = fake_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); fake_connection_.reset(); } } @@ -291,7 +296,8 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { } void expectInitialHeaders(FakeStream& fake_stream, const TestMetadata& initial_metadata) { - ASSERT(fake_stream.waitForHeadersComplete()); + AssertionResult result = fake_stream.waitForHeadersComplete(); + RELEASE_ASSERT(result, result.message()); Http::TestHeaderMapImpl stream_headers(fake_stream.headers()); EXPECT_EQ("POST", stream_headers.get_(":method")); EXPECT_EQ("/helloworld.Greeter/SayHello", stream_headers.get_(":path")); @@ -333,10 +339,13 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { EXPECT_NE(request->grpc_request_, nullptr); if (!fake_connection_) { - ASSERT(fake_upstream_->waitForHttpConnection(dispatcher_, &fake_connection_)); + AssertionResult result = + fake_upstream_->waitForHttpConnection(dispatcher_, &fake_connection_); + RELEASE_ASSERT(result, result.message()); } fake_streams_.emplace_back(); - ASSERT(fake_connection_->waitForNewStream(dispatcher_, &fake_streams_.back())); + AssertionResult result = fake_connection_->waitForNewStream(dispatcher_, &fake_streams_.back()); + RELEASE_ASSERT(result, result.message()); auto& fake_stream = *fake_streams_.back(); request->fake_stream_ = &fake_stream; @@ -344,7 +353,8 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { expectExtraHeaders(fake_stream); helloworld::HelloRequest received_msg; - ASSERT(fake_stream.waitForGrpcMessage(dispatcher_, received_msg)); + result = fake_stream.waitForGrpcMessage(dispatcher_, received_msg); + RELEASE_ASSERT(result, result.message()); EXPECT_THAT(request_msg, ProtoEq(received_msg)); return request; @@ -363,10 +373,13 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { EXPECT_NE(stream->grpc_stream_, nullptr); if (!fake_connection_) { - ASSERT(fake_upstream_->waitForHttpConnection(dispatcher_, &fake_connection_)); + AssertionResult result = + fake_upstream_->waitForHttpConnection(dispatcher_, &fake_connection_); + RELEASE_ASSERT(result, result.message()); } fake_streams_.emplace_back(); - ASSERT(fake_connection_->waitForNewStream(dispatcher_, &fake_streams_.back())); + AssertionResult result = fake_connection_->waitForNewStream(dispatcher_, &fake_streams_.back()); + RELEASE_ASSERT(result, result.message()); auto& fake_stream = *fake_streams_.back(); stream->fake_stream_ = &fake_stream; diff --git a/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc b/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc index 0bd519c3a0f4c..ea145c4534506 100644 --- a/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc +++ b/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc @@ -90,8 +90,10 @@ class AccessLogIntegrationTest : public HttpIntegrationTest, void cleanup() { if (fake_access_log_connection_ != nullptr) { - ASSERT(fake_access_log_connection_->close()); - ASSERT(fake_access_log_connection_->waitForDisconnect()); + AssertionResult result = fake_access_log_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_access_log_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } } diff --git a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc index d0178216b4bb4..afe1f8c27a956 100644 --- a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc @@ -109,9 +109,13 @@ class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest { } void waitForJwksResponse(const std::string& status, const std::string& jwks_body) { - ASSERT(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_jwks_connection_)); - ASSERT(fake_jwks_connection_->waitForNewStream(*dispatcher_, &jwks_request_)); - ASSERT(jwks_request_->waitForEndStream(*dispatcher_)); + AssertionResult result = + fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_jwks_connection_); + RELEASE_ASSERT(result, result.message()); + result = fake_jwks_connection_->waitForNewStream(*dispatcher_, &jwks_request_); + RELEASE_ASSERT(result, result.message()); + result = jwks_request_->waitForEndStream(*dispatcher_); + RELEASE_ASSERT(result, result.message()); Http::TestHeaderMapImpl response_headers{{":status", status}}; jwks_request_->encodeHeaders(response_headers, false); @@ -122,12 +126,16 @@ class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest { void cleanup() { codec_client_->close(); if (fake_jwks_connection_ != nullptr) { - ASSERT(fake_jwks_connection_->close()); - ASSERT(fake_jwks_connection_->waitForDisconnect()); + AssertionResult result = fake_jwks_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_jwks_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } if (fake_upstream_connection_ != nullptr) { - ASSERT(fake_upstream_connection_->close()); - ASSERT(fake_upstream_connection_->waitForDisconnect()); + AssertionResult result = fake_upstream_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_upstream_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } } diff --git a/test/extensions/filters/http/lua/lua_integration_test.cc b/test/extensions/filters/http/lua/lua_integration_test.cc index 91485ff7efcb3..daccac54f1f59 100644 --- a/test/extensions/filters/http/lua/lua_integration_test.cc +++ b/test/extensions/filters/http/lua/lua_integration_test.cc @@ -72,12 +72,16 @@ class LuaIntegrationTest : public HttpIntegrationTest, void cleanup() { codec_client_->close(); if (fake_lua_connection_ != nullptr) { - ASSERT(fake_lua_connection_->close()); - ASSERT(fake_lua_connection_->waitForDisconnect()); + AssertionResult result = fake_lua_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_lua_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } if (fake_upstream_connection_ != nullptr) { - ASSERT(fake_upstream_connection_->close()); - ASSERT(fake_upstream_connection_->waitForDisconnect()); + AssertionResult result = fake_upstream_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_upstream_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } } diff --git a/test/extensions/filters/http/squash/squash_filter_integration_test.cc b/test/extensions/filters/http/squash/squash_filter_integration_test.cc index 1631364842e12..491f27d650392 100644 --- a/test/extensions/filters/http/squash/squash_filter_integration_test.cc +++ b/test/extensions/filters/http/squash/squash_filter_integration_test.cc @@ -22,20 +22,27 @@ class SquashFilterIntegrationTest : public HttpIntegrationTest, ~SquashFilterIntegrationTest() { if (fake_squash_connection_) { - ASSERT(fake_squash_connection_->close()); - ASSERT(fake_squash_connection_->waitForDisconnect()); + AssertionResult result = fake_squash_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_squash_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } } FakeStreamPtr sendSquash(const std::string& status, const std::string& body) { if (!fake_squash_connection_) { - ASSERT(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_squash_connection_)); + AssertionResult result = + fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_squash_connection_); + RELEASE_ASSERT(result, result.message()); } FakeStreamPtr request_stream; - ASSERT(fake_squash_connection_->waitForNewStream(*dispatcher_, &request_stream)); - ASSERT(request_stream->waitForEndStream(*dispatcher_)); + AssertionResult result = + fake_squash_connection_->waitForNewStream(*dispatcher_, &request_stream); + RELEASE_ASSERT(result, result.message()); + result = request_stream->waitForEndStream(*dispatcher_); + RELEASE_ASSERT(result, result.message()); if (body.empty()) { request_stream->encodeHeaders(Http::TestHeaderMapImpl{{":status", status}}, true); } else { diff --git a/test/extensions/grpc_credentials/file_based_metadata/file_based_metadata_grpc_credentials_test.cc b/test/extensions/grpc_credentials/file_based_metadata/file_based_metadata_grpc_credentials_test.cc index 4a41fb7ced654..dee133eaf6062 100644 --- a/test/extensions/grpc_credentials/file_based_metadata/file_based_metadata_grpc_credentials_test.cc +++ b/test/extensions/grpc_credentials/file_based_metadata/file_based_metadata_grpc_credentials_test.cc @@ -19,7 +19,8 @@ namespace { class GrpcFileBasedMetadataClientIntegrationTest : public GrpcSslClientIntegrationTest { public: void expectExtraHeaders(FakeStream& fake_stream) override { - ASSERT(fake_stream.waitForHeadersComplete()); + AssertionResult result = fake_stream.waitForHeadersComplete(); + RELEASE_ASSERT(result, result.message()); Http::TestHeaderMapImpl stream_headers(fake_stream.headers()); if (!header_value_1_.empty()) { EXPECT_EQ(header_prefix_1_ + header_value_1_, stream_headers.get_(header_key_1_)); diff --git a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc index d0a79bee9c6f8..4193a35870864 100644 --- a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc +++ b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc @@ -116,8 +116,10 @@ class MetricsServiceIntegrationTest : public HttpIntegrationTest, void cleanup() { if (fake_metrics_service_connection_ != nullptr) { - ASSERT(fake_metrics_service_connection_->close()); - ASSERT(fake_metrics_service_connection_->waitForDisconnect()); + AssertionResult result = fake_metrics_service_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_metrics_service_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } } diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index 02a319b321991..f3caf4bfea4e1 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -68,7 +68,8 @@ class AdsIntegrationBaseTest : public HttpIntegrationTest { void createAdsConnection(FakeUpstream& upstream) { ads_upstream_ = &upstream; - ASSERT(ads_upstream_->waitForHttpConnection(*dispatcher_, &ads_connection_)); + AssertionResult result = ads_upstream_->waitForHttpConnection(*dispatcher_, &ads_connection_); + RELEASE_ASSERT(result, result.message()); } void cleanUpAdsConnection() { @@ -76,8 +77,10 @@ class AdsIntegrationBaseTest : public HttpIntegrationTest { // Don't ASSERT fail if an ADS reconnect ends up unparented. ads_upstream_->set_allow_unexpected_disconnects(true); - ASSERT(ads_connection_->close()); - ASSERT(ads_connection_->waitForDisconnect()); + AssertionResult result = ads_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = ads_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); ads_connection_.reset(); } @@ -266,7 +269,8 @@ class AdsIntegrationTest : public AdsIntegrationBaseTest, AdsIntegrationBaseTest::initialize(); if (ads_stream_ == nullptr) { createAdsConnection(*(fake_upstreams_[1])); - ASSERT(ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_)); + AssertionResult result = ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_); + RELEASE_ASSERT(result, result.message()); ads_stream_->startGrpcStream(); } } diff --git a/test/integration/h1_capture_fuzz_test.cc b/test/integration/h1_capture_fuzz_test.cc index 4de4e3de70735..cc778c9a01d3d 100644 --- a/test/integration/h1_capture_fuzz_test.cc +++ b/test/integration/h1_capture_fuzz_test.cc @@ -48,7 +48,10 @@ class H1FuzzIntegrationTest : public HttpIntegrationTest { tcp_client->close(); return; } - ASSERT(fake_upstream_connection->write(event.upstream_send_bytes())); + { + AssertionResult result = fake_upstream_connection->write(event.upstream_send_bytes()); + RELEASE_ASSERT(result, result.message()); + } break; case test::integration::Event::kUpstreamRecvBytes: // TODO(htuch): Should we wait for some data? @@ -60,9 +63,11 @@ class H1FuzzIntegrationTest : public HttpIntegrationTest { } if (fake_upstream_connection != nullptr) { if (fake_upstream_connection->connected()) { - ASSERT(fake_upstream_connection->close()); + AssertionResult result = fake_upstream_connection->close(); + RELEASE_ASSERT(result, result.message()); } - ASSERT(fake_upstream_connection->waitForDisconnect(true)); + AssertionResult result = fake_upstream_connection->waitForDisconnect(true); + RELEASE_ASSERT(result, result.message()); } tcp_client->close(); } diff --git a/test/integration/hds_integration_test.cc b/test/integration/hds_integration_test.cc index 8e8f435d291ac..fbc2498ec3fe6 100644 --- a/test/integration/hds_integration_test.cc +++ b/test/integration/hds_integration_test.cc @@ -53,8 +53,11 @@ class HdsIntegrationTest : public HttpIntegrationTest, } void waitForHdsStream() { - ASSERT(hds_upstream_->waitForHttpConnection(*dispatcher_, &fake_hds_connection_)); - ASSERT(fake_hds_connection_->waitForNewStream(*dispatcher_, &hds_stream_)); + AssertionResult result = + hds_upstream_->waitForHttpConnection(*dispatcher_, &fake_hds_connection_); + RELEASE_ASSERT(result, result.message()); + result = fake_hds_connection_->waitForNewStream(*dispatcher_, &hds_stream_); + RELEASE_ASSERT(result, result.message()); } void requestHealthCheckSpecifier() { @@ -68,8 +71,10 @@ class HdsIntegrationTest : public HttpIntegrationTest, void cleanupHdsConnection() { if (fake_hds_connection_ != nullptr) { - ASSERT(fake_hds_connection_->close()); - ASSERT(fake_hds_connection_->waitForDisconnect()); + AssertionResult result = fake_hds_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_hds_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } } diff --git a/test/integration/header_integration_test.cc b/test/integration/header_integration_test.cc index 2e50e8fecff15..3475c5b29676e 100644 --- a/test/integration/header_integration_test.cc +++ b/test/integration/header_integration_test.cc @@ -142,8 +142,10 @@ class HeaderIntegrationTest if (eds_connection_ != nullptr) { // Don't ASSERT fail if an EDS reconnect ends up unparented. fake_upstreams_[1]->set_allow_unexpected_disconnects(true); - ASSERT(eds_connection_->close()); - ASSERT(eds_connection_->waitForDisconnect()); + AssertionResult result = eds_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = eds_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); eds_connection_.reset(); } cleanupUpstreamAndDownstream(); @@ -309,12 +311,16 @@ class HeaderIntegrationTest void initialize() override { if (use_eds_) { pre_worker_start_test_steps_ = [this]() { - ASSERT(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &eds_connection_)); - ASSERT(eds_connection_->waitForNewStream(*dispatcher_, &eds_stream_)); + AssertionResult result = + fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &eds_connection_); + RELEASE_ASSERT(result, result.message()); + result = eds_connection_->waitForNewStream(*dispatcher_, &eds_stream_); + RELEASE_ASSERT(result, result.message()); eds_stream_->startGrpcStream(); envoy::api::v2::DiscoveryRequest discovery_request; - ASSERT(eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); + result = eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request); + RELEASE_ASSERT(result, result.message()); envoy::api::v2::DiscoveryResponse discovery_response; discovery_response.set_version_info("1"); @@ -343,7 +349,8 @@ class HeaderIntegrationTest eds_stream_->sendGrpcMessage(discovery_response); // Wait for the next request to make sure the first response was consumed. - ASSERT(eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); + result = eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request); + RELEASE_ASSERT(result, result.message()); }; } diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index 394223e2e5ad3..cc34759e4bfc9 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -400,8 +400,10 @@ Http2RingHashIntegrationTest::~Http2RingHashIntegrationTest() { codec_client_ = nullptr; } for (auto it = fake_upstream_connections_.begin(); it != fake_upstream_connections_.end(); ++it) { - ASSERT((*it)->close()); - ASSERT((*it)->waitForDisconnect()); + AssertionResult result = (*it)->close(); + RELEASE_ASSERT(result, result.message()); + result = (*it)->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } } diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index c16f469e44995..01e49f6132c64 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -220,8 +220,10 @@ void HttpIntegrationTest::cleanupUpstreamAndDownstream() { // will interpret that as an unexpected disconnect. The codec client is not // subject to the same failure mode. if (fake_upstream_connection_) { - ASSERT(fake_upstream_connection_->close()); - ASSERT(fake_upstream_connection_->waitForDisconnect()); + AssertionResult result = fake_upstream_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_upstream_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } if (codec_client_) { codec_client_->close(); @@ -231,13 +233,17 @@ void HttpIntegrationTest::cleanupUpstreamAndDownstream() { void HttpIntegrationTest::waitForNextUpstreamRequest(uint64_t upstream_index) { // If there is no upstream connection, wait for it to be established. if (!fake_upstream_connection_) { - ASSERT(fake_upstreams_[upstream_index]->waitForHttpConnection(*dispatcher_, - &fake_upstream_connection_)); + AssertionResult result = fake_upstreams_[upstream_index]->waitForHttpConnection( + *dispatcher_, &fake_upstream_connection_); + RELEASE_ASSERT(result, result.message()); } // Wait for the next stream on the upstream connection. - ASSERT(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + AssertionResult result = + fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_); + RELEASE_ASSERT(result, result.message()); // Wait for the stream to be completely received. - ASSERT(upstream_request_->waitForEndStream(*dispatcher_)); + result = upstream_request_->waitForEndStream(*dispatcher_); + RELEASE_ASSERT(result, result.message()); } void HttpIntegrationTest::testRouterRequestAndResponseWithBody( diff --git a/test/integration/idle_timeout_integration_test.cc b/test/integration/idle_timeout_integration_test.cc index 9a62cb46ac221..adc526280f44f 100644 --- a/test/integration/idle_timeout_integration_test.cc +++ b/test/integration/idle_timeout_integration_test.cc @@ -37,9 +37,13 @@ class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { {":authority", "host"}}); request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); - ASSERT(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); - ASSERT(upstream_request_->waitForHeadersComplete()); + AssertionResult result = + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_); + RELEASE_ASSERT(result, result.message()); + result = fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_); + RELEASE_ASSERT(result, result.message()); + result = upstream_request_->waitForHeadersComplete(); + RELEASE_ASSERT(result, result.message()); return response; } diff --git a/test/integration/load_stats_integration_test.cc b/test/integration/load_stats_integration_test.cc index 9829af77c8fb9..d746e31f8c958 100644 --- a/test/integration/load_stats_integration_test.cc +++ b/test/integration/load_stats_integration_test.cc @@ -147,8 +147,11 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, } void waitForLoadStatsStream() { - ASSERT(load_report_upstream_->waitForHttpConnection(*dispatcher_, &fake_loadstats_connection_)); - ASSERT(fake_loadstats_connection_->waitForNewStream(*dispatcher_, &loadstats_stream_)); + AssertionResult result = + load_report_upstream_->waitForHttpConnection(*dispatcher_, &fake_loadstats_connection_); + RELEASE_ASSERT(result, result.message()); + result = fake_loadstats_connection_->waitForNewStream(*dispatcher_, &loadstats_stream_); + RELEASE_ASSERT(result, result.message()); } void @@ -218,7 +221,9 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, // merge until all the expected load has been reported. do { envoy::service::load_stats::v2::LoadStatsRequest local_loadstats_request; - ASSERT(loadstats_stream_->waitForGrpcMessage(*dispatcher_, local_loadstats_request)); + AssertionResult result = + loadstats_stream_->waitForGrpcMessage(*dispatcher_, local_loadstats_request); + RELEASE_ASSERT(result, result.message()); // Sanity check and clear the measured load report interval. for (auto& cluster_stats : *local_loadstats_request.mutable_cluster_stats()) { const uint32_t actual_load_report_interval_ms = @@ -244,10 +249,13 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, } void waitForUpstreamResponse(uint32_t endpoint_index, uint32_t response_code = 200) { - ASSERT(service_upstream_[endpoint_index]->waitForHttpConnection(*dispatcher_, - &fake_upstream_connection_)); - ASSERT(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); - ASSERT(upstream_request_->waitForEndStream(*dispatcher_)); + AssertionResult result = service_upstream_[endpoint_index]->waitForHttpConnection( + *dispatcher_, &fake_upstream_connection_); + RELEASE_ASSERT(result, result.message()); + result = fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_); + RELEASE_ASSERT(result, result.message()); + result = upstream_request_->waitForEndStream(*dispatcher_); + RELEASE_ASSERT(result, result.message()); upstream_request_->encodeHeaders( Http::TestHeaderMapImpl{{":status", std::to_string(response_code)}}, false); @@ -293,8 +301,10 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, void cleanupLoadStatsConnection() { if (fake_loadstats_connection_ != nullptr) { - ASSERT(fake_loadstats_connection_->close()); - ASSERT(fake_loadstats_connection_->waitForDisconnect()); + AssertionResult result = fake_loadstats_connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_loadstats_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } } diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index d44ac4bc7b473..97a754f5596fb 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -74,11 +74,16 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, } void waitForRatelimitRequest() { - ASSERT(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_ratelimit_connection_)); - ASSERT(fake_ratelimit_connection_->waitForNewStream(*dispatcher_, &ratelimit_request_)); + AssertionResult result = + fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_ratelimit_connection_); + RELEASE_ASSERT(result, result.message()); + result = fake_ratelimit_connection_->waitForNewStream(*dispatcher_, &ratelimit_request_); + RELEASE_ASSERT(result, result.message()); envoy::service::ratelimit::v2::RateLimitRequest request_msg; - ASSERT(ratelimit_request_->waitForGrpcMessage(*dispatcher_, request_msg)); - ASSERT(ratelimit_request_->waitForEndStream(*dispatcher_)); + result = ratelimit_request_->waitForGrpcMessage(*dispatcher_, request_msg); + RELEASE_ASSERT(result, result.message()); + result = ratelimit_request_->waitForEndStream(*dispatcher_); + RELEASE_ASSERT(result, result.message()); EXPECT_STREQ("POST", ratelimit_request_->headers().Method()->value().c_str()); if (useDataPlaneProto()) { EXPECT_STREQ("/envoy.service.ratelimit.v2.RateLimitService/ShouldRateLimit", @@ -98,9 +103,13 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, } void waitForSuccessfulUpstreamResponse() { - ASSERT(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); - ASSERT(upstream_request_->waitForEndStream(*dispatcher_)); + AssertionResult result = + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_); + RELEASE_ASSERT(result, result.message()); + result = fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_); + RELEASE_ASSERT(result, result.message()); + result = upstream_request_->waitForEndStream(*dispatcher_); + RELEASE_ASSERT(result, result.message()); upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); upstream_request_->encodeData(response_size_, true); @@ -133,9 +142,11 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, if (fake_ratelimit_connection_ != nullptr) { if (clientType() != Grpc::ClientType::GoogleGrpc) { // TODO(htuch) we should document the underlying cause of this difference and/or fix it. - ASSERT(fake_ratelimit_connection_->close()); + AssertionResult result = fake_ratelimit_connection_->close(); + RELEASE_ASSERT(result, result.message()); } - ASSERT(fake_ratelimit_connection_->waitForDisconnect()); + AssertionResult result = fake_ratelimit_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); } cleanupUpstreamAndDownstream(); } diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index c00b69690a703..cdf93dfda6690 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -397,7 +397,8 @@ void TcpProxySslIntegrationTest::setupConnections() { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); } - ASSERT(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection_)); + AssertionResult result = fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection_); + RELEASE_ASSERT(result, result.message()); } // Test proxying data in both directions with envoy doing TCP and TLS From 5d26fcad6c7409984e84c3f47df45ed174f152ce Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Wed, 25 Jul 2018 12:48:42 -0400 Subject: [PATCH 4/6] Bugfixes for fake_upstream timeouts RELEASE_ASSERT in AutonomousUpstream::createNetworkFilterChain; ABSL_MUST_USE_RESULT on FakeRawConnection::initialize Signed-off-by: Michael Behr --- test/integration/autonomous_upstream.cc | 3 ++- test/integration/fake_upstream.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/integration/autonomous_upstream.cc b/test/integration/autonomous_upstream.cc index 5d56f03809b15..c3c0a5f94f914 100644 --- a/test/integration/autonomous_upstream.cc +++ b/test/integration/autonomous_upstream.cc @@ -73,7 +73,8 @@ bool AutonomousUpstream::createNetworkFilterChain(Network::Connection& connectio shared_connections_.emplace_back(new SharedConnectionWrapper(connection, true)); AutonomousHttpConnectionPtr http_connection( new AutonomousHttpConnection(*shared_connections_.back(), stats_store_, http_type_, *this)); - http_connection->initialize(); + testing::AssertionResult result = http_connection->initialize(); + RELEASE_ASSERT(result, result.message()); http_connections_.push_back(std::move(http_connection)); return true; } diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index 5a079d5933a9f..6e107657c3771 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -438,6 +438,7 @@ class FakeRawConnection : public FakeConnectionBase { write(const std::string& data, bool end_stream = false, std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + ABSL_MUST_USE_RESULT testing::AssertionResult initialize() override { testing::AssertionResult result = shared_connection_.executeOnDispatcher([this](Network::Connection& connection) { From 22fe837fad32dcbf419c7a44ab9aa00b2c824d42 Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Thu, 26 Jul 2018 14:57:17 -0400 Subject: [PATCH 5/6] Missing VERIFY_ASSERTION in FakeUpstream::waitForRawConnection Signed-off-by: Michael Behr --- test/integration/fake_upstream.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 856eaeb715173..ab167de1653a6 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -452,7 +452,7 @@ AssertionResult FakeUpstream::waitForRawConnection(FakeRawConnectionPtr* connect } *connection = std::make_unique(consumeConnection()); } - (*connection)->initialize(); + VERIFY_ASSERTION((*connection)->initialize()); VERIFY_ASSERTION((*connection)->readDisable(false)); VERIFY_ASSERTION((*connection)->enableHalfClose(enable_half_close_)); return AssertionSuccess(); From f2a8bb457a546fa23412e53d74882cd9b899da18 Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Tue, 31 Jul 2018 11:08:26 -0400 Subject: [PATCH 6/6] Code review changes - Static constant for default timeouts - do while false for VERIFY_ASSERTION - Reference for output arguments in waitForNewStream et al. Signed-off-by: Michael Behr --- .../grpc_client_integration_test_harness.h | 10 ++- .../grpc_access_log_integration_test.cc | 4 +- .../grpc_json_transcoder_integration_test.cc | 5 +- .../http/jwt_authn/filter_integration_test.cc | 4 +- .../filters/http/lua/lua_integration_test.cc | 8 +-- .../squash/squash_filter_integration_test.cc | 4 +- .../network/thrift_proxy/integration_test.cc | 8 +-- .../metrics_service_integration_test.cc | 4 +- test/integration/ads_integration_test.cc | 10 +-- test/integration/fake_upstream.cc | 30 ++++----- test/integration/fake_upstream.h | 66 +++++++++++-------- test/integration/h1_capture_fuzz_test.cc | 2 +- test/integration/hds_integration_test.cc | 8 +-- test/integration/header_integration_test.cc | 4 +- test/integration/http2_integration_test.cc | 20 +++--- .../http2_upstream_integration_test.cc | 18 ++--- test/integration/http_integration.cc | 32 +++++---- .../idle_timeout_integration_test.cc | 4 +- test/integration/integration_test.cc | 6 +- .../load_stats_integration_test.cc | 8 +-- .../integration/ratelimit_integration_test.cc | 10 +-- .../tcp_conn_pool_integration_test.cc | 6 +- .../integration/tcp_proxy_integration_test.cc | 22 +++---- .../integration/websocket_integration_test.cc | 16 ++--- test/integration/xfcc_integration_test.cc | 4 +- test/test_common/utility.cc | 2 + test/test_common/utility.h | 6 +- 27 files changed, 166 insertions(+), 155 deletions(-) diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index 3dd69fd2d8373..b13b802b3fc4c 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -339,12 +339,11 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { EXPECT_NE(request->grpc_request_, nullptr); if (!fake_connection_) { - AssertionResult result = - fake_upstream_->waitForHttpConnection(dispatcher_, &fake_connection_); + AssertionResult result = fake_upstream_->waitForHttpConnection(dispatcher_, fake_connection_); RELEASE_ASSERT(result, result.message()); } fake_streams_.emplace_back(); - AssertionResult result = fake_connection_->waitForNewStream(dispatcher_, &fake_streams_.back()); + AssertionResult result = fake_connection_->waitForNewStream(dispatcher_, fake_streams_.back()); RELEASE_ASSERT(result, result.message()); auto& fake_stream = *fake_streams_.back(); request->fake_stream_ = &fake_stream; @@ -373,12 +372,11 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { EXPECT_NE(stream->grpc_stream_, nullptr); if (!fake_connection_) { - AssertionResult result = - fake_upstream_->waitForHttpConnection(dispatcher_, &fake_connection_); + AssertionResult result = fake_upstream_->waitForHttpConnection(dispatcher_, fake_connection_); RELEASE_ASSERT(result, result.message()); } fake_streams_.emplace_back(); - AssertionResult result = fake_connection_->waitForNewStream(dispatcher_, &fake_streams_.back()); + AssertionResult result = fake_connection_->waitForNewStream(dispatcher_, fake_streams_.back()); RELEASE_ASSERT(result, result.message()); auto& fake_stream = *fake_streams_.back(); stream->fake_stream_ = &fake_stream; diff --git a/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc b/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc index ea145c4534506..7888dfc9a1554 100644 --- a/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc +++ b/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc @@ -54,12 +54,12 @@ class AccessLogIntegrationTest : public HttpIntegrationTest, ABSL_MUST_USE_RESULT AssertionResult waitForAccessLogConnection() { - return fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_access_log_connection_); + return fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, fake_access_log_connection_); } ABSL_MUST_USE_RESULT AssertionResult waitForAccessLogStream() { - return fake_access_log_connection_->waitForNewStream(*dispatcher_, &access_log_request_); + return fake_access_log_connection_->waitForNewStream(*dispatcher_, access_log_request_); } ABSL_MUST_USE_RESULT diff --git a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc index 92188de9f01fd..a4bc68362c246 100644 --- a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc +++ b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc @@ -71,9 +71,8 @@ class GrpcJsonTranscoderIntegrationTest response = codec_client_->makeHeaderOnlyRequest(request_headers); } - ASSERT_TRUE( - fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); if (!grpc_request_messages.empty()) { ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); diff --git a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc index afe1f8c27a956..eb3f8803de900 100644 --- a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc @@ -110,9 +110,9 @@ class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest { void waitForJwksResponse(const std::string& status, const std::string& jwks_body) { AssertionResult result = - fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_jwks_connection_); + fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, fake_jwks_connection_); RELEASE_ASSERT(result, result.message()); - result = fake_jwks_connection_->waitForNewStream(*dispatcher_, &jwks_request_); + result = fake_jwks_connection_->waitForNewStream(*dispatcher_, jwks_request_); RELEASE_ASSERT(result, result.message()); result = jwks_request_->waitForEndStream(*dispatcher_); RELEASE_ASSERT(result, result.message()); diff --git a/test/extensions/filters/http/lua/lua_integration_test.cc b/test/extensions/filters/http/lua/lua_integration_test.cc index daccac54f1f59..c3b62e5ab6546 100644 --- a/test/extensions/filters/http/lua/lua_integration_test.cc +++ b/test/extensions/filters/http/lua/lua_integration_test.cc @@ -238,8 +238,8 @@ name: envoy.lua {"x-forwarded-for", "10.0.0.1"}}; auto response = codec_client_->makeHeaderOnlyRequest(request_headers); - ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_lua_connection_)); - ASSERT_TRUE(fake_lua_connection_->waitForNewStream(*dispatcher_, &lua_request_)); + ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, fake_lua_connection_)); + ASSERT_TRUE(fake_lua_connection_->waitForNewStream(*dispatcher_, lua_request_)); ASSERT_TRUE(lua_request_->waitForEndStream(*dispatcher_)); Http::TestHeaderMapImpl response_headers{{":status", "200"}, {"foo", "bar"}}; lua_request_->encodeHeaders(response_headers, false); @@ -296,8 +296,8 @@ name: envoy.lua {"x-forwarded-for", "10.0.0.1"}}; auto response = codec_client_->makeHeaderOnlyRequest(request_headers); - ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_lua_connection_)); - ASSERT_TRUE(fake_lua_connection_->waitForNewStream(*dispatcher_, &lua_request_)); + ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, fake_lua_connection_)); + ASSERT_TRUE(fake_lua_connection_->waitForNewStream(*dispatcher_, lua_request_)); ASSERT_TRUE(lua_request_->waitForEndStream(*dispatcher_)); Http::TestHeaderMapImpl response_headers{{":status", "200"}, {"foo", "bar"}}; lua_request_->encodeHeaders(response_headers, true); diff --git a/test/extensions/filters/http/squash/squash_filter_integration_test.cc b/test/extensions/filters/http/squash/squash_filter_integration_test.cc index 491f27d650392..fd8379194900f 100644 --- a/test/extensions/filters/http/squash/squash_filter_integration_test.cc +++ b/test/extensions/filters/http/squash/squash_filter_integration_test.cc @@ -33,13 +33,13 @@ class SquashFilterIntegrationTest : public HttpIntegrationTest, if (!fake_squash_connection_) { AssertionResult result = - fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_squash_connection_); + fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, fake_squash_connection_); RELEASE_ASSERT(result, result.message()); } FakeStreamPtr request_stream; AssertionResult result = - fake_squash_connection_->waitForNewStream(*dispatcher_, &request_stream); + fake_squash_connection_->waitForNewStream(*dispatcher_, request_stream); RELEASE_ASSERT(result, result.message()); result = request_stream->waitForEndStream(*dispatcher_); RELEASE_ASSERT(result, result.message()); diff --git a/test/extensions/filters/network/thrift_proxy/integration_test.cc b/test/extensions/filters/network/thrift_proxy/integration_test.cc index b162072227944..b15ea8fb4e820 100644 --- a/test/extensions/filters/network/thrift_proxy/integration_test.cc +++ b/test/extensions/filters/network/thrift_proxy/integration_test.cc @@ -181,7 +181,7 @@ TEST_P(ThriftConnManagerIntegrationTest, Success) { tcp_client->write(request_bytes_.toString()); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(request_bytes_.length(), &data)); Buffer::OwnedImpl upstream_request(data); @@ -207,7 +207,7 @@ TEST_P(ThriftConnManagerIntegrationTest, IDLException) { tcp_client->write(request_bytes_.toString()); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(request_bytes_.length(), &data)); Buffer::OwnedImpl upstream_request(data); @@ -233,7 +233,7 @@ TEST_P(ThriftConnManagerIntegrationTest, Exception) { tcp_client->write(request_bytes_.toString()); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(request_bytes_.length(), &data)); Buffer::OwnedImpl upstream_request(data); @@ -259,7 +259,7 @@ TEST_P(ThriftConnManagerIntegrationTest, Oneway) { tcp_client->write(request_bytes_.toString()); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(request_bytes_.length(), &data)); Buffer::OwnedImpl upstream_request(data); diff --git a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc index 4193a35870864..fb27999178082 100644 --- a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc +++ b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc @@ -53,13 +53,13 @@ class MetricsServiceIntegrationTest : public HttpIntegrationTest, ABSL_MUST_USE_RESULT AssertionResult waitForMetricsServiceConnection() { return fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, - &fake_metrics_service_connection_); + fake_metrics_service_connection_); } ABSL_MUST_USE_RESULT AssertionResult waitForMetricsStream() { return fake_metrics_service_connection_->waitForNewStream(*dispatcher_, - &metrics_service_request_); + metrics_service_request_); } ABSL_MUST_USE_RESULT diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index 15ddb8fcb939d..1f09256426c63 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -68,7 +68,7 @@ class AdsIntegrationBaseTest : public HttpIntegrationTest { void createAdsConnection(FakeUpstream& upstream) { ads_upstream_ = &upstream; - AssertionResult result = ads_upstream_->waitForHttpConnection(*dispatcher_, &ads_connection_); + AssertionResult result = ads_upstream_->waitForHttpConnection(*dispatcher_, ads_connection_); RELEASE_ASSERT(result, result.message()); } @@ -270,7 +270,7 @@ class AdsIntegrationTest : public AdsIntegrationBaseTest, AdsIntegrationBaseTest::initialize(); if (ads_stream_ == nullptr) { createAdsConnection(*(fake_upstreams_[1])); - AssertionResult result = ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_); + AssertionResult result = ads_connection_->waitForNewStream(*dispatcher_, ads_stream_); RELEASE_ASSERT(result, result.message()); ads_stream_->startGrpcStream(); } @@ -589,7 +589,7 @@ INSTANTIATE_TEST_CASE_P(IpVersionsClientType, AdsFailIntegrationTest, TEST_P(AdsFailIntegrationTest, ConnectDisconnect) { initialize(); createAdsConnection(*fake_upstreams_[1]); - ASSERT_TRUE(ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_)); + ASSERT_TRUE(ads_connection_->waitForNewStream(*dispatcher_, ads_stream_)); ads_stream_->startGrpcStream(); ads_stream_->finishGrpcStream(Grpc::Status::Internal); } @@ -642,7 +642,7 @@ INSTANTIATE_TEST_CASE_P(IpVersionsClientType, AdsConfigIntegrationTest, TEST_P(AdsConfigIntegrationTest, EdsClusterWithAdsConfigSource) { initialize(); createAdsConnection(*fake_upstreams_[1]); - ASSERT_TRUE(ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_)); + ASSERT_TRUE(ads_connection_->waitForNewStream(*dispatcher_, ads_stream_)); ads_stream_->startGrpcStream(); ads_stream_->finishGrpcStream(Grpc::Status::Ok); } @@ -663,7 +663,7 @@ TEST_P(AdsIntegrationTest, XdsBatching) { pre_worker_start_test_steps_ = [this]() { createAdsConnection(*fake_upstreams_.back()); - ASSERT_TRUE(ads_connection_->waitForNewStream(*dispatcher_, &ads_stream_)); + ASSERT_TRUE(ads_connection_->waitForNewStream(*dispatcher_, ads_stream_)); ads_stream_->startGrpcStream(); EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "", diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index ab167de1653a6..6aa74011e4f1d 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -271,7 +271,7 @@ AssertionResult FakeConnectionBase::waitForHalfClose(bool ignore_spurious_events } AssertionResult FakeHttpConnection::waitForNewStream(Event::Dispatcher& client_dispatcher, - FakeStreamPtr* stream, + FakeStreamPtr& stream, bool ignore_spurious_events, milliseconds timeout) { auto end_time = std::chrono::steady_clock::now() + timeout; @@ -296,7 +296,7 @@ AssertionResult FakeHttpConnection::waitForNewStream(Event::Dispatcher& client_d if (new_streams_.empty()) { return AssertionFailure() << "Expected new stream event, but got a different event."; } - *stream = std::move(new_streams_.front()); + stream = std::move(new_streams_.front()); new_streams_.pop_front(); return AssertionSuccess(); } @@ -382,7 +382,7 @@ void FakeUpstream::threadRoutine() { } AssertionResult FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher, - FakeHttpConnectionPtr* connection, + FakeHttpConnectionPtr& connection, milliseconds timeout) { auto end_time = std::chrono::steady_clock::now() + timeout; { @@ -401,18 +401,18 @@ AssertionResult FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_di if (new_connections_.empty()) { return AssertionFailure() << "Got a new connection event, but didn't create a connection."; } - *connection = + connection = std::make_unique(consumeConnection(), stats_store_, http_type_); } - VERIFY_ASSERTION((*connection)->initialize()); - VERIFY_ASSERTION((*connection)->readDisable(false)); + VERIFY_ASSERTION(connection->initialize()); + VERIFY_ASSERTION(connection->readDisable(false)); return AssertionSuccess(); } AssertionResult FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher, std::vector>& upstreams, - FakeHttpConnectionPtr* connection, milliseconds timeout) { + FakeHttpConnectionPtr& connection, milliseconds timeout) { auto end_time = std::chrono::steady_clock::now() + timeout; while (std::chrono::steady_clock::now() < end_time) { for (auto it = upstreams.begin(); it != upstreams.end(); ++it) { @@ -426,11 +426,11 @@ FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher, // Run the client dispatcher since we may need to process window updates, etc. client_dispatcher.run(Event::Dispatcher::RunType::NonBlock); } else { - *connection = std::make_unique( + connection = std::make_unique( upstream.consumeConnection(), upstream.stats_store_, upstream.http_type_); lock.release(); - VERIFY_ASSERTION((*connection)->initialize()); - VERIFY_ASSERTION((*connection)->readDisable(false)); + VERIFY_ASSERTION(connection->initialize()); + VERIFY_ASSERTION(connection->readDisable(false)); return AssertionSuccess(); } } @@ -438,7 +438,7 @@ FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher, return AssertionFailure() << "Timed out waiting for HTTP connection."; } -AssertionResult FakeUpstream::waitForRawConnection(FakeRawConnectionPtr* connection, +AssertionResult FakeUpstream::waitForRawConnection(FakeRawConnectionPtr& connection, milliseconds timeout) { { Thread::LockGuard lock(lock_); @@ -450,11 +450,11 @@ AssertionResult FakeUpstream::waitForRawConnection(FakeRawConnectionPtr* connect if (new_connections_.empty()) { return AssertionFailure() << "Timed out waiting for raw connection"; } - *connection = std::make_unique(consumeConnection()); + connection = std::make_unique(consumeConnection()); } - VERIFY_ASSERTION((*connection)->initialize()); - VERIFY_ASSERTION((*connection)->readDisable(false)); - VERIFY_ASSERTION((*connection)->enableHalfClose(enable_half_close_)); + VERIFY_ASSERTION(connection->initialize()); + VERIFY_ASSERTION(connection->readDisable(false)); + VERIFY_ASSERTION(connection->enableHalfClose(enable_half_close_)); return AssertionSuccess(); } diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index 6e107657c3771..0c98a9efbd7f7 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -56,20 +56,24 @@ class FakeStream : public Http::StreamDecoder, const Http::HeaderMap& headers() { return *headers_; } void setAddServedByHeader(bool add_header) { add_served_by_header_ = add_header; } const Http::HeaderMapPtr& trailers() { return trailers_; } + ABSL_MUST_USE_RESULT testing::AssertionResult - waitForHeadersComplete(std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + waitForHeadersComplete(std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + ABSL_MUST_USE_RESULT testing::AssertionResult waitForData(Event::Dispatcher& client_dispatcher, uint64_t body_length, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + ABSL_MUST_USE_RESULT testing::AssertionResult waitForEndStream(Event::Dispatcher& client_dispatcher, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + ABSL_MUST_USE_RESULT testing::AssertionResult - waitForReset(std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + waitForReset(std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); // gRPC convenience methods. void startGrpcStream(); @@ -94,7 +98,7 @@ class FakeStream : public Http::StreamDecoder, template ABSL_MUST_USE_RESULT testing::AssertionResult waitForGrpcMessage(Event::Dispatcher& client_dispatcher, T& message, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)) { + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) { auto end_time = std::chrono::steady_clock::now() + timeout; ENVOY_LOG(debug, "Waiting for gRPC message..."); if (!decoded_grpc_frames_.empty()) { @@ -226,7 +230,7 @@ class SharedConnectionWrapper : public Network::ConnectionCallbacks { ABSL_MUST_USE_RESULT testing::AssertionResult executeOnDispatcher(std::function f, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)) { + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) { Thread::LockGuard lock(lock_); if (disconnected_) { return testing::AssertionSuccess(); @@ -323,12 +327,14 @@ class FakeConnectionBase : public Logger::Loggable { ASSERT(disconnect_callback_handle_ != nullptr); shared_connection_.removeDisconnectCallback(disconnect_callback_handle_); } + ABSL_MUST_USE_RESULT - testing::AssertionResult - close(std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + testing::AssertionResult close(std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + ABSL_MUST_USE_RESULT testing::AssertionResult - readDisable(bool disable, std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + readDisable(bool disable, std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + // By default waitForDisconnect and waitForHalfClose assume the next event is // a disconnect and return an AssertionFailure if an unexpected event occurs. // If a caller truly wishes to wait until disconnect, set @@ -336,11 +342,12 @@ class FakeConnectionBase : public Logger::Loggable { ABSL_MUST_USE_RESULT testing::AssertionResult waitForDisconnect(bool ignore_spurious_events = false, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + ABSL_MUST_USE_RESULT testing::AssertionResult waitForHalfClose(bool ignore_spurious_events = false, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); ABSL_MUST_USE_RESULT virtual testing::AssertionResult initialize() { @@ -351,8 +358,7 @@ class FakeConnectionBase : public Logger::Loggable { } ABSL_MUST_USE_RESULT testing::AssertionResult - enableHalfClose(bool enabled, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + enableHalfClose(bool enabled, std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); SharedConnectionWrapper& shared_connection() { return shared_connection_; } // The same caveats apply here as in SharedConnectionWrapper::connection(). Network::Connection& connection() const { return shared_connection_.connection(); } @@ -378,14 +384,16 @@ class FakeHttpConnection : public Http::ServerConnectionCallbacks, public FakeCo enum class Type { HTTP1, HTTP2 }; FakeHttpConnection(SharedConnectionWrapper& shared_connection, Stats::Store& store, Type type); + // By default waitForNewStream assumes the next event is a new stream and // returns AssertionFaliure if an unexpected event occurs. If a caller truly - // wishes to wait for a new stream, set ignore_spurious_events = true. + // wishes to wait for a new stream, set ignore_spurious_events = true. Returns + // the new stream via the stream argument. ABSL_MUST_USE_RESULT testing::AssertionResult - waitForNewStream(Event::Dispatcher& client_dispatcher, FakeStreamPtr* stream, + waitForNewStream(Event::Dispatcher& client_dispatcher, FakeStreamPtr& stream, bool ignore_spurious_events = false, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); // Http::ServerConnectionCallbacks Http::StreamDecoder& newStream(Http::StreamEncoder& response_encoder) override; @@ -423,7 +431,8 @@ class FakeRawConnection : public FakeConnectionBase { ABSL_MUST_USE_RESULT testing::AssertionResult waitForData(uint64_t num_bytes, std::string* data = nullptr, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + // Wait until data_validator returns true. // example usage: // std::string data; @@ -432,11 +441,11 @@ class FakeRawConnection : public FakeConnectionBase { ABSL_MUST_USE_RESULT testing::AssertionResult waitForData(const ValidatorFunction& data_validator, std::string* data = nullptr, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + ABSL_MUST_USE_RESULT - testing::AssertionResult - write(const std::string& data, bool end_stream = false, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + testing::AssertionResult write(const std::string& data, bool end_stream = false, + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); ABSL_MUST_USE_RESULT testing::AssertionResult initialize() override { @@ -489,14 +498,17 @@ class FakeUpstream : Logger::Loggable, ~FakeUpstream(); FakeHttpConnection::Type httpType() { return http_type_; } + + // Returns the new connection via the connection argument. ABSL_MUST_USE_RESULT testing::AssertionResult - waitForHttpConnection(Event::Dispatcher& client_dispatcher, FakeHttpConnectionPtr* connection, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + waitForHttpConnection(Event::Dispatcher& client_dispatcher, FakeHttpConnectionPtr& connection, + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); + ABSL_MUST_USE_RESULT testing::AssertionResult - waitForRawConnection(FakeRawConnectionPtr* connection, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + waitForRawConnection(FakeRawConnectionPtr& connection, + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); Network::Address::InstanceConstSharedPtr localAddress() const { return socket_->localAddress(); } // Wait for one of the upstreams to receive a connection @@ -504,8 +516,8 @@ class FakeUpstream : Logger::Loggable, static testing::AssertionResult waitForHttpConnection(Event::Dispatcher& client_dispatcher, std::vector>& upstreams, - FakeHttpConnectionPtr* connection, - std::chrono::milliseconds timeout = std::chrono::milliseconds(10000)); + FakeHttpConnectionPtr& connection, + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); // Network::FilterChainManager const Network::FilterChain* findFilterChain(const Network::ConnectionSocket&) const override { diff --git a/test/integration/h1_capture_fuzz_test.cc b/test/integration/h1_capture_fuzz_test.cc index cc778c9a01d3d..fe7d41bf0c51c 100644 --- a/test/integration/h1_capture_fuzz_test.cc +++ b/test/integration/h1_capture_fuzz_test.cc @@ -37,7 +37,7 @@ class H1FuzzIntegrationTest : public HttpIntegrationTest { break; case test::integration::Event::kUpstreamSendBytes: if (fake_upstream_connection == nullptr) { - if (!fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection, max_wait_ms_)) { + if (!fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection, max_wait_ms_)) { // If we timed out, we fail out. tcp_client->close(); return; diff --git a/test/integration/hds_integration_test.cc b/test/integration/hds_integration_test.cc index fbc2498ec3fe6..ae0cd1d189248 100644 --- a/test/integration/hds_integration_test.cc +++ b/test/integration/hds_integration_test.cc @@ -54,9 +54,9 @@ class HdsIntegrationTest : public HttpIntegrationTest, void waitForHdsStream() { AssertionResult result = - hds_upstream_->waitForHttpConnection(*dispatcher_, &fake_hds_connection_); + hds_upstream_->waitForHttpConnection(*dispatcher_, fake_hds_connection_); RELEASE_ASSERT(result, result.message()); - result = fake_hds_connection_->waitForNewStream(*dispatcher_, &hds_stream_); + result = fake_hds_connection_->waitForNewStream(*dispatcher_, hds_stream_); RELEASE_ASSERT(result, result.message()); } @@ -103,8 +103,8 @@ TEST_P(HdsIntegrationTest, Simple) { server_health_check_specifier.mutable_interval()->set_nanos(500000000); // 500ms // Server <--> Envoy - ASSERT_TRUE(hds_upstream_->waitForHttpConnection(*dispatcher_, &fake_hds_connection_)); - ASSERT_TRUE(fake_hds_connection_->waitForNewStream(*dispatcher_, &hds_stream_)); + ASSERT_TRUE(hds_upstream_->waitForHttpConnection(*dispatcher_, fake_hds_connection_)); + ASSERT_TRUE(fake_hds_connection_->waitForNewStream(*dispatcher_, hds_stream_)); ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, envoy_msg)); EXPECT_EQ(0, test_server_->counter("hds_delegate.requests")->value()); diff --git a/test/integration/header_integration_test.cc b/test/integration/header_integration_test.cc index 3475c5b29676e..e25e7b79a4f62 100644 --- a/test/integration/header_integration_test.cc +++ b/test/integration/header_integration_test.cc @@ -312,9 +312,9 @@ class HeaderIntegrationTest if (use_eds_) { pre_worker_start_test_steps_ = [this]() { AssertionResult result = - fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &eds_connection_); + fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, eds_connection_); RELEASE_ASSERT(result, result.message()); - result = eds_connection_->waitForNewStream(*dispatcher_, &eds_stream_); + result = eds_connection_->waitForNewStream(*dispatcher_, eds_stream_); RELEASE_ASSERT(result, result.message()); eds_stream_->startGrpcStream(); diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index cc34759e4bfc9..35359c8b6ab73 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -253,8 +253,8 @@ TEST_P(Http2IntegrationTest, IdleTimeoutWithSimultaneousRequests) { encoder1 = &encoder_decoder.first; auto response1 = std::move(encoder_decoder.second); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection1)); - ASSERT_TRUE(fake_upstream_connection1->waitForNewStream(*dispatcher_, &upstream_request1)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection1)); + ASSERT_TRUE(fake_upstream_connection1->waitForNewStream(*dispatcher_, upstream_request1)); // Start request 2 auto encoder_decoder2 = @@ -264,8 +264,8 @@ TEST_P(Http2IntegrationTest, IdleTimeoutWithSimultaneousRequests) { {":authority", "host"}}); encoder2 = &encoder_decoder2.first; auto response2 = std::move(encoder_decoder2.second); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection2)); - ASSERT_TRUE(fake_upstream_connection2->waitForNewStream(*dispatcher_, &upstream_request2)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection2)); + ASSERT_TRUE(fake_upstream_connection2->waitForNewStream(*dispatcher_, upstream_request2)); // Finish request 1 codec_client_->sendData(*encoder1, request1_bytes, true); @@ -325,8 +325,8 @@ void Http2IntegrationTest::simultaneousRequest(int32_t request1_bytes, int32_t r encoder1 = &encoder_decoder.first; auto response1 = std::move(encoder_decoder.second); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection1)); - ASSERT_TRUE(fake_upstream_connection1->waitForNewStream(*dispatcher_, &upstream_request1)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection1)); + ASSERT_TRUE(fake_upstream_connection1->waitForNewStream(*dispatcher_, upstream_request1)); // Start request 2 auto encoder_decoder2 = @@ -336,8 +336,8 @@ void Http2IntegrationTest::simultaneousRequest(int32_t request1_bytes, int32_t r {":authority", "host"}}); encoder2 = &encoder_decoder2.first; auto response2 = std::move(encoder_decoder2.second); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection2)); - ASSERT_TRUE(fake_upstream_connection2->waitForNewStream(*dispatcher_, &upstream_request2)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection2)); + ASSERT_TRUE(fake_upstream_connection2->waitForNewStream(*dispatcher_, upstream_request2)); // Finish request 1 codec_client_->sendData(*encoder1, request1_bytes, true); @@ -439,12 +439,12 @@ void Http2RingHashIntegrationTest::sendMultipleRequests( for (uint32_t i = 0; i < num_requests; ++i) { FakeHttpConnectionPtr fake_upstream_connection; ASSERT_TRUE(FakeUpstream::waitForHttpConnection(*dispatcher_, fake_upstreams_, - &fake_upstream_connection)); + fake_upstream_connection)); // As data and streams are interwoven, make sure waitForNewStream() // ignores incoming data and waits for actual stream establishment. upstream_requests.emplace_back(); ASSERT_TRUE( - fake_upstream_connection->waitForNewStream(*dispatcher_, &upstream_requests.back(), true)); + fake_upstream_connection->waitForNewStream(*dispatcher_, upstream_requests.back(), true)); upstream_requests.back()->setAddServedByHeader(true); fake_upstream_connections_.push_back(std::move(fake_upstream_connection)); } diff --git a/test/integration/http2_upstream_integration_test.cc b/test/integration/http2_upstream_integration_test.cc index 2445c9fa04bae..d1401a5f0baac 100644 --- a/test/integration/http2_upstream_integration_test.cc +++ b/test/integration/http2_upstream_integration_test.cc @@ -97,8 +97,8 @@ void Http2UpstreamIntegrationTest::bidirectionalStreaming(uint32_t bytes) { {":authority", "host"}}); auto response = std::move(encoder_decoder.second); request_encoder_ = &encoder_decoder.first; - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); // Send part of the request body and ensure it is received upstream. codec_client_->sendData(*request_encoder_, bytes, false); @@ -138,8 +138,8 @@ TEST_P(Http2UpstreamIntegrationTest, BidirectionalStreamingReset) { {":authority", "host"}}); auto response = std::move(encoder_decoder.second); request_encoder_ = &encoder_decoder.first; - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); // Send some request data. codec_client_->sendData(*request_encoder_, 1024, false); @@ -177,8 +177,8 @@ void Http2UpstreamIntegrationTest::simultaneousRequest(uint32_t request1_bytes, {":authority", "host"}}); Http::StreamEncoder* encoder1 = &encoder_decoder1.first; auto response1 = std::move(encoder_decoder1.second); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request1)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request1)); // Start request 2 auto encoder_decoder2 = @@ -190,7 +190,7 @@ void Http2UpstreamIntegrationTest::simultaneousRequest(uint32_t request1_bytes, auto response2 = std::move(encoder_decoder2.second); // DO NOT SUBMIT replace other ASSERT_TRUE with ASSERT (?) // (in places like this, ASSERT_TRUE is probably fine) - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request2)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request2)); // Finish request 1 codec_client_->sendData(*encoder1, request1_bytes, true); @@ -304,12 +304,12 @@ TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) { codec_client_->sendData(*encoders[i], 0, true); } } - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); for (uint32_t i = 0; i < num_requests; ++i) { FakeStreamPtr stream; upstream_requests.emplace_back(); ASSERT_TRUE( - fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_requests.back())); + fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_requests.back())); } for (uint32_t i = 0; i < num_requests; ++i) { if (i % 15 != 0) { diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 01e49f6132c64..3f93b90aed7e4 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -234,12 +234,12 @@ void HttpIntegrationTest::waitForNextUpstreamRequest(uint64_t upstream_index) { // If there is no upstream connection, wait for it to be established. if (!fake_upstream_connection_) { AssertionResult result = fake_upstreams_[upstream_index]->waitForHttpConnection( - *dispatcher_, &fake_upstream_connection_); + *dispatcher_, fake_upstream_connection_); RELEASE_ASSERT(result, result.message()); } // Wait for the next stream on the upstream connection. AssertionResult result = - fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_); + fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_); RELEASE_ASSERT(result, result.message()); // Wait for the stream to be completely received. result = upstream_request_->waitForEndStream(*dispatcher_); @@ -456,9 +456,9 @@ void HttpIntegrationTest::testRouterUpstreamDisconnectBeforeRequestComplete() { {":authority", "host"}}); auto response = std::move(encoder_decoder.second); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); ASSERT_TRUE(fake_upstream_connection_->close()); ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); @@ -520,8 +520,8 @@ void HttpIntegrationTest::testRouterDownstreamDisconnectBeforeRequestComplete( {":scheme", "http"}, {":authority", "host"}}); auto response = std::move(encoder_decoder.second); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); codec_client_->close(); @@ -580,8 +580,8 @@ void HttpIntegrationTest::testRouterUpstreamResponseBeforeRequestComplete() { {":scheme", "http"}, {":authority", "host"}}); auto response = std::move(encoder_decoder.second); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); upstream_request_->encodeData(512, true); @@ -625,8 +625,7 @@ void HttpIntegrationTest::testRetry() { if (fake_upstreams_[0]->httpType() == FakeHttpConnection::Type::HTTP1) { ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); - ASSERT_TRUE( - fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); } else { ASSERT_TRUE(upstream_request_->waitForReset()); } @@ -677,8 +676,7 @@ void HttpIntegrationTest::testGrpcRetry() { Http::TestHeaderMapImpl{{":status", "200"}, {"grpc-status", "1"}}, false); if (fake_upstreams_[0]->httpType() == FakeHttpConnection::Type::HTTP1) { ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); - ASSERT_TRUE( - fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); } else { ASSERT_TRUE(upstream_request_->waitForReset()); } @@ -808,10 +806,10 @@ void HttpIntegrationTest::testEnvoyHandling100Continue(bool additional_continue_ {"expect", "100-continue"}}); request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); // The continue headers should arrive immediately. response->waitForContinueHeaders(); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); // Send the rest of the request. codec_client_->sendData(*request_encoder_, 10, true); @@ -880,8 +878,8 @@ void HttpIntegrationTest::testEnvoyProxying100Continue(bool continue_before_upst auto response = std::move(encoder_decoder.second); // Wait for the request headers to be received upstream. - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); if (continue_before_upstream_complete) { // This case tests sending on 100-Continue headers before the client has sent all the @@ -1414,7 +1412,7 @@ void HttpIntegrationTest::testUpstreamProtocolError() { auto response = std::move(encoder_decoder.second); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); // TODO(mattklein123): Waiting for exact amount of data is a hack. This needs to // be fixed. std::string data; diff --git a/test/integration/idle_timeout_integration_test.cc b/test/integration/idle_timeout_integration_test.cc index adc526280f44f..3006fcf25a28d 100644 --- a/test/integration/idle_timeout_integration_test.cc +++ b/test/integration/idle_timeout_integration_test.cc @@ -38,9 +38,9 @@ class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); AssertionResult result = - fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_); + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_); RELEASE_ASSERT(result, result.message()); - result = fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_); + result = fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_); RELEASE_ASSERT(result, result.message()); result = upstream_request_->waitForHeadersComplete(); RELEASE_ASSERT(result, result.message()); diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 163cc3f894df2..f5773c3532646 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -252,7 +252,7 @@ TEST_P(IntegrationTest, TestHeadWithExplicitTE) { auto tcp_client = makeTcpConnection(lookupPort("http")); tcp_client->write("HEAD / HTTP/1.1\r\nHost: host\r\n\r\n"); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData( FakeRawConnection::waitForInexactMatch("\r\n\r\n"), &data)); @@ -290,12 +290,12 @@ TEST_P(IntegrationTest, TestBind) { {":scheme", "http"}, {":authority", "host"}}, 1024); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); ASSERT_NE(fake_upstream_connection_, nullptr); std::string address = fake_upstream_connection_->connection().remoteAddress()->ip()->addressAsString(); EXPECT_EQ(address, address_string); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); ASSERT_NE(upstream_request_, nullptr); ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); diff --git a/test/integration/load_stats_integration_test.cc b/test/integration/load_stats_integration_test.cc index d746e31f8c958..a7c16e034c316 100644 --- a/test/integration/load_stats_integration_test.cc +++ b/test/integration/load_stats_integration_test.cc @@ -148,9 +148,9 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, void waitForLoadStatsStream() { AssertionResult result = - load_report_upstream_->waitForHttpConnection(*dispatcher_, &fake_loadstats_connection_); + load_report_upstream_->waitForHttpConnection(*dispatcher_, fake_loadstats_connection_); RELEASE_ASSERT(result, result.message()); - result = fake_loadstats_connection_->waitForNewStream(*dispatcher_, &loadstats_stream_); + result = fake_loadstats_connection_->waitForNewStream(*dispatcher_, loadstats_stream_); RELEASE_ASSERT(result, result.message()); } @@ -250,9 +250,9 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, void waitForUpstreamResponse(uint32_t endpoint_index, uint32_t response_code = 200) { AssertionResult result = service_upstream_[endpoint_index]->waitForHttpConnection( - *dispatcher_, &fake_upstream_connection_); + *dispatcher_, fake_upstream_connection_); RELEASE_ASSERT(result, result.message()); - result = fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_); + result = fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_); RELEASE_ASSERT(result, result.message()); result = upstream_request_->waitForEndStream(*dispatcher_); RELEASE_ASSERT(result, result.message()); diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index 97a754f5596fb..c45084e716fb2 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -75,9 +75,9 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, void waitForRatelimitRequest() { AssertionResult result = - fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_ratelimit_connection_); + fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, fake_ratelimit_connection_); RELEASE_ASSERT(result, result.message()); - result = fake_ratelimit_connection_->waitForNewStream(*dispatcher_, &ratelimit_request_); + result = fake_ratelimit_connection_->waitForNewStream(*dispatcher_, ratelimit_request_); RELEASE_ASSERT(result, result.message()); envoy::service::ratelimit::v2::RateLimitRequest request_msg; result = ratelimit_request_->waitForGrpcMessage(*dispatcher_, request_msg); @@ -104,9 +104,9 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, void waitForSuccessfulUpstreamResponse() { AssertionResult result = - fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_); + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_); RELEASE_ASSERT(result, result.message()); - result = fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_); + result = fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_); RELEASE_ASSERT(result, result.message()); result = upstream_request_->waitForEndStream(*dispatcher_); RELEASE_ASSERT(result, result.message()); @@ -224,7 +224,7 @@ TEST_P(RatelimitIntegrationTest, Timeout) { TEST_P(RatelimitIntegrationTest, ConnectImmediateDisconnect) { initiateClientConnection(); - ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, &fake_ratelimit_connection_)); + ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, fake_ratelimit_connection_)); ASSERT_TRUE(fake_ratelimit_connection_->close()); ASSERT_TRUE(fake_ratelimit_connection_->waitForDisconnect(true)); fake_ratelimit_connection_ = nullptr; diff --git a/test/integration/tcp_conn_pool_integration_test.cc b/test/integration/tcp_conn_pool_integration_test.cc index 01233273f9419..c13d836e8d2e6 100644 --- a/test/integration/tcp_conn_pool_integration_test.cc +++ b/test/integration/tcp_conn_pool_integration_test.cc @@ -153,7 +153,7 @@ TEST_P(TcpConnPoolIntegrationTest, SingleRequest) { tcp_client->write(request); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->waitForData(request.size())); ASSERT_TRUE(fake_upstream_connection->write(response)); @@ -172,7 +172,7 @@ TEST_P(TcpConnPoolIntegrationTest, MultipleRequests) { // send request 1 tcp_client->write(request1); FakeRawConnectionPtr fake_upstream_connection1; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection1)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection1)); std::string data; ASSERT_TRUE(fake_upstream_connection1->waitForData(request1.size(), &data)); EXPECT_EQ(request1, data); @@ -180,7 +180,7 @@ TEST_P(TcpConnPoolIntegrationTest, MultipleRequests) { // send request 2 tcp_client->write(request2); FakeRawConnectionPtr fake_upstream_connection2; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection2)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection2)); ASSERT_TRUE(fake_upstream_connection2->waitForData(request2.size(), &data)); EXPECT_EQ(request2, data); diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index cdf93dfda6690..ae3d1f132ef8f 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -34,7 +34,7 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamWritesFirst) { initialize(); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->write("hello")); tcp_client->waitForData("hello"); @@ -56,7 +56,7 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamDisconnect) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); tcp_client->write("hello"); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->waitForData(5)); ASSERT_TRUE(fake_upstream_connection->write("world")); ASSERT_TRUE(fake_upstream_connection->close()); @@ -74,7 +74,7 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyDownstreamDisconnect) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); tcp_client->write("hello"); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->waitForData(5)); ASSERT_TRUE(fake_upstream_connection->write("world")); tcp_client->waitForData("world"); @@ -94,7 +94,7 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyLargeWrite) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); tcp_client->write(data); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->waitForData(data.size())); ASSERT_TRUE(fake_upstream_connection->write(data)); tcp_client->waitForData(data); @@ -128,7 +128,7 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyDownstreamFlush) { std::string data(size, 'a'); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); tcp_client->readDisable(true); tcp_client->write("", true); @@ -167,7 +167,7 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamFlush) { std::string data(size, 'a'); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->readDisable(true)); ASSERT_TRUE(fake_upstream_connection->write("", true)); @@ -197,7 +197,7 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamFlushEnvoyExit) { std::string data(size, 'a'); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->readDisable(true)); ASSERT_TRUE(fake_upstream_connection->write("", true)); @@ -241,7 +241,7 @@ TEST_P(TcpProxyIntegrationTest, AccessLog) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->write("hello")); tcp_client->waitForData("hello"); @@ -286,7 +286,7 @@ TEST_P(TcpProxyIntegrationTest, ShutdownWithOpenConnections) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); tcp_client->write("hello"); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->waitForData(5)); ASSERT_TRUE(fake_upstream_connection->write("world")); tcp_client->waitForData("world"); @@ -343,7 +343,7 @@ TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithLargeOutstandingData) { initialize(); IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data(1024 * 16, 'a'); tcp_client->write(data); @@ -397,7 +397,7 @@ void TcpProxySslIntegrationTest::setupConnections() { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); } - AssertionResult result = fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection_); + AssertionResult result = fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection_); RELEASE_ASSERT(result, result.message()); } diff --git a/test/integration/websocket_integration_test.cc b/test/integration/websocket_integration_test.cc index 0248abd466167..c3b4289a641fb 100644 --- a/test/integration/websocket_integration_test.cc +++ b/test/integration/websocket_integration_test.cc @@ -124,7 +124,7 @@ TEST_P(WebsocketIntegrationTest, WebSocketConnectionDownstreamDisconnect) { if (old_style_websockets_) { test_server_->waitForCounterGe("tcp.websocket.downstream_cx_total", 1); } - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(&headersRead, &data)); validateInitialUpstreamData(data); @@ -177,7 +177,7 @@ TEST_P(WebsocketIntegrationTest, WebSocketConnectionUpstreamDisconnect) { tcp_client = makeTcpConnection(lookupPort("http")); // Send websocket upgrade request tcp_client->write(upgrade_req_str_); - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(&headersRead, &data)); validateInitialUpstreamData(data); @@ -220,7 +220,7 @@ TEST_P(WebsocketIntegrationTest, EarlyData) { tcp_client = makeTcpConnection(lookupPort("http")); // Send early data alongside websocket upgrade request tcp_client->write(upgrade_req_str + early_data_req_str); - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); // Wait for both the upgrade, and the early data. std::string data; @@ -268,7 +268,7 @@ TEST_P(WebsocketIntegrationTest, WebSocketConnectionIdleTimeout) { // The request path gets rewritten from /websocket/test to /websocket. // The size of headers received by the destination is 228 bytes. tcp_client->write(upgrade_req_str_); - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(&headersRead, &data)); validateInitialUpstreamData(data); @@ -341,7 +341,7 @@ TEST_P(WebsocketIntegrationTest, WebSocketLogging) { // The request path gets rewritten from /websocket/test to /websocket. // The size of headers received by the destination is 228 bytes. tcp_client->write(upgrade_req_str_); - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(228, &data)); // Accept websocket upgrade request @@ -407,7 +407,7 @@ TEST_P(WebsocketIntegrationTest, NonWebsocketUpgrade) { if (old_style_websockets_) { test_server_->waitForCounterGe("tcp.websocket.downstream_cx_total", 1); } - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(&headersRead, &data)); validateInitialUpstreamData(data); @@ -511,7 +511,7 @@ TEST_P(WebsocketIntegrationTest, WebsocketCustomFilterChain) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("http")); tcp_client->write(upgrade_req_str + early_data_req_str); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); // Make sure the full payload arrives. ASSERT_TRUE(fake_upstream_connection->waitForData( FakeRawConnection::waitForInexactMatch(early_data_req_str.c_str()))); @@ -534,7 +534,7 @@ TEST_P(WebsocketIntegrationTest, BidirectionalChunkedData) { IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("http")); tcp_client->write(upgrade_req_str); FakeRawConnectionPtr fake_upstream_connection; - ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(&fake_upstream_connection)); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); ASSERT_TRUE(fake_upstream_connection->waitForData( FakeRawConnection::waitForInexactMatch("SomeWebSocketPayload"))); diff --git a/test/integration/xfcc_integration_test.cc b/test/integration/xfcc_integration_test.cc index dfe53530944af..a2d55546a25e1 100644 --- a/test/integration/xfcc_integration_test.cc +++ b/test/integration/xfcc_integration_test.cc @@ -149,8 +149,8 @@ void XfccIntegrationTest::testRequestAndResponseWithXfccHeader(std::string previ codec_client_ = makeHttpConnection(std::move(conn)); auto response = codec_client_->makeHeaderOnlyRequest(header_map); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, &fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, &upstream_request_)); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); if (expected_xfcc.empty()) { EXPECT_EQ(nullptr, upstream_request_->headers().ForwardedClientCert()); diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index 85508ea5e6755..cd2bea3296c3d 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -183,6 +183,8 @@ void ConditionalInitializer::waitReady() { ScopedFdCloser::ScopedFdCloser(int fd) : fd_(fd) {} ScopedFdCloser::~ScopedFdCloser() { ::close(fd_); } +constexpr std::chrono::milliseconds TestUtility::DefaultTimeout; + namespace Http { // Satisfy linker diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 250492354e9dc..0baaacd63c966 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -76,12 +76,12 @@ namespace Envoy { } while (false) #define VERIFY_ASSERTION(statement) \ - { \ + do { \ ::testing::AssertionResult status = statement; \ if (!status) { \ return status; \ } \ - } + } while (false) // Random number generator which logs its seed to stderr. To repeat a test run with a non-zero seed // one can run the test with --test_arg=--gtest_random_seed=[seed] @@ -271,6 +271,8 @@ class TestUtility { } return result; } + + static constexpr std::chrono::milliseconds DefaultTimeout = std::chrono::milliseconds(10000); }; /**