From 0d688ad1b81b9abaa34796bc702d04a13da97b6c Mon Sep 17 00:00:00 2001 From: Anthony Astolfi Date: Wed, 3 Jan 2024 17:45:35 -0500 Subject: [PATCH] Finished coverage testing of IoRingStreamBuffer (for now). Ready for MR! --- src/llfs/ioring_stream_buffer.cpp | 21 +------ src/llfs/ioring_stream_buffer.test.cpp | 87 ++++++-------------------- src/llfs/ioring_stream_buffer.test.hpp | 6 +- 3 files changed, 26 insertions(+), 88 deletions(-) diff --git a/src/llfs/ioring_stream_buffer.cpp b/src/llfs/ioring_stream_buffer.cpp index dc4175f..80cf8bd 100644 --- a/src/llfs/ioring_stream_buffer.cpp +++ b/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 locked{this->queue_}; this->end_of_stream_.store(true); @@ -76,7 +74,6 @@ StatusOr 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 { - 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 }); 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& 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); diff --git a/src/llfs/ioring_stream_buffer.test.cpp b/src/llfs/ioring_stream_buffer.test.cpp index 6ff898f..f4e07d7 100644 --- a/src/llfs/ioring_stream_buffer.test.cpp +++ b/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 view = this->stream_buffer_->prepare(); + + EXPECT_EQ(view.status(), batt::StatusCode::kClosed); +} + //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // TEST_F(IoringStreamBufferFullTest, PrepareWaitOk1) diff --git a/src/llfs/ioring_stream_buffer.test.hpp b/src/llfs/ioring_stream_buffer.test.hpp index 9844ee0..78bd82f 100644 --- a/src/llfs/ioring_stream_buffer.test.hpp +++ b/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_); }