test: switching one more wait to not infinitely time out for failures

test/config/utility.cc

@@ -1049,6 +1049,12 @@ void ConfigHelper::setConnectTimeout(std::chrono::milliseconds timeout) {
   connect_timeout_set_ = true;
 }
 
+void ConfigHelper::disableDelayClose() {
+  addConfigModifier(
+      [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+             hcm) { hcm.mutable_delayed_close_timeout()->set_nanos(0); });
+}
+
 void ConfigHelper::setDownstreamMaxRequestsPerConnection(uint64_t max_requests_per_connection) {
   addConfigModifier(
       [max_requests_per_connection](

test/config/utility.h

@@ -246,6 +246,10 @@ class ConfigHelper {
   // Set the connect timeout on upstream connections.
   void setConnectTimeout(std::chrono::milliseconds timeout);
 
+  // Disable delay close. This is especially useful for tests doing raw TCP for
+  // HTTP/1.1 which functionally frame by connection close.
+  void disableDelayClose();
+
   // Set the max_requests_per_connection for downstream through the HttpConnectionManager.
   void setDownstreamMaxRequestsPerConnection(uint64_t max_requests_per_connection);
 

test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc

@@ -271,6 +271,7 @@ name: router
 // Verify the grpc cached logger is available after the initial logger filter is destroyed.
 // Regression test for https://github.com/envoyproxy/envoy/issues/18066
 TEST_P(AccessLogIntegrationTest, GrpcLoggerSurvivesAfterReloadConfig) {
+  config_helper_.disableDelayClose();
   autonomous_upstream_ = true;
   // The grpc access logger connection never closes. It's ok to see an incomplete logging stream.
   autonomous_allow_incomplete_streams_ = true;
@@ -301,16 +302,8 @@ TEST_P(AccessLogIntegrationTest, GrpcLoggerSurvivesAfterReloadConfig) {
 
   // HTTP 1.1 is allowed and the connection is kept open until the listener update.
   std::string response;
-  auto connection =
-      createConnectionDriver(lookupPort("http"), "GET / HTTP/1.1\r\nHost: host\r\n\r\n",
-                             [&response, &dispatcher = *dispatcher_](
-                                 Network::ClientConnection&, const Buffer::Instance& data) -> void {
-                               response.append(data.toString());
-                               if (response.find("\r\n\r\n") != std::string::npos) {
-                                 dispatcher.exit();
-                               }
-                             });
-  connection->run();
+  sendRawHttpAndWaitForResponse(lookupPort("http"), "GET / HTTP/1.1\r\nHost: host\r\n\r\n",
+                                &response, true);
   EXPECT_TRUE(response.find("HTTP/1.1 200") == 0);
 
   test_server_->waitForCounterEq("access_logs.grpc_access_log.logs_written", 2);

test/extensions/filters/network/direct_response/direct_response_integration_test.cc

@@ -43,7 +43,7 @@ TEST_P(DirectResponseIntegrationTest, DirectResponseOnConnection) {
         response.append(data.toString());
         conn.close(Network::ConnectionCloseType::FlushWrite);
       });
-  connection->run();
+  ASSERT_TRUE(connection->run());
   EXPECT_EQ("hello, world!\n", response);
   EXPECT_THAT(waitForAccessLog(listener_access_log_name_),
               testing::HasSubstr(StreamInfo::ResponseCodeDetails::get().DirectResponse));

test/extensions/filters/network/echo/echo_integration_test.cc

@@ -43,7 +43,7 @@ TEST_P(EchoIntegrationTest, Hello) {
         response.append(data.toString());
         conn.close(Network::ConnectionCloseType::FlushWrite);
       });
-  connection->run();
+  ASSERT_TRUE(connection->run());
   EXPECT_EQ("hello", response);
 }
 
@@ -90,7 +90,7 @@ name: new_listener
         response.append(data.toString());
         conn.close(Network::ConnectionCloseType::FlushWrite);
       });
-  connection->run();
+  ASSERT_TRUE(connection->run());
   EXPECT_EQ("hello", response);
 
   // Remove the listener.

test/extensions/stats_sinks/hystrix/hystrix_integration_test.cc

@@ -34,7 +34,7 @@ TEST_P(HystrixIntegrationTest, NoChunkEncoding) {
             conn.close(Network::ConnectionCloseType::NoFlush);
           }
         });
-    connection->run();
+    ASSERT_TRUE(connection->run());
     EXPECT_THAT(response, StartsWith("HTTP/1.1 200 OK\r\n"));
     // Make sure that the response is not actually chunk encoded, but it does have the hystrix flush
     // trailer.

test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc

@@ -333,7 +333,7 @@ class RawWriteSslIntegrationTest : public SslIntegrationTest {
 
     // Drive the connection until we get a response.
     while (response.empty()) {
-      connection->run(Event::Dispatcher::RunType::NonBlock);
+      EXPECT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock));
     }
     EXPECT_THAT(response, testing::HasSubstr("HTTP/1.1 200 OK\r\n"));
 

test/integration/base_integration_test.cc

@@ -446,7 +446,10 @@ void BaseIntegrationTest::sendRawHttpAndWaitForResponse(
       },
       std::move(transport_socket));
 
-  connection->run();
+  if (connection->run() != testing::AssertionSuccess()) {
+    FAIL() << "Failed to get expected response within the time bound\n"
+           << "received " << *response << "\n";
+  }
 }
 
 void BaseIntegrationTest::useListenerAccessLog(absl::string_view format) {

test/integration/command_formatter_extension_integration_test.cc

@@ -17,6 +17,7 @@ class CommandFormatterExtensionIntegrationTest : public testing::Test, public Ht
 };
 
 TEST_F(CommandFormatterExtensionIntegrationTest, BasicExtension) {
+  autonomous_upstream_ = true;
   TestCommandFactory factory;
   Registry::InjectFactory<CommandParserFactory> command_register(factory);
   std::vector<envoy::config::core::v3::TypedExtensionConfig> formatters;

test/integration/http_timeout_integration_test.cc

@@ -533,13 +533,13 @@ TEST_P(HttpTimeoutIntegrationTest, RequestHeaderTimeout) {
       });
 
   while (!connection_driver->allBytesSent()) {
-    connection_driver->run(Event::Dispatcher::RunType::NonBlock);
+    ASSERT_TRUE(connection_driver->run(Event::Dispatcher::RunType::NonBlock));
   }
   test_server_->waitForGaugeGe("http.config_test.downstream_rq_active", 1);
   ASSERT_FALSE(connection_driver->closed());
 
   timeSystem().advanceTimeWait(std::chrono::milliseconds(1001));
-  connection_driver->run();
+  ASSERT_TRUE(connection_driver->run());
 
   // The upstream should send a 40x response and send a local reply.
   EXPECT_TRUE(connection_driver->closed());

test/integration/idle_timeout_integration_test.cc

@@ -12,6 +12,7 @@ namespace {
 class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest {
 public:
   void initialize() override {
+    config_helper_.disableDelayClose();
     useAccessLog("%RESPONSE_CODE_DETAILS%");
     config_helper_.addConfigModifier(
         [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&

test/integration/integration_test.cc

@@ -717,6 +717,7 @@ TEST_P(IntegrationTest, UpstreamDisconnectWithTwoRequests) {
 }
 
 TEST_P(IntegrationTest, TestSmuggling) {
+  config_helper_.disableDelayClose();
   initialize();
 
   // Make sure the http parser rejects having content-length and transfer-encoding: chunked
@@ -1106,13 +1107,13 @@ TEST_P(IntegrationTest, Pipeline) {
       });
   // First response should be success.
   while (response.find("200") == std::string::npos) {
-    connection->run(Event::Dispatcher::RunType::NonBlock);
+    ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock));
   }
   EXPECT_THAT(response, StartsWith("HTTP/1.1 200 OK\r\n"));
 
   // Second response should be 400 (no host)
   while (response.find("400") == std::string::npos) {
-    connection->run(Event::Dispatcher::RunType::NonBlock);
+    ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock));
   }
   EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n"));
   connection->close();
@@ -1155,14 +1156,14 @@ TEST_P(IntegrationTest, PipelineWithTrailers) {
   // First response should be success.
   size_t pos;
   while ((pos = response.find("200")) == std::string::npos) {
-    connection->run(Event::Dispatcher::RunType::NonBlock);
+    ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock));
   }
   EXPECT_THAT(response, StartsWith("HTTP/1.1 200 OK\r\n"));
   while (response.find("200", pos + 1) == std::string::npos) {
-    connection->run(Event::Dispatcher::RunType::NonBlock);
+    ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock));
   }
   while (response.find("400") == std::string::npos) {
-    connection->run(Event::Dispatcher::RunType::NonBlock);
+    ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock));
   }
 
   EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n"));
@@ -1188,12 +1189,12 @@ TEST_P(IntegrationTest, PipelineInline) {
       });
 
   while (response.find("400") == std::string::npos) {
-    connection->run(Event::Dispatcher::RunType::NonBlock);
+    ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock));
   }
   EXPECT_THAT(response, StartsWith("HTTP/1.1 400 Bad Request\r\n"));
 
   while (response.find("426") == std::string::npos) {
-    connection->run(Event::Dispatcher::RunType::NonBlock);
+    ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock));
   }
   EXPECT_THAT(response, HasSubstr("HTTP/1.1 426 Upgrade Required\r\n"));
   connection->close();

test/integration/multiplexed_integration_test.cc

@@ -972,6 +972,7 @@ TEST_P(MultiplexedIntegrationTest, CodecErrorAfterStreamStart) {
 }
 
 TEST_P(MultiplexedIntegrationTest, Http2BadMagic) {
+  config_helper_.disableDelayClose();
   if (downstreamProtocol() == Http::CodecType::HTTP3) {
     // The "magic" payload is an HTTP/2 specific thing.
     return;
@@ -983,7 +984,7 @@ TEST_P(MultiplexedIntegrationTest, Http2BadMagic) {
       [&response](Network::ClientConnection&, const Buffer::Instance& data) -> void {
         response.append(data.toString());
       });
-  connection->run();
+  ASSERT_TRUE(connection->run());
   EXPECT_EQ("", response);
 }
 
@@ -997,7 +998,7 @@ TEST_P(MultiplexedIntegrationTest, BadFrame) {
       [&response](Network::ClientConnection&, const Buffer::Instance& data) -> void {
         response.append(data.toString());
       });
-  connection->run();
+  ASSERT_TRUE(connection->run());
   EXPECT_TRUE(response.find("SETTINGS expected") != std::string::npos);
 }
 
@@ -1324,14 +1325,14 @@ TEST_P(MultiplexedIntegrationTest, DelayedCloseAfterBadFrame) {
         connection.dispatcher().exit();
       });
 
-  connection->run();
+  ASSERT_TRUE(connection->run());
   EXPECT_THAT(response, HasSubstr("SETTINGS expected"));
   // Due to the multiple dispatchers involved (one for the RawConnectionDriver and another for the
   // Envoy server), it's possible the delayed close timer could fire and close the server socket
   // prior to the data callback above firing. Therefore, we may either still be connected, or have
   // received a remote close.
   if (connection->lastConnectionEvent() == Network::ConnectionEvent::Connected) {
-    connection->run();
+    ASSERT_TRUE(connection->run());
   }
   EXPECT_EQ(connection->lastConnectionEvent(), Network::ConnectionEvent::RemoteClose);
   EXPECT_EQ(test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value(),
@@ -1353,13 +1354,13 @@ TEST_P(MultiplexedIntegrationTest, DelayedCloseDisabled) {
         connection.dispatcher().exit();
       });
 
-  connection->run();
+  ASSERT_TRUE(connection->run());
   EXPECT_THAT(response, HasSubstr("SETTINGS expected"));
   // Due to the multiple dispatchers involved (one for the RawConnectionDriver and another for the
   // Envoy server), it's possible for the 'connection' to receive the data and exit the dispatcher
   // prior to the FIN being received from the server.
   if (connection->lastConnectionEvent() == Network::ConnectionEvent::Connected) {
-    connection->run();
+    ASSERT_TRUE(connection->run());
   }
   EXPECT_EQ(connection->lastConnectionEvent(), Network::ConnectionEvent::RemoteClose);
   EXPECT_EQ(test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value(),

test/integration/original_ip_detection_integration_test.cc

@@ -18,6 +18,7 @@ class OriginalIPDetectionIntegrationTest
   OriginalIPDetectionIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) {}
 
   void runTest(const std::string& ip) {
+    autonomous_upstream_ = true;
     useAccessLog("%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%");
     config_helper_.addConfigModifier(
         [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&

test/integration/protocol_integration_test.cc

@@ -3466,6 +3466,7 @@ TEST_P(ProtocolIntegrationTest, UpstreamDisconnectBeforeResponseCompleteWireByte
 }
 
 TEST_P(DownstreamProtocolIntegrationTest, BadRequest) {
+  config_helper_.disableDelayClose();
   // we only care about upstream protocol.
   if (downstreamProtocol() != Http::CodecType::HTTP1) {
     return;

test/integration/socket_interface_integration_test.cc

@@ -61,7 +61,7 @@ TEST_P(SocketInterfaceIntegrationTest, Basic) {
         response.append(data.toString());
         conn.close(Network::ConnectionCloseType::FlushWrite);
       });
-  connection->run();
+  ASSERT_TRUE(connection->run());
   EXPECT_EQ("hello", response);
 }
 

test/integration/utility.cc

@@ -267,12 +267,14 @@ RawConnectionDriver::RawConnectionDriver(uint32_t port, DoWriteCallback write_re
     : dispatcher_(dispatcher), remaining_bytes_to_send_(0) {
   api_ = Api::createApiForTest(stats_store_);
   Event::GlobalTimeSystem time_system;
-  callbacks_ = std::make_unique<ConnectionCallbacks>([this, write_request_callback]() {
-    Buffer::OwnedImpl buffer;
-    const bool close_after = write_request_callback(buffer);
-    remaining_bytes_to_send_ += buffer.length();
-    client_->write(buffer, close_after);
-  });
+  callbacks_ = std::make_unique<ConnectionCallbacks>(
+      [this, write_request_callback]() {
+        Buffer::OwnedImpl buffer;
+        const bool close_after = write_request_callback(buffer);
+        remaining_bytes_to_send_ += buffer.length();
+        client_->write(buffer, close_after);
+      },
+      dispatcher);
 
   if (transport_socket == nullptr) {
     transport_socket = Network::Test::createRawBufferSocket();
@@ -307,7 +309,19 @@ void RawConnectionDriver::waitForConnection() {
   }
 }
 
-void RawConnectionDriver::run(Event::Dispatcher::RunType run_type) { dispatcher_.run(run_type); }
+testing::AssertionResult RawConnectionDriver::run(Event::Dispatcher::RunType run_type,
+                                                  std::chrono::milliseconds timeout) {
+  Event::TimerPtr timeout_timer = dispatcher_.createTimer([this]() -> void { dispatcher_.exit(); });
+  timeout_timer->enableTimer(timeout);
+
+  dispatcher_.run(run_type);
+
+  if (timeout_timer->enabled()) {
+    timeout_timer->disableTimer();
+    return testing::AssertionSuccess();
+  }
+  return testing::AssertionFailure();
+}
 
 void RawConnectionDriver::close() { client_->close(Network::ConnectionCloseType::FlushWrite); }
 

test/integration/utility.h

@@ -19,6 +19,7 @@
 
 #include "test/test_common/printers.h"
 #include "test/test_common/test_time.h"
+#include "test/test_common/utility.h"
 
 #include "gtest/gtest.h"
 
@@ -85,7 +86,9 @@ class RawConnectionDriver {
                       Network::TransportSocketPtr transport_socket = nullptr);
   ~RawConnectionDriver();
   const Network::Connection& connection() { return *client_; }
-  void run(Event::Dispatcher::RunType run_type = Event::Dispatcher::RunType::Block);
+  testing::AssertionResult
+  run(Event::Dispatcher::RunType run_type = Event::Dispatcher::RunType::Block,
+      std::chrono::milliseconds timeout = TestUtility::DefaultTimeout);
   void close();
   Network::ConnectionEvent lastConnectionEvent() const {
     return callbacks_->last_connection_event_;
@@ -115,7 +118,8 @@ class RawConnectionDriver {
   struct ConnectionCallbacks : public Network::ConnectionCallbacks {
     using WriteCb = std::function<void()>;
 
-    ConnectionCallbacks(WriteCb write_cb) : write_cb_(write_cb) {}
+    ConnectionCallbacks(WriteCb write_cb, Event::Dispatcher& dispatcher)
+        : write_cb_(write_cb), dispatcher_(dispatcher) {}
     bool connected() const { return connected_; }
     bool closed() const { return closed_; }
 
@@ -124,11 +128,15 @@ class RawConnectionDriver {
       if (!connected_ && event == Network::ConnectionEvent::Connected) {
         write_cb_();
       }
-
       last_connection_event_ = event;
+
       closed_ |= (event == Network::ConnectionEvent::RemoteClose ||
                   event == Network::ConnectionEvent::LocalClose);
       connected_ |= (event == Network::ConnectionEvent::Connected);
+
+      if (closed_) {
+        dispatcher_.exit();
+      }
     }
     void onAboveWriteBufferHighWatermark() override {}
     void onBelowWriteBufferLowWatermark() override { write_cb_(); }
@@ -137,6 +145,7 @@ class RawConnectionDriver {
 
   private:
     WriteCb write_cb_;
+    Event::Dispatcher& dispatcher_;
     bool connected_{false};
     bool closed_{false};
   };