Improving the fake clock and using it to fix a flaky STUN timeout test.

webrtc/base/gunit.h

Index: webrtc/base/gunit.h
diff --git a/webrtc/base/gunit.h b/webrtc/base/gunit.h
index e705322e6f827fd3dac94c2bb75a474376be977f..229513324b7bf7f93d14e4db75a4ffa9cc08228d 100644
--- a/webrtc/base/gunit.h
+++ b/webrtc/base/gunit.h
@@ -11,6 +11,7 @@
 #ifndef WEBRTC_BASE_GUNIT_H_
 #define WEBRTC_BASE_GUNIT_H_
 
+#include "webrtc/base/fakeclock.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/thread.h"
 #if defined(GTEST_RELATIVE_PATH)
@@ -23,7 +24,8 @@
 #define WAIT(ex, timeout)                             \
   for (int64_t start = rtc::TimeMillis();             \
        !(ex) && rtc::TimeMillis() < start + timeout;) \
-    rtc::Thread::Current()->ProcessMessages(1);
+    rtc::Thread::Current()->ProcessMessages(0);       \
+  rtc::Thread::Current()->SleepMs(1);

[pthatcher1, 2016/06/01 22:38:14]

Did you intend the sleep to be in the for loop?  If so, I think you need some
{}s.

[pthatcher1, 2016/06/01 22:38:14]

Would it make sense to have an AdvanceTime method like this?

void AdvanceTime(rtc::TimeDelta delta) {
  if (g_clock) {
    g_clock->AdvanceTime(delta);
    return;
  }
  rtc::Thread::Current()->ProcessMessages(0);
  rtc::Thread::Current()->SleepMs(1);
}

That way, the WAIT and SIMULATED_WAIT macros could be the same (and all the
calls to WAIT would magically work if we simply inserted a ScopedFakeClock?)

[Taylor Brandstetter, 2016/06/01 23:28:37]

That seems like too much hidden magic for me. g_clock isn't public and isn't
intended to be. Plus, eventually didn't we plan to get rid of g_clock and have a
ClockInterface passed into everything that needs it?

[Taylor Brandstetter, 2016/06/01 23:28:37]

Oops. I blame whoever initially wrote this code and didn't use {}s. :)

 
 // This returns the result of the test in res, so that we don't re-evaluate
 // the expression in the XXXX_WAIT macros below, since that causes problems
@@ -33,7 +35,8 @@
     int64_t start = rtc::TimeMillis();                    \
     res = (ex);                                           \
     while (!res && rtc::TimeMillis() < start + timeout) { \
-      rtc::Thread::Current()->ProcessMessages(1);         \
+      rtc::Thread::Current()->ProcessMessages(0);         \
+      rtc::Thread::Current()->SleepMs(1);                 \
       res = (ex);                                         \
     }                                                     \
   } while (0)
@@ -85,4 +88,41 @@
     } \
   } while (0)
 
+// Wait until "ex" is true, or "timeout" expires, using fake clock where
+// messages are processed every millisecond.
+#define SIMULATED_WAIT(ex, clock, timeout)            \

[pthatcher1, 2016/06/01 22:38:14]

Would it make sense for the clock to be the last arg on all of these?  Or the
first?  It seems weird in the middle.

[Taylor Brandstetter, 2016/06/01 23:28:37]

Sure, I don't have any preference.

+  for (int64_t start = rtc::TimeMillis();             \
+       !(ex) && rtc::TimeMillis() < start + timeout;) \
+    clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1));
+
+// This returns the result of the test in res, so that we don't re-evaluate
+// the expression in the XXXX_WAIT macros below, since that causes problems
+// when the expression is only true the first time you check it.
+#define SIMULATED_WAIT_(ex, clock, timeout, res)              \
+  do {                                                        \
+    int64_t start = rtc::TimeMillis();                        \
+    res = (ex);                                               \
+    while (!res && rtc::TimeMillis() < start + timeout) {     \
+      clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1)); \
+      res = (ex);                                             \
+    }                                                         \
+  } while (0)
+
+// The typical EXPECT_XXXX, but done until true or a timeout with a fake clock.
+#define EXPECT_TRUE_SIMULATED_WAIT(ex, clock, timeout) \
+  do {                                                 \
+    bool res;                                          \
+    SIMULATED_WAIT_(ex, clock, timeout, res);          \
+    if (!res)                                          \
+      EXPECT_TRUE(ex);                                 \
+  } while (0)
+
+#define EXPECT_EQ_SIMULATED_WAIT(v1, v2, clock, timeout) \
+  do {                                                   \
+    bool res;                                            \
+    SIMULATED_WAIT_(v1 == v2, clock, timeout, res);      \
+    if (!res)                                            \
+      EXPECT_EQ(v1, v2);                                 \
+  } while (0)
+
 #endif  // WEBRTC_BASE_GUNIT_H_