[BP] MB-55535: Fix SnapshotMarker check in CollectionsSeqnoAdvanced tests

And re-enable the check in all testsuite tests.

Change-Id: If07043475a743bf0088e57567b856468d8e41368
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/200040
Reviewed-by: Trond Norbye <[email protected]>
Tested-by: Paolo Cocchi <[email protected]>
Well-Formed: Restriction Checker

Author: paolococchi

engines/ep/src/dcp/response.cc

@@ -149,8 +149,9 @@ uint32_t MutationConsumerMessage::getMessageSize() const {
 }
 
 std::ostream& operator<<(std::ostream& os, const DcpResponse& r) {
-    os << "DcpResponse[" << &r << "] with"
-       << " event:" << r.to_string();
+    os << "DcpResponse[" << &r << "]"
+       << " event:" << r.to_string() << " seqno:"
+       << (r.getBySeqno() ? std::to_string(*r.getBySeqno()) : "nullopt");
     return os;
 }
 

engines/ep/tests/module_tests/collections/collections_seqno_advance.cc

@@ -91,6 +91,12 @@ class CollectionsSeqnoAdvanced
                                                    IncludeXattrs::Yes,
                                                    IncludeDeletedUserXattrs::No,
                                                    R"({"collections":["9"]})");
+
+        // Always queue an initial checkpoint_start item for making the final
+        // SnapshotMarker's flags easily predictable.
+        // The test-suites stresses extra operations at the end of each test
+        // (see ::TearDown()), so this step doesn't invalidate any test.
+        queueCPStart();
     }
 
     void TearDown() override {
@@ -106,25 +112,8 @@ class CollectionsSeqnoAdvanced
         for (const auto& e : expected.responses) {
             auto rsp = stream->public_nextQueuedItem(
                     static_cast<DcpProducer&>(*producer));
-            // Note
-            // All the markers generated by generateExpectedResponses() are
-            // flagged by just MARKER_FLAG_MEMORY. That was correctly mirroring
-            // a bug that we fixed in MB-55465, ie we fix a case in the
-            // SeqnoAdvance code where a marker is generated with a missing CHK
-            // flag set.
-            // Problem now in this testsuite is that is more difficult to
-            // predict the marker flag that is being generated by ActiveStream.
-            // Sometimes the CHK is set, other times it's not, depending on
-            // whether the stream jumps onto new checkpoints.
-            // For that, for now we exclude SnapshotMarker in the full check.
-            // @todo MB-55535: Try to re-enable full SnapshotMarker verification
             if (rsp) {
-                if (rsp->getEvent() == DcpResponse::Event::SnapshotMarker) {
-                    EXPECT_EQ(DcpResponse::Event::SnapshotMarker,
-                              e->getEvent());
-                } else {
-                    EXPECT_EQ(*e, *rsp);
-                }
+                EXPECT_EQ(*e, *rsp);
             }
             EXPECT_TRUE(rsp) << "DCP response expected:" << e->to_string();
         }
@@ -206,7 +195,6 @@ class CollectionsSeqnoAdvanced
         StoredDocKey key(to_string(checkpoint_op), CollectionID::System);
         queued_item qi(new Item(key, vbid, checkpoint_op, 1, currentSeqno + 1));
         input.items.emplace_back(qi);
-        checkStart = currentSeqno + 1;
         input.ranges.push_back({{currentSeqno + 1, currentSeqno + 1}, {}, {}});
     }
 
@@ -229,8 +217,6 @@ class CollectionsSeqnoAdvanced
 
     // Starting seqno, each operation will increment this
     uint64_t currentSeqno{1};
-    uint64_t checkStart{1};
-    uint64_t checkEnd{1};
 
     std::shared_ptr<MockDcpProducer> producer;
     std::shared_ptr<MockActiveStream> stream;
@@ -342,7 +328,11 @@ void CollectionsSeqnoAdvanced::generateExpectedResponses() {
     } else {
         // else a snapshot will be seen (this is pushed to the front of the
         // expected responses)
-        expected.snapshot(0, myHighCollectionSeqno.value(), MARKER_FLAG_MEMORY);
+        // Note: At setup we always queue an initial checkpoint_start item, so
+        // the CHK flag is set too in the marker
+        expected.snapshot(0,
+                          myHighCollectionSeqno.value(),
+                          MARKER_FLAG_MEMORY | MARKER_FLAG_CHK);
     }
 }