test: Stop fake_upstream methods from accidentally succeeding (#4232)

mkbehr · zuercher · commit aa06142fffda · 2018-09-05T11:43:35.000-07:00
Description: FakeConnectionBase::waitForDisconnect and FakeHttpConnection::waitForNewStream were returning assertion successes when they timed out, because an AssertionResult constructed with a (non-empty) string counts as a success. Fix that.
Risk Level: Low (test only)
Testing: bazel test //test/...
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Michael Behr &lt;mkbehr@google.com&gt;
diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc
@@ -248,7 +248,7 @@ AssertionResult FakeConnectionBase::waitForDisconnect(bool ignore_spurious_event
   Thread::LockGuard lock(lock_);
   while (shared_connection_.connected()) {
     if (std::chrono::steady_clock::now() >= end_time) {
-      return AssertionResult("Timed out waiting for disconnect.");
+      return AssertionFailure() << "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
@@ -300,7 +300,7 @@ AssertionResult FakeHttpConnection::waitForNewStream(Event::Dispatcher& client_d
   Thread::LockGuard lock(lock_);
   while (new_streams_.empty()) {
     if (std::chrono::steady_clock::now() >= end_time) {
-      return AssertionResult("Timed out waiting for new stream.");
+      return AssertionFailure() << "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.
diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc
@@ -584,6 +584,13 @@ void HttpIntegrationTest::testRouterDownstreamDisconnectBeforeRequestComplete(
 
 void HttpIntegrationTest::testRouterDownstreamDisconnectBeforeResponseComplete(
     ConnectionCreationFunction* create_connection) {
+#ifdef __APPLE__
+  // Skip this test on OS X: we can't detect the early close on OS X, and we
+  // won't clean up the upstream connection until it times out. See #4294.
+  if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) {
+    return;
+  }
+#endif
   initialize();
   codec_client_ = makeHttpConnection(
       create_connection ? ((*create_connection)()) : makeClientConnection((lookupPort("http"))));
diff --git a/test/integration/ssl_integration_test.cc b/test/integration/ssl_integration_test.cc
@@ -154,6 +154,13 @@ TEST_P(SslIntegrationTest, RouterDownstreamDisconnectBeforeRequestComplete) {
 }
 
 TEST_P(SslIntegrationTest, RouterDownstreamDisconnectBeforeResponseComplete) {
+#ifdef __APPLE__
+  // Skip this test on OS X: we can't detect the early close on OS X, and we
+  // won't clean up the upstream connection until it times out. See #4294.
+  if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) {
+    return;
+  }
+#endif
   ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr {
     return makeSslClientConnection(false, false);
   };
