This is an automated email from the ASF dual-hosted git repository. adar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 22f0ea360a5f8b820631248a60fd8c1c879d81e4 Author: Alexey Serbin <[email protected]> AuthorDate: Wed Oct 16 16:34:24 2019 -0700 [clock] clean up hybrid_clock-test This patch contains a small clean up on the HybridClockTest: * added ASSERT/CHECK for methods returning Status * added scoped cleanup into TestClockDoesntGoBackwardsWithUpdates Change-Id: I1c62cf51e11041b2369d29ff4f714876fd352cea Reviewed-on: http://gerrit.cloudera.org:8080/14474 Reviewed-by: Adar Dembo <[email protected]> Tested-by: Kudu Jenkins --- src/kudu/clock/hybrid_clock-test.cc | 45 +++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/kudu/clock/hybrid_clock-test.cc b/src/kudu/clock/hybrid_clock-test.cc index 837bc9a..a427b8e 100644 --- a/src/kudu/clock/hybrid_clock-test.cc +++ b/src/kudu/clock/hybrid_clock-test.cc @@ -37,6 +37,7 @@ #include "kudu/util/monotime.h" #include "kudu/util/random.h" #include "kudu/util/random_util.h" +#include "kudu/util/scoped_cleanup.h" #include "kudu/util/status.h" #include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" @@ -67,15 +68,15 @@ class HybridClockTest : public KuduTest { scoped_refptr<HybridClock> clock_; }; -clock::MockNtp* mock_ntp(const scoped_refptr<HybridClock>& clock) { +clock::MockNtp* mock_ntp(HybridClock* clock) { return down_cast<clock::MockNtp*>(clock->time_service()); } TEST(MockHybridClockTest, TestMockedSystemClock) { google::FlagSaver saver; FLAGS_time_source = "mock"; - scoped_refptr<HybridClock> clock(new HybridClock()); - clock->Init(); + scoped_refptr<HybridClock> clock(new HybridClock); + ASSERT_OK(clock->Init()); Timestamp timestamp; uint64_t max_error_usec; clock->NowWithError(×tamp, &max_error_usec); @@ -87,8 +88,8 @@ TEST(MockHybridClockTest, TestMockedSystemClock) { // Now set an arbitrary time and check that is the time returned by the clock. uint64_t time = 1234 * 1000; uint64_t error = 100 * 1000; - mock_ntp(clock)->SetMockClockWallTimeForTests(time); - mock_ntp(clock)->SetMockMaxClockErrorForTests(error); + mock_ntp(clock.get())->SetMockClockWallTimeForTests(time); + mock_ntp(clock.get())->SetMockMaxClockErrorForTests(error); clock->NowWithError(×tamp, &max_error_usec); ASSERT_EQ(timestamp.ToUint64(), HybridClock::TimestampFromMicrosecondsAndLogicalValue(time, 0).ToUint64()); @@ -108,14 +109,14 @@ TEST(MockHybridClockTest, TestMockedSystemClock) { TEST(MockHybridClockTest, TestClockDealsWithWrapping) { google::FlagSaver saver; FLAGS_time_source = "mock"; - scoped_refptr<HybridClock> clock(new HybridClock()); - clock->Init(); - mock_ntp(clock)->SetMockClockWallTimeForTests(1000); + scoped_refptr<HybridClock> clock(new HybridClock); + ASSERT_OK(clock->Init()); + mock_ntp(clock.get())->SetMockClockWallTimeForTests(1000); Timestamp prev = clock->Now(); // Update the clock from 10us in the future - clock->Update(HybridClock::TimestampFromMicroseconds(1010)); + ASSERT_OK(clock->Update(HybridClock::TimestampFromMicroseconds(1010))); // Now read the clock value enough times so that the logical value wraps // over, and should increment the _physical_ portion of the clock. @@ -129,7 +130,7 @@ TEST(MockHybridClockTest, TestClockDealsWithWrapping) { // Advance the time microsecond by microsecond, and ensure the clock never // goes backwards. for (int time = 1001; time < 1020; time++) { - mock_ntp(clock)->SetMockClockWallTimeForTests(time); + mock_ntp(clock.get())->SetMockClockWallTimeForTests(time); Timestamp now = clock->Now(); // Clock should run strictly forwards. @@ -192,10 +193,7 @@ TEST_F(HybridClockTest, TestWaitUntilAfter_TestCase1) { Timestamp past_ts_changed = HybridClock::AddPhysicalTimeToTimestamp( past_ts, MonoDelta::FromMicroseconds(-3 * static_cast<int64_t>(max_error))); - - Status s = clock_->WaitUntilAfter(past_ts_changed, no_deadline); - - ASSERT_OK(s); + ASSERT_OK(clock_->WaitUntilAfter(past_ts_changed, no_deadline)); MonoTime after = MonoTime::Now(); MonoDelta delta = after - before; @@ -229,7 +227,7 @@ TEST_F(HybridClockTest, TestWaitUntilAfter_TestCase2) { { MonoTime deadline = before; Status s = clock_->WaitUntilAfter(wait_until, deadline); - ASSERT_TRUE(s.IsTimedOut()); + ASSERT_TRUE(s.IsTimedOut()) << s.ToString(); } // Wait with a deadline well in the future. This should succeed. @@ -280,16 +278,23 @@ void StresserThread(HybridClock* clock, AtomicBool* stop) { // Add a random bit of offset to the clock, and perform an update. Timestamp new_ts = HybridClock::AddPhysicalTimeToTimestamp( t, MonoDelta::FromMicroseconds(rng.Uniform(10000))); - clock->Update(new_ts); + CHECK_OK(clock->Update(new_ts)); } } // Regression test for KUDU-953: if threads are updating and polling the // clock concurrently, the clock should still never run backwards. TEST_F(HybridClockTest, TestClockDoesntGoBackwardsWithUpdates) { - vector<scoped_refptr<kudu::Thread> > threads; - + vector<scoped_refptr<kudu::Thread>> threads; AtomicBool stop(false); + + SCOPED_CLEANUP({ + stop.Store(true); + for (const auto& t : threads) { + t->Join(); + } + }); + for (int i = 0; i < 4; i++) { scoped_refptr<Thread> thread; ASSERT_OK(Thread::Create("test", "stresser", @@ -299,10 +304,6 @@ TEST_F(HybridClockTest, TestClockDoesntGoBackwardsWithUpdates) { } SleepFor(MonoDelta::FromSeconds(1)); - stop.Store(true); - for (const scoped_refptr<Thread> t : threads) { - t->Join(); - } } TEST_F(HybridClockTest, TestGetPhysicalComponentDifference) {
