Make KVBucketTest::store_items() simpiler to use

Currently KVBucketTest::store_items() returns the status of if all writes have been successful, which requires the caller to wrap the call to store_items() in a ASSERT_TRUE()/EXPECT_TRUE(). However, if you don't look at the definition of the function that this might not be clear. So add [[nodiscard]] declaration to ensure that that caller checks the return value. Change-Id: I4a89fa24c2fcaf3476af9e3d42c816fd32941018 Reviewed-on: http://review.couchbase.org/c/kv_engine/+/164992 Reviewed-by: Dave Rigby <daver@couchbase.com> Tested-by: Build Bot <build@couchbase.com>
couchbase · Nov 3, 2021 · 15b3671 · 15b3671
1 parent 42ba1b6
commit 15b3671
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 48 deletions.
diff --git a/engines/ep/tests/module_tests/collections/evp_store_collections_dcp_stream_test.cc b/engines/ep/tests/module_tests/collections/evp_store_collections_dcp_stream_test.cc
@@ -93,14 +93,15 @@ TEST_F(CollectionsDcpStreamsTest, streamRequestNoRollbackSeqnoAdvanced) {
     vb->updateFromManifest(Collections::Manifest{std::string{cm}});
     flushVBucketToDiskIfPersistent(vbid, 2);
 
-    store_items(
-            5, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice");
+    ASSERT_TRUE(store_items(
+            5, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice"));
     flushVBucketToDiskIfPersistent(vbid, 5);
     auto streamSeqno = vb->getHighSeqno();
     EXPECT_EQ(streamSeqno,
               vb->getManifest().lock().getHighSeqno(CollectionEntry::fruit));
 
-    store_items(4, vbid, StoredDocKey{"Beef", CollectionEntry::meat}, "nice");
+    ASSERT_TRUE(store_items(
+            4, vbid, StoredDocKey{"Beef", CollectionEntry::meat}, "nice"));
     flushVBucketToDiskIfPersistent(vbid, 4);
 
     delete_item(vbid, StoredDocKey{"Beef1", CollectionEntry::meat});
@@ -148,8 +149,8 @@ TEST_F(CollectionsDcpStreamsTest, streamRequestNoRollbackNoSeqnoAdvanced) {
     vb->updateFromManifest(Collections::Manifest{std::string{cm}});
     flushVBucketToDiskIfPersistent(vbid, 2);
 
-    store_items(
-            2, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice");
+    ASSERT_TRUE(store_items(
+            2, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice"));
     flushVBucketToDiskIfPersistent(vbid, 2);
 
     store_item(vbid, StoredDocKey{"Beef", CollectionEntry::meat}, "nice");
@@ -221,16 +222,17 @@ TEST_F(CollectionsDcpStreamsTest,
     vb->updateFromManifest(Collections::Manifest{std::string{cm}});
     flushVBucketToDiskIfPersistent(vbid, 2);
 
-    store_items(
-            5, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice");
+    ASSERT_TRUE(store_items(
+            5, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice"));
     flushVBucketToDiskIfPersistent(vbid, 5);
 
     auto streamSeqno = vb->getHighSeqno();
     EXPECT_EQ(7, streamSeqno);
     EXPECT_EQ(streamSeqno,
               vb->getManifest().lock().getHighSeqno(CollectionEntry::fruit));
 
-    store_items(4, vbid, StoredDocKey{"Beef", CollectionEntry::meat}, "nice");
+    ASSERT_TRUE(store_items(
+            4, vbid, StoredDocKey{"Beef", CollectionEntry::meat}, "nice"));
     flushVBucketToDiskIfPersistent(vbid, 4);
 
     delete_item(vbid, StoredDocKey{"Beef1", CollectionEntry::meat});
@@ -280,11 +282,12 @@ TEST_F(CollectionsDcpStreamsTest, streamRequestNoRollbackMultiCollection) {
     vb->updateFromManifest(Collections::Manifest{std::string{cm}});
     flushVBucketToDiskIfPersistent(vbid, 2);
 
-    store_items(
-            2, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice");
+    ASSERT_TRUE(store_items(
+            2, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice"));
     flushVBucketToDiskIfPersistent(vbid, 2);
 
-    store_items(2, vbid, StoredDocKey{"Beef", CollectionEntry::meat}, "nice");
+    ASSERT_TRUE(store_items(
+            2, vbid, StoredDocKey{"Beef", CollectionEntry::meat}, "nice"));
     flushVBucketToDiskIfPersistent(vbid, 2);
 
     auto streamSeqno = vb->getHighSeqno();
@@ -353,11 +356,12 @@ TEST_F(CollectionsDcpStreamsTest, streamRequestRollbackMultiCollection) {
     vb->updateFromManifest(Collections::Manifest{std::string{cm}});
     flushVBucketToDiskIfPersistent(vbid, 2);
 
-    store_items(
-            2, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice");
+    ASSERT_TRUE(store_items(
+            2, vbid, StoredDocKey{"orange", CollectionEntry::fruit}, "nice"));
     flushVBucketToDiskIfPersistent(vbid, 2);
 
-    store_items(2, vbid, StoredDocKey{"Beef", CollectionEntry::meat}, "nice");
+    ASSERT_TRUE(store_items(
+            2, vbid, StoredDocKey{"Beef", CollectionEntry::meat}, "nice"));
     flushVBucketToDiskIfPersistent(vbid, 2);
 
     auto streamSeqno = vb->getHighSeqno();

diff --git a/engines/ep/tests/module_tests/collections/evp_store_collections_legacy_dcp_tests.cc b/engines/ep/tests/module_tests/collections/evp_store_collections_legacy_dcp_tests.cc
@@ -86,10 +86,10 @@ TEST_P(CollectionsLegacyDcpTest,
 
     // Store documents - importantly the final documents are !defaultCollection
     const int items = 5;
-    store_items(items,
-                vbid,
-                StoredDocKey{"d1_", CollectionEntry::defaultC},
-                "value");
+    ASSERT_TRUE(store_items(items,
+                            vbid,
+                            StoredDocKey{"d1_", CollectionEntry::defaultC},
+                            "value"));
     flushVBucketToDiskIfPersistent(vbid, items);
 
     CollectionsManifest cm;

diff --git a/engines/ep/tests/module_tests/collections/evp_store_collections_test.cc b/engines/ep/tests/module_tests/collections/evp_store_collections_test.cc
@@ -3112,11 +3112,13 @@ TEST_F(CollectionsTest, GetAllKeysNonCollectionConnection) {
     flushVBucketToDiskIfPersistent(vbid, 1);
 
     // Create and flush items for the default and meat collections
-    store_items(
-            10, vbid, makeStoredDocKey("beef", CollectionEntry::meat), "value");
+    ASSERT_TRUE(store_items(10,
+                            vbid,
+                            makeStoredDocKey("beef", CollectionEntry::meat),
+                            "value"));
     flushVBucketToDiskIfPersistent(vbid, 10);
 
-    store_items(5, vbid, makeStoredDocKey("default"), "value");
+    ASSERT_TRUE(store_items(5, vbid, makeStoredDocKey("default"), "value"));
     flushVBucketToDiskIfPersistent(vbid, 5);
 
     EXPECT_EQ(cb::engine_errc::would_block,
@@ -3135,7 +3137,7 @@ TEST_F(CollectionsTest, GetAllKeysNonCollectionConnectionMaxCountTen) {
     flushVBucketToDiskIfPersistent(vbid, 1);
 
     // Create and flush items for the default collection
-    store_items(20, vbid, makeStoredDocKey("default"), "value");
+    ASSERT_TRUE(store_items(20, vbid, makeStoredDocKey("default"), "value"));
     flushVBucketToDiskIfPersistent(vbid, 20);
 
     EXPECT_EQ(cb::engine_errc::would_block,
@@ -3146,7 +3148,7 @@ TEST_F(CollectionsTest, GetAllKeysNonCollectionConnectionMaxCountTen) {
 }
 
 TEST_F(CollectionsTest, GetAllKeysStartHalfWay) {
-    store_items(4, vbid, makeStoredDocKey("default"), "value");
+    ASSERT_TRUE(store_items(4, vbid, makeStoredDocKey("default"), "value"));
     flushVBucketToDiskIfPersistent(vbid, 4);
 
     // All keys default2 and after from default collection
@@ -3170,8 +3172,8 @@ TEST_F(CollectionsTest, GetAllKeysStartHalfWayForCollection) {
     setCollections(cookie, cm);
     flushVBucketToDiskIfPersistent(vbid, 1);
 
-    store_items(
-            4, vbid, makeStoredDocKey("meat", CollectionEntry::meat), "value");
+    ASSERT_TRUE(store_items(
+            4, vbid, makeStoredDocKey("meat", CollectionEntry::meat), "value"));
     flushVBucketToDiskIfPersistent(vbid, 4);
 
     // All keys meat2 and after from meat collection
@@ -3198,8 +3200,8 @@ TEST_F(CollectionsTest, GetAllKeysForCollectionEmptyKey) {
     setCollections(cookie, cm);
     flushVBucketToDiskIfPersistent(vbid, 1);
 
-    store_items(
-            4, vbid, makeStoredDocKey("meat", CollectionEntry::meat), "value");
+    ASSERT_TRUE(store_items(
+            4, vbid, makeStoredDocKey("meat", CollectionEntry::meat), "value"));
     flushVBucketToDiskIfPersistent(vbid, 4);
 
     // All keys meat2 and after from meat collection
@@ -3216,7 +3218,7 @@ TEST_F(CollectionsTest, GetAllKeysForCollectionEmptyKey) {
 }
 
 TEST_F(CollectionsTest, GetAllKeysNonCollectionConnectionCidEncodeKey) {
-    store_items(5, vbid, makeStoredDocKey("default"), "value");
+    ASSERT_TRUE(store_items(5, vbid, makeStoredDocKey("default"), "value"));
     flushVBucketToDiskIfPersistent(vbid, 5);
 
     // Ensure we treat any key as part of th default collection on a
@@ -3245,11 +3247,13 @@ TEST_F(CollectionsTest, GetAllKeysCollectionConnection) {
     flushVBucketToDiskIfPersistent(vbid, 1);
 
     // Create and flush items for the default and meat collections
-    store_items(
-            10, vbid, makeStoredDocKey("beef", CollectionEntry::meat), "value");
+    ASSERT_TRUE(store_items(10,
+                            vbid,
+                            makeStoredDocKey("beef", CollectionEntry::meat),
+                            "value"));
     flushVBucketToDiskIfPersistent(vbid, 10);
 
-    store_items(5, vbid, makeStoredDocKey("default"), "value");
+    ASSERT_TRUE(store_items(5, vbid, makeStoredDocKey("default"), "value"));
     flushVBucketToDiskIfPersistent(vbid, 5);
 
     // Get the keys for default collection, in this case we should get all 5
@@ -3495,11 +3499,13 @@ TEST_F(CollectionsRbacTest, GetAllKeysRbacCollectionConnection) {
     flushVBucketToDiskIfPersistent(vbid, 2);
 
     // Create and flush items for the default and meat collections
-    store_items(
-            10, vbid, makeStoredDocKey("beef", CollectionEntry::meat), "value");
+    ASSERT_TRUE(store_items(10,
+                            vbid,
+                            makeStoredDocKey("beef", CollectionEntry::meat),
+                            "value"));
     flushVBucketToDiskIfPersistent(vbid, 10);
 
-    store_items(5, vbid, makeStoredDocKey("default"), "value");
+    ASSERT_TRUE(store_items(5, vbid, makeStoredDocKey("default"), "value"));
     flushVBucketToDiskIfPersistent(vbid, 5);
 
     // Try and access all keys from the default collection, in this case we

diff --git a/engines/ep/tests/module_tests/collections/scope_data_limit_tests.cc b/engines/ep/tests/module_tests/collections/scope_data_limit_tests.cc
@@ -308,14 +308,14 @@ TEST_F(CollectionsTest, ScopeWithManyCollectionsWarmup) {
     cm.add(CollectionEntry::dairy, ScopeEntry::shop1);
     setCollections(cookie, cm);
 
-    store_items(
-            2, vbid, makeStoredDocKey("f", CollectionEntry::fruit), "value");
-    store_items(2,
-                vbid,
-                makeStoredDocKey("v", CollectionEntry::vegetable),
-                "value");
-    store_items(
-            2, vbid, makeStoredDocKey("d", CollectionEntry::dairy), "value");
+    ASSERT_TRUE(store_items(
+            2, vbid, makeStoredDocKey("f", CollectionEntry::fruit), "value"));
+    ASSERT_TRUE(store_items(2,
+                            vbid,
+                            makeStoredDocKey("v", CollectionEntry::vegetable),
+                            "value"));
+    ASSERT_TRUE(store_items(
+            2, vbid, makeStoredDocKey("d", CollectionEntry::dairy), "value"));
 
     EXPECT_EQ(0, vb->getManifest().lock().getDataSize(ScopeEntry::shop1));
 

diff --git a/engines/ep/tests/module_tests/couchstore_bucket_tests.cc b/engines/ep/tests/module_tests/couchstore_bucket_tests.cc
@@ -1286,12 +1286,12 @@ TEST_P(STParamCouchstoreBucketTest,
                PROTOCOL_BINARY_RAW_BYTES,
                {{cb::durability::Level::Majority, cb::durability::Timeout{}}});
 
-    store_items(5, vbid, makeStoredDocKey("key"), "value");
+    ASSERT_TRUE(store_items(5, vbid, makeStoredDocKey("key"), "value"));
     auto res = dynamic_cast<EPBucket&>(*store).flushVBucket(vbid);
     EXPECT_EQ(6, res.numFlushed);
 
     // Add another 5 items to disk so we can tell EP Engine to roll these back
-    store_items(5, vbid, makeStoredDocKey("key"), "value");
+    ASSERT_TRUE(store_items(5, vbid, makeStoredDocKey("key"), "value"));
     res = dynamic_cast<EPBucket&>(*store).flushVBucket(vbid);
     EXPECT_EQ(5, res.numFlushed);
     EXPECT_EQ(11, vbucket.getHighSeqno());

diff --git a/engines/ep/tests/module_tests/evp_store_rollback_test.cc b/engines/ep/tests/module_tests/evp_store_rollback_test.cc
@@ -1040,7 +1040,7 @@ class RollbackDcpTest : public RollbackTest {
         // Flush multiple checkpoints of unique keys
         for (int ii = start; ii < flushes; ii++) {
             std::string key = "anykey_" + std::to_string(ii) + "_";
-            EXPECT_TRUE(store_items(items,
+            ASSERT_TRUE(store_items(items,
                                     vbid,
                                     {key, DocKeyEncodesCollectionId::No},
                                     "value"));
@@ -1329,7 +1329,7 @@ TEST_P(RollbackDcpTest, test_rollback_zero_MB_48398) {
 
     // Store items, followed by failover entry - then one more seqno
     std::string key = "anykey_";
-    EXPECT_TRUE(store_items(
+    ASSERT_TRUE(store_items(
             nitems, vbid, {key, DocKeyEncodesCollectionId::No}, "value"));
     flush_vbucket_to_disk(vbid, nitems);
     // Add an entry for this seqno

diff --git a/engines/ep/tests/module_tests/kv_bucket_test.cc b/engines/ep/tests/module_tests/kv_bucket_test.cc
@@ -218,7 +218,8 @@ ::testing::AssertionResult KVBucketTest::store_items(
         auto err = store->set(item, cookie);
         if (cb::engine_errc::success != err) {
             return ::testing::AssertionFailure()
-                   << "Failed to store " << keyii.data() << " error:" << err;
+                   << "Failed to store key:'" << keyii.to_string()
+                   << "' error:" << err;
         }
     }
     return ::testing::AssertionSuccess();

diff --git a/engines/ep/tests/module_tests/kv_bucket_test.h b/engines/ep/tests/module_tests/kv_bucket_test.h
@@ -90,7 +90,7 @@ class KVBucketTest : virtual public ::testing::Test {
      * Store multiple items into the vbucket, the given key will have an
      * iteration appended to it.
      */
-    ::testing::AssertionResult store_items(
+    [[nodiscard]] ::testing::AssertionResult store_items(
             int nitems,
             Vbid vbid,
             const DocKey& key,