Commit c612fab5 authored by Stepan Palamarchuk's avatar Stepan Palamarchuk Committed by Facebook Github Bot

Change T_CHECK_TIMEOUT and T_CHECK_TIME_LT to fail tests instead of skipping them

Summary:
Most of the tests that use this macro, use it to ensure proper behavior of time-related logic (i.e. timeout didn't fire too early/late). Skipping them, simply masks a failure. The underlying utility already takes care of lags due to scheduling, so whenever this check fails - means we have a bug.

In particular, we have 3 tests for HHWheelTimer that are always skipped, because their tolerance is below the expected lag of HHWheelTimer (it always rounds up, so we should expect +1ms always).

Reviewed By: vitaut

Differential Revision: D13746558

fbshipit-source-id: 78f954f52414e640af92e5bb50790135cdd89a92
parent 8f7f677d
......@@ -254,7 +254,6 @@ TEST_F(HHWheelTimerTest, DestroyTimeoutSet) {
/*
* Test an event scheduled before the last event fires on time
*/
TEST_F(HHWheelTimerTest, SlowFast) {
StackWheelTimer t(&eventBase, milliseconds(1));
......@@ -276,9 +275,8 @@ TEST_F(HHWheelTimerTest, SlowFast) {
ASSERT_EQ(t2.timestamps.size(), 1);
ASSERT_EQ(t.count(), 0);
// Check that the timeout was delayed by sleep
T_CHECK_TIMEOUT(start, t1.timestamps[0], milliseconds(10), milliseconds(1));
T_CHECK_TIMEOUT(start, t2.timestamps[0], milliseconds(5), milliseconds(1));
T_CHECK_TIMEOUT(start, t1.timestamps[0], milliseconds(10));
T_CHECK_TIMEOUT(start, t2.timestamps[0], milliseconds(5));
}
TEST_F(HHWheelTimerTest, ReschedTest) {
......@@ -307,8 +305,8 @@ TEST_F(HHWheelTimerTest, ReschedTest) {
ASSERT_EQ(t2.timestamps.size(), 1);
ASSERT_EQ(t.count(), 0);
T_CHECK_TIMEOUT(start, t1.timestamps[0], milliseconds(128), milliseconds(1));
T_CHECK_TIMEOUT(start2, t2.timestamps[0], milliseconds(255), milliseconds(1));
T_CHECK_TIMEOUT(start, t1.timestamps[0], milliseconds(128));
T_CHECK_TIMEOUT(start2, t2.timestamps[0], milliseconds(255));
}
TEST_F(HHWheelTimerTest, DeleteWheelInTimeout) {
......@@ -335,7 +333,7 @@ TEST_F(HHWheelTimerTest, DeleteWheelInTimeout) {
ASSERT_EQ(t1.timestamps.size(), 1);
ASSERT_EQ(t2.timestamps.size(), 0);
T_CHECK_TIMEOUT(start, t1.timestamps[0], milliseconds(128), milliseconds(1));
T_CHECK_TIMEOUT(start, t1.timestamps[0], milliseconds(128));
}
/*
......
......@@ -35,11 +35,14 @@
* @param expectedMS The timeout duration, in milliseconds
* @param tolerance The tolerance, in milliseconds.
*/
#define T_CHECK_TIMEOUT(start, end, expectedMS, ...) \
if (!::folly::checkTimeout( \
(start), (end), (expectedMS), false, ##__VA_ARGS__)) { \
SKIP() << "T_CHECK_TIMEOUT lapsed"; \
}
#define T_CHECK_TIMEOUT(start, end, expectedMS, ...) \
EXPECT_TRUE(::folly::checkTimeout( \
(start), (end), (expectedMS), false, ##__VA_ARGS__)) \
<< "Timeout violates constraints, expectedMs = " << (expectedMS).count() \
<< ", elapsed wall time ms = " \
<< std::chrono::duration_cast<std::chrono::milliseconds>( \
(end).getTime() - (start).getTime()) \
.count();
/**
* Verify that an event took less than a specified amount of time.
......@@ -48,7 +51,10 @@
* than the allowed time.
*/
#define T_CHECK_TIME_LT(start, end, expectedMS, ...) \
if (!::folly::checkTimeout( \
(start), (end), (expectedMS), true, ##__VA_ARGS__)) { \
SKIP() << "T_CHECK_TIMEOUT_LT lapsed"; \
}
EXPECT_TRUE(::folly::checkTimeout( \
(start), (end), (expectedMS), true, ##__VA_ARGS__)) \
<< "Interval violates constraints, expectedMs = " \
<< (expectedMS).count() << ", elapsed wall time ms = " \
<< std::chrono::duration_cast<std::chrono::milliseconds>( \
(end).getTime() - (start).getTime()) \
.count();
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment