luwei16 commented on code in PR #65973:
URL: https://github.com/apache/doris/pull/65973#discussion_r3665365560
##########
be/test/storage/tablet_reader_test.cpp:
##########
@@ -128,4 +128,57 @@ TEST_F(TabletReaderTest,
remove_delete_columns_keeps_unrelated_paths) {
EXPECT_EQ(size_t(2), access_paths.size());
}
+// Contract test for the binlog/snapshot incremental-read TSO range forwarding
added to
+// TabletReader::_capture_rs_readers (tablet_reader.cpp:184-186):
+// _reader_context.start_tso = read_params.start_tso;
+// _reader_context.end_tso = read_params.end_tso;
+// Exercising _capture_rs_readers end to end would require a fully constructed
Tablet + rowset
+// readers (it unconditionally dereferences _tablet), which is far too heavy
and brittle for a
+// unit test. Instead we pin down the field contract on both sides: both must
be
+// std::optional<int64_t> defaulting to nullopt, and the forwarding must
preserve the optional
+// state (both set / only one / none). This guards against the fields being
dropped or their
+// type changed, which would silently break the forwarding.
+TEST_F(TabletReaderTest, forward_tso_range_field_contract) {
+ // Both sides default to nullopt.
+ TabletReader::ReaderParams params;
+ EXPECT_FALSE(params.start_tso.has_value());
+ EXPECT_FALSE(params.end_tso.has_value());
+
+ RowsetReaderContext default_ctx;
+ EXPECT_FALSE(default_ctx.start_tso.has_value());
+ EXPECT_FALSE(default_ctx.end_tso.has_value());
+
+ // Equivalent of the forwarding assignments; the optional state must be
preserved verbatim.
+ auto forward = [](const TabletReader::ReaderParams& p) {
Review Comment:
[P3] Test the production forwarding rather than copying it
This lambda reimplements the expected assignments in test code, so deleting
either production assignment at `tablet_reader.cpp:185-186`, swapping
start/end, or forwarding only one bound leaves this test green. It therefore
cannot guard the behavior described in lines 131-140. Exercise
`_capture_rs_readers` through a minimal fixture, extract the context-population
step into a production helper used by this test, or remove this false-assurance
unit and rely on the real end-to-end coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
