This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 4816c19b1e1c0b31e023b876edefc748b5c1f0b5 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. (cherry picked from commit 4c0bcfe9ca034ec602e373b471dcb7cf8cbf7a19) --- 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 4d7f9498ad..6e67dc1866 100644 --- a/src/iocore/cache/CMakeLists.txt +++ b/src/iocore/cache/CMakeLists.txt @@ -85,5 +85,6 @@ 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 8c0f8a7bdf..05d22740d2 100644 --- a/src/iocore/cache/P_CacheVol.h +++ b/src/iocore/cache/P_CacheVol.h @@ -292,9 +292,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); + } + } +}
