chenBright commented on code in PR #3545:
URL: https://github.com/apache/brpc/pull/3545#discussion_r4029016575
##########
test/bthread_timer_thread_unittest.cpp:
##########
@@ -98,59 +114,70 @@ class TimeKeeper {
keeper->run();
}
+ bool wait_finished() {
+ return WaitUntil([this] {
+ return _finished.load(butil::memory_order_acquire);
+ });
+ }
+
+ bool wait_started() {
+ return WaitUntil([this] {
+ return _started.load(butil::memory_order_acquire);
+ });
+ }
+
timespec _expect_run_time;
bthread::TimerThread::TaskId _task_id;
private:
const char* _name;
- int _sleep_ms;
+ butil::atomic<int> _sleep_ms;
+ butil::atomic<bool> _started{false};
+ butil::atomic<bool> _finished{false};
std::vector<timespec> _run_times;
};
TEST(TimerThreadTest, RunTasks) {
bthread::TimerThread timer_thread;
ASSERT_EQ(0, timer_thread.start(nullptr));
- timespec _2s_later = butil::seconds_from_now(2);
+ timespec _2s_later = butil::milliseconds_from_now(20);
TimeKeeper keeper1(_2s_later, "keeper1");
keeper1.schedule(&timer_thread);
- TimeKeeper keeper2(_2s_later, "keeper2"); // same time with keeper1
+ TimeKeeper keeper2(butil::seconds_from_now(3600), "keeper2");
keeper2.schedule(&timer_thread);
- timespec _1s_later = butil::seconds_from_now(1);
+ timespec _1s_later = butil::milliseconds_from_now(10);
TimeKeeper keeper3(_1s_later, "keeper3");
keeper3.schedule(&timer_thread);
- timespec _10s_later = butil::seconds_from_now(10);
+ timespec _10s_later = butil::seconds_from_now(3600);
TimeKeeper keeper4(_10s_later, "keeper4");
keeper4.schedule(&timer_thread);
TimeKeeper keeper5(_10s_later, "keeper5");
keeper5.schedule(&timer_thread);
- // sleep 1 second, and unschedule task2
- LOG(INFO) << "Sleep 1s";
- sleep(1);
- timer_thread.unschedule(keeper2._task_id);
- timer_thread.unschedule(keeper4._task_id);
+ ASSERT_EQ(0, timer_thread.unschedule(keeper2._task_id));
+ ASSERT_EQ(0, timer_thread.unschedule(keeper4._task_id));
timespec old_time = { 0, 0 };
TimeKeeper keeper6(old_time, "keeper6");
+ timespec keeper6_addtime = butil::seconds_from_now(0);
keeper6.schedule(&timer_thread);
- const timespec keeper6_addtime = butil::seconds_from_now(0);
- // sleep 10 seconds and stop.
- LOG(INFO) << "Sleep 2s";
- sleep(2);
+ ASSERT_TRUE(keeper1.wait_started());
+ ASSERT_TRUE(keeper3.wait_started());
+ ASSERT_TRUE(keeper6.wait_started());
Review Comment:
This only fires when the timer thread failed to run an already-due task
within a 10s budget.
At that point the test has failed for a real reason and the remaining
assertions are meaningless.
I'd rather keep it fatal and accept a messier teardown on an already-red run
than let the test
continue in a known-broken state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
