From b9d8d10d429150dfac0128cb20355b3abc86d406 Mon Sep 17 00:00:00 2001 From: skvlad Date: Tue, 6 Sep 2016 17:17:17 -0700 Subject: [PATCH] Fixed flaky StunRequestTests which depended on the wall clock StunRequestTests were using the real time clock to measure fairly large retransmit intervals (up to several seconds). This was making the tests slow and flaky when the system was heavily loaded. See https://build.chromium.org/p/client.webrtc/builders/Win64%20Release/builds/9274/steps/rtc_unittests/logs/stdio for an example of a recent failure. This change makes the tests use a simulated clock instead. They are now very quick, precise and reliable. R=honghaiz@webrtc.org, zhihuang@webrtc.org Review URL: https://codereview.webrtc.org/2300143005 . Cr-Commit-Position: refs/heads/master@{#14097} --- webrtc/p2p/base/stunrequest_unittest.cc | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/webrtc/p2p/base/stunrequest_unittest.cc b/webrtc/p2p/base/stunrequest_unittest.cc index 5fb34a85d2..47583b90e8 100644 --- a/webrtc/p2p/base/stunrequest_unittest.cc +++ b/webrtc/p2p/base/stunrequest_unittest.cc @@ -9,6 +9,7 @@ */ #include "webrtc/p2p/base/stunrequest.h" +#include "webrtc/base/fakeclock.h" #include "webrtc/base/gunit.h" #include "webrtc/base/helpers.h" #include "webrtc/base/logging.h" @@ -17,12 +18,6 @@ using namespace cricket; -// STUN timeout (with all retries) is 9500ms. -// Add some margin of error for slow bots. -// TODO(deadbeef): Use simulated clock instead of just increasing timeouts to -// fix flaky tests. -static const int kTimeoutMs = 15000; - class StunRequestTest : public testing::Test, public sigslot::has_slots<> { public: @@ -150,18 +145,19 @@ TEST_F(StunRequestTest, TestUnexpected) { // Test that requests are sent at the right times, and that the 9th request // (sent at 7900 ms) can be properly replied to. TEST_F(StunRequestTest, TestBackoff) { + const int MAX_TIMEOUT_MS = 10000; + rtc::ScopedFakeClock fake_clock; StunMessage* req = CreateStunMessage(STUN_BINDING_REQUEST, NULL); int64_t start = rtc::TimeMillis(); manager_.Send(new StunRequestThunker(req, this)); StunMessage* res = CreateStunMessage(STUN_BINDING_RESPONSE, req); for (int i = 0; i < 9; ++i) { - while (request_count_ == i) - rtc::Thread::Current()->ProcessMessages(1); + EXPECT_TRUE_SIMULATED_WAIT(request_count_ != i, MAX_TIMEOUT_MS, fake_clock); int64_t elapsed = rtc::TimeMillis() - start; LOG(LS_INFO) << "STUN request #" << (i + 1) << " sent at " << elapsed << " ms"; - EXPECT_GE(TotalDelay(i + 1), elapsed); + EXPECT_EQ(TotalDelay(i), elapsed); } EXPECT_TRUE(manager_.CheckResponse(res)); @@ -174,13 +170,15 @@ TEST_F(StunRequestTest, TestBackoff) { // Test that we timeout properly if no response is received in 9500 ms. TEST_F(StunRequestTest, TestTimeout) { + rtc::ScopedFakeClock fake_clock; StunMessage* req = CreateStunMessage(STUN_BINDING_REQUEST, NULL); StunMessage* res = CreateStunMessage(STUN_BINDING_RESPONSE, req); manager_.Send(new StunRequestThunker(req, this)); - rtc::Thread::Current()->ProcessMessages(kTimeoutMs); - EXPECT_FALSE(manager_.CheckResponse(res)); + // Simulate the 9500 ms STUN timeout + SIMULATED_WAIT(false, 9500, fake_clock); + EXPECT_FALSE(manager_.CheckResponse(res)); EXPECT_TRUE(response_ == NULL); EXPECT_FALSE(success_); EXPECT_FALSE(failure_);