Skip to content

Commit

Permalink
test: Stop fake_upstream methods from accidentally succeeding (#4232)
Browse files Browse the repository at this point in the history
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 <mkbehr@google.com>
  • Loading branch information
mkbehr authored and zuercher committed Sep 5, 2018
1 parent 5d73187 commit aa06142
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 2 deletions.
4 changes: 2 additions & 2 deletions test/integration/fake_upstream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions test/integration/http_integration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"))));
Expand Down
7 changes: 7 additions & 0 deletions test/integration/ssl_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down

0 comments on commit aa06142

Please sign in to comment.