[kudu] 02/02: [clock] clean up hybrid_clock-test

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit eaae22b086e70af600efd39781871c5812277442
Author: Alexey Serbin <al...@apache.org>
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 <ad...@cloudera.com>
    Tested-by: Kudu Jenkins
    (cherry picked from commit 22f0ea360a5f8b820631248a60fd8c1c879d81e4)
    Reviewed-on: http://gerrit.cloudera.org:8080/14490
    Reviewed-by: Grant Henke <gr...@apache.org>
---
 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(&timestamp, &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(&timestamp, &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) {