Finished coverage testing of IoRingStreamBuffer (for now). Ready for MR!

src/llfs/ioring_stream_buffer.cpp

@@ -28,7 +28,6 @@ namespace llfs {
 //
 Status IoRingStreamBuffer::initialize() noexcept
 {
-  BATT_UNTESTED_COND(this->private_buffer_pool_);
   if (!this->private_buffer_pool_) {
     BATT_ASSIGN_OK_RESULT(
         IoRingBufferPool::BufferVec buffers,
@@ -61,7 +60,6 @@ usize IoRingStreamBuffer::size() const noexcept
 //
 void IoRingStreamBuffer::close()
 {
-  BATT_UNTESTED_LINE();
   {
     batt::ScopedLock<Fragment> locked{this->queue_};
     this->end_of_stream_.store(true);
@@ -76,7 +74,6 @@ StatusOr<IoRingMutableBufferView> IoRingStreamBuffer::prepare()
   LLFS_VLOG(1) << "minimum consume pos reached; allocating buffer...";
 
   if (this->end_of_stream_.load()) {
-    BATT_UNTESTED_LINE();
     return {batt::StatusCode::kClosed};
   }
 
@@ -116,8 +113,6 @@ void IoRingStreamBuffer::commit(BufferView&& view)
 //
 auto IoRingStreamBuffer::consume(i64 start, i64 end) -> StatusOr<Fragment>
 {
-  BATT_UNTESTED_LINE();
-
   LLFS_VLOG(1) << "IoRingStreamBuffer::consume(" << start << ", " << end << ")"
                << BATT_INSPECT(this->commit_pos_.get_value());
 
@@ -169,9 +164,9 @@ auto IoRingStreamBuffer::consume_some() -> StatusOr<Fragment>
       });
 
   if (BATT_HINT_FALSE(final_commit_pos.status() == batt::StatusCode::kClosed)) {
-    BATT_UNTESTED_LINE();
     const i64 observed_commit_pos = this->commit_pos_.get_value();
     if (observed_commit_pos - observed_consume_pos > 0) {
+      BATT_UNTESTED_LINE();
       final_commit_pos = observed_commit_pos;
     }
   }
@@ -210,7 +205,6 @@ usize IoRingStreamBuffer::buffer_size() const noexcept
 void IoRingStreamBuffer::check_for_end_of_stream(batt::ScopedLock<Fragment>& locked)
 {
   if (this->end_of_stream_.load() && locked->empty()) {
-    BATT_UNTESTED_LINE();
     this->commit_pos_.close();
     this->consume_pos_.close();
   }
@@ -238,7 +232,6 @@ usize IoRingStreamBuffer::Fragment::view_count() const noexcept
 void IoRingStreamBuffer::Fragment::push(BufferView&& view)
 {
   if (!this->views_.empty() && this->views_.back().merge_with(view)) {
-    BATT_UNTESTED_LINE();
     return;
   }
   this->views_.emplace_back(std::move(view));
@@ -248,15 +241,10 @@ void IoRingStreamBuffer::Fragment::push(BufferView&& view)
 //
 auto IoRingStreamBuffer::Fragment::pop(usize max_byte_count) -> Fragment
 {
-  BATT_UNTESTED_LINE();
   usize bytes_popped = 0;
   Fragment result;
 
-  BATT_UNTESTED_COND((!this->views_.empty() && bytes_popped < max_byte_count));
-  BATT_UNTESTED_COND(!(!this->views_.empty() && bytes_popped < max_byte_count));
-
   while (!this->views_.empty() && bytes_popped < max_byte_count) {
-    BATT_UNTESTED_LINE();
     BufferView& this_view = this->views_.front();
 
     const usize bytes_this_view = std::min(this_view.slice.size(), max_byte_count - bytes_popped);
@@ -273,10 +261,7 @@ auto IoRingStreamBuffer::Fragment::pop(usize max_byte_count) -> Fragment
     this_view.slice += bytes_this_view;
 
     if (this_view.slice.size() == 0) {
-      BATT_UNTESTED_LINE();
       this->views_.erase(this->views_.begin());
-    } else {
-      BATT_UNTESTED_LINE();
     }
   }
 
@@ -298,13 +283,9 @@ usize IoRingStreamBuffer::Fragment::byte_size() const noexcept
 //
 ConstBuffer IoRingStreamBuffer::Fragment::gather_impl(MutableBuffer dst) const noexcept
 {
-  BATT_UNTESTED_LINE();
   const void* dst_begin = dst.data();
   usize n_copied = 0;
 
-  BATT_UNTESTED_COND(this->views_.empty());
-  BATT_UNTESTED_COND(!this->views_.empty());
-
   for (const BufferView& view : this->views_) {
     const usize n_to_copy = std::min(view.slice.size(), dst.size());
     std::memcpy(dst.data(), view.slice.data(), n_to_copy);

src/llfs/ioring_stream_buffer.test.cpp

@@ -22,73 +22,6 @@ namespace {
 
 using namespace llfs::int_types;
 
-// Test Plan:
-//  Sizes:
-//   1. Empty
-//   2. Full
-//   3. Low (contains <=1 buffer worth, >0 bytes)
-//   4. High (contains >1 buffer worth, not full)
-//  States:
-//   a. open
-//   b. closed
-//   c. other task waiting on prepare
-//   d. other task waiting on consume_some
-//   e. other task waiting on consume(start, end)
-//  Actions:
-//   i. close
-//   ii. prepare
-//   iii. commit
-//   iv. consume(start, end)
-//   v. consume_some
-//  Results:
-//   (w. blocking until resolved into one of the following...)
-//   x. success, no blocking
-//   y. fail, error
-//   z. fail, panic
-//
-
-// 1.a.i.w. invalid (close never blocks)
-// 1.a.i.x.
-// 1.a.i.y. invalid
-// 1.a.i.z. invalid
-// 1.a.ii.w.
-// 1.a.ii.x.
-// 1.a.ii.y. invalid
-// 1.a.ii.z. invalid
-// 1.a.iii.w. invalid (commit never blocks)
-// 1.a.iii.x.
-// 1.a.iii.y. invalid
-// 1.a.iii.z.
-// 1.a.iv.w.
-// 1.a.iv.x. (end == start)
-// 1.a.iv.y.
-// 1.a.iv.z.
-// 1.a.v.w.
-// 1.a.v.x. (end == start)
-// 1.a.v.y.
-// 1.a.v.z. invalid
-
-// 1.b.i.w. invalid (close never blocks)
-// 1.b.i.x. invalid
-// 1.b.i.y.
-// 1.b.i.z. invalid
-// 1.b.ii.w.
-// 1.b.ii.x.
-// 1.b.ii.y. invalid
-// 1.b.ii.z. invalid
-// 1.b.iii.w. invalid (commit never blocks)
-// 1.b.iii.x.
-// 1.b.iii.y. invalid
-// 1.b.iii.z.
-// 1.b.iv.w.
-// 1.b.iv.x. (end == start)
-// 1.b.iv.y.
-// 1.b.iv.z.
-// 1.b.v.w.
-// 1.b.v.x. (end == start)
-// 1.b.v.y.
-// 1.b.v.z. invalid
-
 using llfs::testing::IoringStreamBufferClosedEmptyTest;
 using llfs::testing::IoringStreamBufferEmptyTest;
 using llfs::testing::IoringStreamBufferFullTest;
@@ -118,6 +51,15 @@ TEST_F(IoringStreamBufferTest, EmptyFragment)
   llfs::ConstBuffer gathered = fragment.gather(storage);
 
   EXPECT_EQ(gathered.size(), 0u);
+
+  llfs::IoRingStreamBuffer::Fragment fragment2 = fragment.pop(1);
+
+  EXPECT_TRUE(fragment.empty());
+  EXPECT_EQ(fragment.view_count(), 0u);
+  EXPECT_EQ(fragment.byte_size(), 0u);
+  EXPECT_TRUE(fragment2.empty());
+  EXPECT_EQ(fragment2.view_count(), 0u);
+  EXPECT_EQ(fragment2.byte_size(), 0u);
 }
 
 //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- -  -  -   -
@@ -371,6 +313,17 @@ TEST_F(IoringStreamBufferNotEmptyTest, ConsumeRangeWaitClosed)
   EXPECT_EQ(fragment2.status(), batt::StatusCode::kClosed);
 }
 
+//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- -  -  -   -
+//
+TEST_F(IoringStreamBufferNotEmptyTest, PrepareAfterClose)
+{
+  this->stream_buffer_->close();
+
+  batt::StatusOr<llfs::IoRingStreamBuffer::PreparedView> view = this->stream_buffer_->prepare();
+
+  EXPECT_EQ(view.status(), batt::StatusCode::kClosed);
+}
+
 //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- -  -  -   -
 //
 TEST_F(IoringStreamBufferFullTest, PrepareWaitOk1)

src/llfs/ioring_stream_buffer.test.hpp

@@ -265,7 +265,11 @@ class IoringStreamBufferNotEmptyTest : public IoringStreamBufferEmptyTest
   {
     ASSERT_NO_FATAL_FAILURE(Super::SetUp());
 
-    this->commit_test_data(this->buffer_size_);
+    // Commit the test data in two steps, to exercise code paths that merge buffer views inside a
+    // fragment.
+    //
+    this->commit_test_data(this->buffer_size_ - 1);
+    this->commit_test_data(1);
 
     EXPECT_EQ(this->stream_buffer_->size(), this->buffer_size_);
   }