test: fixing interactions between waitFor and ignore_spurious_events (#4309)

alyssawilk · htuch · commit 7bc210e02f0d · 2018-09-04T18:32:36.000-04:00
Making sure if waitForHalfClose stops waiting due to the short waitFor timeout of 5ms, rather than a spurious event occurring, that the test continues waiting for half close. Risk Level: low (test only) Testing: tcp_proxy_integration_testx1000 test/integration:all x1 Docs Changes: n./a Release Notes: n/a Fixes #4031 Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc
@@ -276,13 +276,13 @@ AssertionResult FakeConnectionBase::waitForHalfClose(bool ignore_spurious_events
     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.
+    Thread::CondVar::WaitStatus status = connection_event_.waitFor(lock_, 5ms);
     // 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
     // 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;
     }
   }

Original file line number	Diff line number	Diff line change
`@@ -276,13 +276,13 @@ AssertionResult FakeConnectionBase::waitForHalfClose(bool ignore_spurious_events`
`276`	`276`	`if (std::chrono::steady_clock::now() >= end_time) {`
`277`	`277`	`return AssertionFailure() << "Timed out waiting for half close.";`
`278`	`278`	`}`
`279`		`- connection_event_.waitFor(lock_, 5ms); // Safe since CondVar::waitFor won't throw.`
	`279`	`+ Thread::CondVar::WaitStatus status = connection_event_.waitFor(lock_, 5ms);`
`280`	`280`	`// The default behavior of waitForHalfClose is to assume the test cleanly`
`281`	`281`	`// calls waitForData, waitForNewStream, etc. to handle all events on the`
`282`	`282`	`// connection. If the caller explicitly notes that other events should be`
`283`	`283`	`// ignored, continue looping until a disconnect is detected. Otherwise fall`
`284`	`284`	`// through and hit the assert below.`
`285`		`- if (!ignore_spurious_events) {`
	`285`	`+ if (status == Thread::CondVar::WaitStatus::NoTimeout && !ignore_spurious_events) {`
`286`	`286`	`break;`
`287`	`287`	`}`
`288`	`288`	`}`