Skip to content

Commit aa06142

Browse files
mkbehrzuercher
authored andcommitted
test: Stop fake_upstream methods from accidentally succeeding (envoyproxy#4232)
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 <[email protected]>
1 parent 5d73187 commit aa06142

File tree

3 files changed

+16
-2
lines changed

3 files changed

+16
-2
lines changed

test/integration/fake_upstream.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ AssertionResult FakeConnectionBase::waitForDisconnect(bool ignore_spurious_event
248248
Thread::LockGuard lock(lock_);
249249
while (shared_connection_.connected()) {
250250
if (std::chrono::steady_clock::now() >= end_time) {
251-
return AssertionResult("Timed out waiting for disconnect.");
251+
return AssertionFailure() << "Timed out waiting for disconnect.";
252252
}
253253
Thread::CondVar::WaitStatus status = connection_event_.waitFor(lock_, 5ms);
254254
// The default behavior of waitForDisconnect is to assume the test cleanly
@@ -300,7 +300,7 @@ AssertionResult FakeHttpConnection::waitForNewStream(Event::Dispatcher& client_d
300300
Thread::LockGuard lock(lock_);
301301
while (new_streams_.empty()) {
302302
if (std::chrono::steady_clock::now() >= end_time) {
303-
return AssertionResult("Timed out waiting for new stream.");
303+
return AssertionFailure() << "Timed out waiting for new stream.";
304304
}
305305
Thread::CondVar::WaitStatus status = connection_event_.waitFor(lock_, 5ms);
306306
// As with waitForDisconnect, by default, waitForNewStream returns after the next event.

test/integration/http_integration.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,13 @@ void HttpIntegrationTest::testRouterDownstreamDisconnectBeforeRequestComplete(
584584

585585
void HttpIntegrationTest::testRouterDownstreamDisconnectBeforeResponseComplete(
586586
ConnectionCreationFunction* create_connection) {
587+
#ifdef __APPLE__
588+
// Skip this test on OS X: we can't detect the early close on OS X, and we
589+
// won't clean up the upstream connection until it times out. See #4294.
590+
if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) {
591+
return;
592+
}
593+
#endif
587594
initialize();
588595
codec_client_ = makeHttpConnection(
589596
create_connection ? ((*create_connection)()) : makeClientConnection((lookupPort("http"))));

test/integration/ssl_integration_test.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,13 @@ TEST_P(SslIntegrationTest, RouterDownstreamDisconnectBeforeRequestComplete) {
154154
}
155155

156156
TEST_P(SslIntegrationTest, RouterDownstreamDisconnectBeforeResponseComplete) {
157+
#ifdef __APPLE__
158+
// Skip this test on OS X: we can't detect the early close on OS X, and we
159+
// won't clean up the upstream connection until it times out. See #4294.
160+
if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) {
161+
return;
162+
}
163+
#endif
157164
ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr {
158165
return makeSslClientConnection(false, false);
159166
};

0 commit comments

Comments
 (0)