MB-24034: VBucketTest: addOne/setOne to return Add/MutationStatus

Change VBucketTest::addOne() & setOne() to return the result of the
operation, instead of passing in the expected value and having the
function itself (addOne/setOne) check it.

This was done for two reasons:

1. Sometimes addOne() is used in setup code, where an ASSERT_EQ() is
   more appropriate to check the result.

2. If the EXPECT does fail, then in the previous code the GTest error
   message would point at the implementation of addOne(), which
   generally isn't very helpful - normally you want to see the
   call-site of addOne.

Change-Id: Id478fdf2a96002503b75d6bd40edb62d869d46bc
Reviewed-on: http://review.couchbase.org/77185
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

diff --git a/tests/module_tests/ephemeral_vb_test.cc b/tests/module_tests/ephemeral_vb_test.cc
index fd39c54..0b885f1 100644 (file)
--- a/tests/module_tests/ephemeral_vb_test.cc
+++ b/tests/module_tests/ephemeral_vb_test.cc
@@ -67,7 +67,7 @@ protected:
 // time.
 TEST_F(EphemeralVBucketTest, DoublePageOut) {
     auto key = makeStoredDocKey("key");
-    addOne(key, AddStatus::Success);
+    ASSERT_EQ(AddStatus::Success, addOne(key));
     ASSERT_EQ(1, vbucket->getNumItems());
 
     auto lock_sv = lockAndFind(key);
@@ -173,11 +173,11 @@ TEST_F(EphemeralVBucketTest, UpdateDuringBackfill) {
 
     /* Update the first, middle and last item in the range read and 2 items
        that are outside (before and after) range read */
-    setOne(keys[0], MutationStatus::WasDirty);
+    ASSERT_EQ(MutationStatus::WasDirty, setOne(keys[0]));
     for (int i = 1; i < numItems - 1; ++i) {
-        setOne(keys[i], MutationStatus::WasClean);
+        ASSERT_EQ(MutationStatus::WasClean, setOne(keys[i]));
     }
-    setOne(keys[numItems - 1], MutationStatus::WasDirty);
+    ASSERT_EQ(MutationStatus::WasDirty, setOne(keys[numItems - 1]));
 
     /* Hash table must have only recent (updated) items */
     EXPECT_EQ(numItems, vbucket->getNumItems());
@@ -247,7 +247,7 @@ TEST_F(EphTombstoneTest, ZeroElementPurge) {
 TEST_F(EphTombstoneTest, OneElementPurge) {
     // Create a new empty VB (using parent class SetUp).
     EphemeralVBucketTest::SetUp();
-    setOne(makeStoredDocKey("one"), MutationStatus::WasClean);
+    ASSERT_EQ(MutationStatus::WasClean, setOne(makeStoredDocKey("one")));
     ASSERT_EQ(1, mockEpheVB->public_getNumListItems());
 
     EXPECT_EQ(0, mockEpheVB->purgeTombstones(0));
@@ -341,7 +341,7 @@ TEST_F(EphTombstoneTest, ImmediatePurgeOfAliveStale) {
         std::lock_guard<std::mutex> rrGuard(
                 mockEpheVB->getLL()->getRangeReadLock());
         mockEpheVB->registerFakeReadRange(1, 2);
-        setOne(keys.at(1), MutationStatus::WasClean);
+        ASSERT_EQ(MutationStatus::WasClean, setOne(keys.at(1)));
 
         // Sanity check - our state is as expected:
         ASSERT_EQ(3, vbucket->getNumItems());

diff --git a/tests/module_tests/vbucket_test.cc b/tests/module_tests/vbucket_test.cc
index 43b7956..704c5b4 100644 (file)
--- a/tests/module_tests/vbucket_test.cc
+++ b/tests/module_tests/vbucket_test.cc
@@ -58,30 +58,26 @@ std::vector<StoredDocKey> VBucketTest::generateKeys(int num, int start) {
     return rv;
 }
 
-void VBucketTest::addOne(const StoredDocKey& k, AddStatus expect, int expiry) {
+AddStatus VBucketTest::addOne(const StoredDocKey& k, int expiry) {
     Item i(k, 0, expiry, k.data(), k.size());
-    EXPECT_EQ(expect, public_processAdd(i)) << "Failed to add key "
-                                                     << k.c_str();
+    return public_processAdd(i);
 }
 
 void VBucketTest::addMany(std::vector<StoredDocKey>& keys, AddStatus expect) {
     for (const auto& k : keys) {
-        addOne(k, expect);
+        EXPECT_EQ(expect, addOne(k));
     }
 }
 
-void VBucketTest::setOne(const StoredDocKey& k,
-                         MutationStatus expect,
-                         int expiry) {
+MutationStatus VBucketTest::setOne(const StoredDocKey& k, int expiry) {
     Item i(k, 0, expiry, k.data(), k.size());
-    EXPECT_EQ(expect, public_processSet(i, i.getCas())) << "Failed to set key "
-                                                        << k.c_str();
+    return public_processSet(i, i.getCas());
 }
 
 void VBucketTest::setMany(std::vector<StoredDocKey>& keys,
                           MutationStatus expect) {
     for (const auto& k : keys) {
-        setOne(k, expect);
+        EXPECT_EQ(expect, setOne(k));
     }
 }
 
@@ -288,8 +284,8 @@ TEST_P(VBucketTest, AddExpiry) {
     }
     StoredDocKey k = makeStoredDocKey("aKey");
 
-    addOne(k, AddStatus::Success, ep_real_time() + 5);
-    addOne(k, AddStatus::Exists, ep_real_time() + 5);
+    ASSERT_EQ(AddStatus::Success, addOne(k, ep_real_time() + 5));
+    EXPECT_EQ(AddStatus::Exists, addOne(k, ep_real_time() + 5));
 
     StoredValue* v =
             this->vbucket->ht.find(k, TrackReference::Yes, WantsDeleted::No);
@@ -300,7 +296,7 @@ TEST_P(VBucketTest, AddExpiry) {
     TimeTraveller biffTannen(6);
     EXPECT_TRUE(v->isExpired(ep_real_time()));
 
-    addOne(k, AddStatus::UnDel, ep_real_time() + 5);
+    EXPECT_EQ(AddStatus::UnDel, addOne(k, ep_real_time() + 5));
     EXPECT_TRUE(v);
     EXPECT_FALSE(v->isExpired(ep_real_time()));
     EXPECT_TRUE(v->isExpired(ep_real_time() + 6));

diff --git a/tests/module_tests/vbucket_test.h b/tests/module_tests/vbucket_test.h
index edb2de7..7892d0c 100644 (file)
--- a/tests/module_tests/vbucket_test.h
+++ b/tests/module_tests/vbucket_test.h
@@ -51,11 +51,11 @@ protected:
 
     std::vector<StoredDocKey> generateKeys(int num, int start = 0);
 
-    void addOne(const StoredDocKey& k, AddStatus expect, int expiry = 0);
+    AddStatus addOne(const StoredDocKey& k, int expiry = 0);
 
     void addMany(std::vector<StoredDocKey>& keys, AddStatus expect);
 
-    void setOne(const StoredDocKey& k, MutationStatus expect, int expiry = 0);
+    MutationStatus setOne(const StoredDocKey& k, int expiry = 0);
 
     void setMany(std::vector<StoredDocKey>& keys, MutationStatus expect);