This is an automated email from the ASF dual-hosted git repository.
jvanderzee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 4c0bcfe9ca Test failure checks for Stripe::add_writer (#11160)
4c0bcfe9ca is described below
commit 4c0bcfe9ca034ec602e373b471dcb7cf8cbf7a19
Author: JosiahWI <[email protected]>
AuthorDate: Tue Mar 19 14:04:53 2024 -0500
Test failure checks for Stripe::add_writer (#11160)
This adds a new data-driven unit test to check all 32 conditions for the
behavior of Stripe::add_writer and fixes an error in the docstring for that
method.
There is no test to confirm whether or not the connection was added to the
queue because that cannot be determined easily. I think that should be tested
at the point where the write to the aggregation buffer occurs.
---
src/iocore/cache/CMakeLists.txt | 1 +
src/iocore/cache/P_CacheVol.h | 5 +-
src/iocore/cache/unit_tests/test_Stripe.cc | 159 +++++++++++++++++++++++++++++
3 files changed, 162 insertions(+), 3 deletions(-)
diff --git a/src/iocore/cache/CMakeLists.txt b/src/iocore/cache/CMakeLists.txt
index 3cc6c6ae94..68d10ac594 100644
--- a/src/iocore/cache/CMakeLists.txt
+++ b/src/iocore/cache/CMakeLists.txt
@@ -85,6 +85,7 @@ if(BUILD_TESTING)
add_cache_test(Update_L_to_S unit_tests/test_Update_L_to_S.cc)
add_cache_test(Update_S_to_L unit_tests/test_Update_S_to_L.cc)
add_cache_test(Update_Header unit_tests/test_Update_header.cc)
+ add_cache_test(CacheStripe unit_tests/test_Stripe.cc)
endif()
diff --git a/src/iocore/cache/P_CacheVol.h b/src/iocore/cache/P_CacheVol.h
index 54aabc0644..f19d7b69dc 100644
--- a/src/iocore/cache/P_CacheVol.h
+++ b/src/iocore/cache/P_CacheVol.h
@@ -289,9 +289,8 @@ public:
* - Adding a Doc to the virtual connection header would exceed the
* maximum fragment size.
* - vc->f.readers is not set (this virtual connection is not an
evacuator),
- * the internal aggregation buffer is full, the writes waiting to be
- * aggregated exceed the maximum backlog, and the virtual connection
- * has a non-zero write length.
+ * the writes waiting to be aggregated exceed the maximum backlog,
+ * and the virtual connection has a non-zero write length.
*
* @param vc: The virtual connection.
* @return: Returns true if the operation was successfull, otherwise false.
diff --git a/src/iocore/cache/unit_tests/test_Stripe.cc
b/src/iocore/cache/unit_tests/test_Stripe.cc
new file mode 100644
index 0000000000..52028a76ca
--- /dev/null
+++ b/src/iocore/cache/unit_tests/test_Stripe.cc
@@ -0,0 +1,159 @@
+/** @file
+
+ A brief file description
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+#include "main.h"
+
+#include <array>
+#include <ostream>
+
+// Required by main.h
+int cache_vols = 1;
+bool reuse_existing_cache = false;
+
+struct AddWriterBranchTest {
+ int initial_buffer_size{};
+ int agg_len{};
+ int header_len{};
+ int write_len{};
+ int readers{};
+ bool result{};
+};
+
+std::array<AddWriterBranchTest, 32> add_writer_branch_test_cases = {
+ {
+ {0, 0, 0, 0, 0, true},
+ {0, 0, 0, 0, 1, true},
+ {0, 0, 0, 1, 0, true},
+ {0, 0, 0, 1, 1, true},
+ {0, 0, MAX_FRAG_SIZE + 1 - sizeof(Doc), 0, 0, false},
+ {0, 0, MAX_FRAG_SIZE + 1 - sizeof(Doc), 0, 1, false},
+ {0, 0, MAX_FRAG_SIZE + 1 - sizeof(Doc), 1, 0, false},
+ {0, 0, MAX_FRAG_SIZE + 1 - sizeof(Doc), 1, 1, false},
+ {0, AGG_SIZE + 1, 0, 0, 0, false},
+ {0, AGG_SIZE + 1, 0, 0, 1, false},
+ {0, AGG_SIZE + 1, 0, 1, 0, false},
+ {0, AGG_SIZE + 1, 0, 1, 1, false},
+ {0, AGG_SIZE + 1, MAX_FRAG_SIZE + 1 - sizeof(Doc), 0, 0, false},
+ {0, AGG_SIZE + 1, MAX_FRAG_SIZE + 1 - sizeof(Doc), 0, 1, false},
+ {0, AGG_SIZE + 1, MAX_FRAG_SIZE + 1 - sizeof(Doc), 1, 0, false},
+ {0, AGG_SIZE + 1, MAX_FRAG_SIZE + 1 - sizeof(Doc), 1, 1, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, 0, 0, 0, 0, true},
+ {AGG_SIZE + cache_config_agg_write_backlog, 0, 0, 0, 1, true},
+ {AGG_SIZE + cache_config_agg_write_backlog, 0, 0, 1, 0, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, 0, 0, 1, 1, true},
+ {AGG_SIZE + cache_config_agg_write_backlog, 0, MAX_FRAG_SIZE + 1 -
sizeof(Doc), 0, 0, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, 0, MAX_FRAG_SIZE + 1 -
sizeof(Doc), 0, 1, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, 0, MAX_FRAG_SIZE + 1 -
sizeof(Doc), 1, 0, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, 0, MAX_FRAG_SIZE + 1 -
sizeof(Doc), 1, 1, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, AGG_SIZE + 1, 0, 0, 0, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, AGG_SIZE + 1, 0, 0, 1, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, AGG_SIZE + 1, 0, 1, 0, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, AGG_SIZE + 1, 0, 1, 1, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, AGG_SIZE + 1, MAX_FRAG_SIZE + 1
- sizeof(Doc), 0, 0, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, AGG_SIZE + 1, MAX_FRAG_SIZE + 1
- sizeof(Doc), 0, 1, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, AGG_SIZE + 1, MAX_FRAG_SIZE + 1
- sizeof(Doc), 1, 0, false},
+ {AGG_SIZE + cache_config_agg_write_backlog, AGG_SIZE + 1, MAX_FRAG_SIZE + 1
- sizeof(Doc), 1, 1, false},
+ }
+};
+
+class FakeVC final : public CacheVC
+{
+public:
+ void
+ set_agg_len(int agg_len)
+ {
+ this->agg_len = agg_len;
+ }
+
+ void
+ set_header_len(int header_len)
+ {
+ this->header_len = header_len;
+ }
+
+ void
+ set_write_len(int write_len)
+ {
+ this->write_len = write_len;
+ }
+
+ void
+ set_readers(int readers)
+ {
+ this->f.readers = readers;
+ }
+};
+
+TEST_CASE("The behavior of Stripe::add_writer.")
+{
+ FakeVC vc;
+ Stripe stripe;
+
+ SECTION("Branch tests.")
+ {
+ AddWriterBranchTest test_parameters =
GENERATE(from_range(add_writer_branch_test_cases));
+ INFO("Initial buffer size: " << test_parameters.initial_buffer_size);
+ INFO("VC agg_len: " << test_parameters.agg_len);
+ INFO("VC header length: " << test_parameters.header_len);
+ INFO("VC write length: " << test_parameters.write_len);
+ INFO("VC readers: " << test_parameters.readers);
+ INFO("Expected result: " << (test_parameters.result ? "true" : "false"));
+ vc.set_agg_len(AGG_SIZE);
+ for (int pending = 0; pending <= test_parameters.initial_buffer_size;
pending += AGG_SIZE) {
+ stripe.add_writer(&vc);
+ }
+ vc.set_agg_len(test_parameters.agg_len);
+ vc.set_write_len(test_parameters.write_len);
+ vc.set_header_len(test_parameters.header_len);
+ vc.set_readers(test_parameters.readers);
+ bool result = stripe.add_writer(&vc);
+ CHECK(test_parameters.result == result);
+ }
+
+ SECTION("Boundary cases.")
+ {
+ SECTION("agg_len")
+ {
+ vc.set_agg_len(AGG_SIZE);
+ bool result = stripe.add_writer(&vc);
+ CHECK(true == result);
+ }
+
+ SECTION("header_len")
+ {
+ vc.set_header_len(MAX_FRAG_SIZE - sizeof(Doc));
+ bool result = stripe.add_writer(&vc);
+ CHECK(true == result);
+ }
+
+ SECTION("initial pending bytes")
+ {
+ vc.set_agg_len(1);
+ for (int pending = 0; pending < AGG_SIZE +
cache_config_agg_write_backlog; pending += 1) {
+ stripe.add_writer(&vc);
+ }
+ bool result = stripe.add_writer(&vc);
+ CHECK(true == result);
+ }
+ }
+}
