diff --git a/.cspell.config.yaml b/.cspell.config.yaml index e3764fd2a2f..efe14ea8ef7 100644 --- a/.cspell.config.yaml +++ b/.cspell.config.yaml @@ -362,6 +362,7 @@ words: - xchain - ximinez - XMACRO + - xored - xrpkuwait - xrpl - xrpld diff --git a/cmake/XrplAddBenchmark.cmake b/cmake/XrplAddBenchmark.cmake index 1dd875dd61b..921deb06587 100644 --- a/cmake/XrplAddBenchmark.cmake +++ b/cmake/XrplAddBenchmark.cmake @@ -1,3 +1,5 @@ +include_guard() + include(isolate_headers) # Define a benchmark executable for the module `name`. diff --git a/cmake/XrplCov.cmake b/cmake/XrplCov.cmake index 86ba534a88e..05d9ed3806a 100644 --- a/cmake/XrplCov.cmake +++ b/cmake/XrplCov.cmake @@ -44,6 +44,7 @@ setup_target_for_coverage_gcovr( EXCLUDE "src/test" "src/tests" + "src/benchmarks" "include/xrpl/beast/test" "include/xrpl/beast/unit_test" "${CMAKE_BINARY_DIR}/pb-xrpl.libpb" diff --git a/include/xrpl/basics/Buffer.h b/include/xrpl/basics/Buffer.h index 05af6c409ad..911b829bb2b 100644 --- a/include/xrpl/basics/Buffer.h +++ b/include/xrpl/basics/Buffer.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -156,6 +157,19 @@ class Buffer } /** @} */ + /** + * Set every byte in the buffer to the given value. + * + * The size is unchanged, and this is a no-op on an empty buffer. + * + * @param value the byte to write to every position. + */ + void + fill(std::uint8_t value) noexcept + { + std::fill_n(p_.get(), size_, value); + } + /** * Reset the buffer. * All memory is deallocated. The resulting size is 0. diff --git a/src/benchmarks/libxrpl/nodestore/Backend.cpp b/src/benchmarks/libxrpl/nodestore/Backend.cpp index f854dbd3a77..40ffe480ad9 100644 --- a/src/benchmarks/libxrpl/nodestore/Backend.cpp +++ b/src/benchmarks/libxrpl/nodestore/Backend.cpp @@ -20,30 +20,34 @@ namespace xrpl::node_store { namespace { -constexpr std::size_t kPoolSizes[] = {1000, 10000, 100000}; -constexpr int kThreadCounts[] = {1, 4, 8}; +constexpr auto kPoolSizes = std::to_array({1000, 10000, 100000}); +constexpr auto kThreadCounts = std::to_array({1, 4, 8}); constexpr std::size_t kBatchSize = 256; +constexpr std::size_t kMissRatio = 5; constexpr std::string_view kNamePrefix = "BM_Backend_"; constexpr std::string_view kNameSeparator = "/"; struct RunState { - std::unique_ptr harness; - Batch present; // prefix-1 objects, eligible to be stored - Batch recent; // prefix-1 objects in the "future" key space - std::vector missing; // prefix-2 keys that are never stored - std::vector shuffle; // [0, poolSize) permutation for random-like access - std::size_t avgPayload = 0; // mean getData().size() over `present` + std::unique_ptr harness; ///< backend under test, rebuilt per run + Batch present; ///< prefix-1 objects, eligible to be stored + Batch recent; ///< prefix-1 objects in the "future" key space + std::vector missing; ///< prefix-2 keys that are never stored + std::vector shuffle; ///< [0, poolSize) permutation for random-like access + std::size_t avgPayload = 0; ///< mean getData().size() over `present` void release() { harness.reset(); + // swap-with-empty rather than assignment: only swapping is guaranteed to + // hand the capacity back, and these pools are large. Batch{}.swap(present); Batch{}.swap(recent); std::vector{}.swap(missing); std::vector{}.swap(shuffle); + avgPayload = 0; } }; @@ -85,7 +89,7 @@ Workload const kInsert{ }, .iterate = [](IterateContext const& ctx) { - auto& [rs, backend, index, poolSize] = ctx; + auto const& [rs, backend, index, poolSize] = ctx; backend.store(rs.present[index % poolSize]); }, .reportBytes = true, @@ -104,7 +108,7 @@ Workload const kFetch{ }, .iterate = [](IterateContext const& ctx) { - auto& [rs, backend, index, poolSize] = ctx; + auto const& [rs, backend, index, poolSize] = ctx; std::shared_ptr result; backend.fetch(rs.present[index % poolSize]->getHash(), &result); benchmark::DoNotOptimize(result); @@ -118,7 +122,7 @@ Workload const kMissing{ .setup = [](SetupContext const& ctx) { ctx.rs.missing = makeMissingKeys(ctx.poolSize); }, .iterate = [](IterateContext const& ctx) { - auto& [rs, backend, index, poolSize] = ctx; + auto const& [rs, backend, index, poolSize] = ctx; std::shared_ptr result; backend.fetch(rs.missing[index % poolSize], &result); benchmark::DoNotOptimize(result); @@ -139,10 +143,10 @@ Workload const kMixed{ }, .iterate = [](IterateContext const& ctx) { - auto& [rs, backend, index, poolSize] = ctx; + auto const& [rs, backend, index, poolSize] = ctx; std::shared_ptr result; auto const pick = rs.shuffle[index % poolSize]; - if (index % 5 == 0) + if (index % kMissRatio == 0) { backend.fetch(rs.missing[pick], &result); } @@ -170,7 +174,7 @@ Workload const kWork{ }, .iterate = [](IterateContext const& ctx) { - auto& [rs, backend, index, poolSize] = ctx; + auto const& [rs, backend, index, poolSize] = ctx; auto const slot = index % poolSize; auto const pick = rs.shuffle[slot]; @@ -238,9 +242,13 @@ registerWorkload(BackendConfig const& bc, Workload const& w) if (!w.pinToPool) { auto rs = std::make_shared(); - auto* b = benchmark::RegisterBenchmark(name, makeRunner(w, cfg, rs)); - b->RangeMultiplier(10)->Range(kPoolSizes[0], kPoolSizes[std::size(kPoolSizes) - 1]); - b->Threads(1)->Threads(4)->Threads(8)->UseRealTime(); + benchmark::RegisterBenchmark(name, makeRunner(w, cfg, rs)) + ->RangeMultiplier(10) + ->Range(kPoolSizes.front(), kPoolSizes.back()) + ->Threads(1) + ->Threads(4) + ->Threads(8) + ->UseRealTime(); return; } @@ -249,14 +257,14 @@ registerWorkload(BackendConfig const& bc, Workload const& w) { for (auto const threads : kThreadCounts) { - if (poolSize % static_cast(threads) != 0) + if (poolSize % threads != 0) continue; auto rs = std::make_shared(); benchmark::RegisterBenchmark(name, makeRunner(w, cfg, rs)) ->Arg(poolSize) - ->Iterations(poolSize / static_cast(threads)) - ->Threads(threads) + ->Iterations(poolSize / threads) + ->Threads(static_cast(threads)) ->UseRealTime(); } } @@ -289,7 +297,7 @@ registerStoreBatch(BackendConfig const& bc) rs->harness = std::make_unique(cfg); rs->present = makePool(1, poolSize); rs->avgPayload = averagePayload(rs->present); - std::vector const batches = sliceBatches(rs->present, kBatchSize); + std::vector const batches = slicePreciseBatches(rs->present, kBatchSize); if (batches.empty()) { state.SkipWithError("pool smaller than one batch"); diff --git a/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h b/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h index 57abf42e89b..3768ed642b6 100644 --- a/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h +++ b/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -40,18 +41,13 @@ inline void rngcpy(void* buffer, std::size_t bytes, Generator& g) { using result_type = typename Generator::result_type; - while (bytes >= sizeof(result_type)) + while (bytes > 0) { auto const v = g(); - std::memcpy(buffer, &v, sizeof(v)); - buffer = reinterpret_cast(buffer) + sizeof(v); - bytes -= sizeof(v); - } - - if (bytes > 0) - { - auto const v = g(); - std::memcpy(buffer, &v, bytes); + auto const chunk = std::min(bytes, sizeof(result_type)); + std::memcpy(buffer, &v, chunk); + buffer = reinterpret_cast(buffer) + chunk; + bytes -= chunk; } } @@ -145,7 +141,7 @@ makePool(std::uint8_t prefix, std::size_t count, std::size_t start = 0) Sequence seq(prefix); Batch pool; pool.reserve(count); - for (std::size_t i = 0; i < count; ++i) + for (auto const i : std::views::iota(0uz, count)) pool.push_back(seq.obj(start + i)); return pool; } @@ -158,7 +154,7 @@ makeMissingKeys(std::size_t count) Sequence seq(2); std::vector keys; keys.reserve(count); - for (std::size_t i = 0; i < count; ++i) + for (auto const i : std::views::iota(0uz, count)) keys.push_back(seq.key(i)); return keys; } @@ -206,16 +202,16 @@ inline std::vector makeShuffle(std::size_t size, std::uint64_t seed) { std::vector v(size); - std::iota(v.begin(), v.end(), std::size_t{0}); + std::ranges::iota(v, 0uz); beast::xor_shift_engine gen(seed); - std::shuffle(v.begin(), v.end(), gen); + std::ranges::shuffle(v, gen); return v; } // Partition a pool into fixed-size batches. Any trailing remainder shorter than // `batchSize` is dropped, so every returned batch has exactly `batchSize`. inline std::vector -sliceBatches(Batch const& pool, std::size_t batchSize) +slicePreciseBatches(Batch const& pool, std::size_t batchSize) { std::vector batches; if (batchSize == 0) @@ -228,13 +224,10 @@ sliceBatches(Batch const& pool, std::size_t batchSize) /** * @brief RAII owner of a NodeStore Backend opened on a private temporary directory. - * - * Member declaration order matters: `tempDir` is declared first so it is - * destroyed last, after the backend has closed and released its files. */ struct BackendHarness { - beast::TempDir tempDir; + beast::TempDir tempDir; ///< Declared first so it is destroyed last DummyScheduler scheduler; beast::Journal journal{beast::Journal::getNullSink()}; std::unique_ptr backend; @@ -304,12 +297,11 @@ struct BackendConfig inline std::vector const& backendConfigs() { + // Use factory settings for each DB static std::vector const kConfigs = { {.name = "nudb", .config = "type=nudb"}, #if XRPL_ROCKSDB_AVAILABLE - {.name = "rocksdb", - .config = "type=rocksdb,open_files=2000,filter_bits=12,cache_mb=256," - "file_size_mb=8,file_size_mult=2"}, + {.name = "rocksdb", .config = "type=rocksdb"}, #endif }; return kConfigs; diff --git a/src/tests/libxrpl/basics/Buffer.cpp b/src/tests/libxrpl/basics/Buffer.cpp index 9cdf610282b..3919c7a83a7 100644 --- a/src/tests/libxrpl/basics/Buffer.cpp +++ b/src/tests/libxrpl/basics/Buffer.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -12,8 +13,18 @@ namespace xrpl::test { +static_assert(std::is_nothrow_move_constructible_v); +static_assert(std::is_nothrow_move_assignable_v); + struct BufferTest : public ::testing::Test { + static constexpr auto kData = std::to_array( + {0xa8, 0xa1, 0x38, 0x45, 0x23, 0xec, 0xe4, 0x23, 0x71, 0x6d, 0x2a, + 0x18, 0xb4, 0x70, 0xcb, 0xf5, 0xac, 0x2d, 0x89, 0x4d, 0x19, 0x9c, + 0xf0, 0x2c, 0x15, 0xd1, 0xf9, 0x9b, 0x66, 0xd2, 0x30, 0xd3}); + + static constexpr std::size_t kHalf = kData.size() / 2; + static bool sane(Buffer const& b) { @@ -22,239 +33,308 @@ struct BufferTest : public ::testing::Test return b.data() != nullptr; } + + Buffer const emptyBuffer; + Buffer const firstHalf{kData.data(), kHalf}; + Buffer const secondHalf{kData.data() + kHalf, kHalf}; + Buffer const whole{kData.data(), kData.size()}; }; -TEST_F(BufferTest, buffer) +TEST_F(BufferTest, default_constructed_is_empty) +{ + Buffer const b; + + EXPECT_TRUE(sane(b)); + EXPECT_TRUE(b.empty()); + EXPECT_EQ(b.data(), nullptr); +} + +TEST_F(BufferTest, zero_sized_construction_is_empty) +{ + Buffer const b{0}; + + EXPECT_TRUE(sane(b)); + EXPECT_TRUE(b.empty()); +} + +TEST_F(BufferTest, alloc_grows_an_empty_buffer) +{ + Buffer b{0}; + std::memcpy(b.alloc(kHalf), kData.data(), kHalf); + + EXPECT_TRUE(sane(b)); + EXPECT_FALSE(b.empty()); + EXPECT_EQ(b.size(), kHalf); + EXPECT_EQ(b, firstHalf); +} + +TEST_F(BufferTest, sized_construction_reserves_without_filling) +{ + Buffer b{kHalf}; + + EXPECT_TRUE(sane(b)); + EXPECT_FALSE(b.empty()); + EXPECT_EQ(b.size(), kHalf); + + std::memcpy(b.data(), kData.data() + kHalf, kHalf); + EXPECT_EQ(b, secondHalf); +} + +TEST_F(BufferTest, construction_copies_raw_memory) +{ + Buffer const b{kData.data(), kData.size()}; + + EXPECT_TRUE(sane(b)); + EXPECT_FALSE(b.empty()); + EXPECT_EQ(b.size(), kData.size()); + EXPECT_EQ(std::memcmp(b.data(), kData.data(), b.size()), 0); +} +TEST_F(BufferTest, equality_compares_contents) +{ + // Uses EXPECT_TRUE rather than EXPECT_EQ/EXPECT_NE because the operators are what is under test + // here. + EXPECT_TRUE(emptyBuffer == emptyBuffer); + EXPECT_TRUE(firstHalf == firstHalf); + + EXPECT_TRUE(emptyBuffer != firstHalf); + EXPECT_TRUE(firstHalf != secondHalf); + EXPECT_TRUE(secondHalf != whole); +} + +TEST_F(BufferTest, copy_construction) +{ + Buffer const fromEmpty{emptyBuffer}; + EXPECT_TRUE(sane(fromEmpty)); + EXPECT_EQ(fromEmpty, emptyBuffer); + + Buffer const fromNonEmpty{firstHalf}; + EXPECT_TRUE(sane(fromNonEmpty)); + EXPECT_EQ(fromNonEmpty, firstHalf); +} + +TEST_F(BufferTest, copy_assignment) +{ + Buffer b{emptyBuffer}; + + // empty <- non-empty + b = secondHalf; + EXPECT_TRUE(sane(b)); + EXPECT_EQ(b, secondHalf); + + // non-empty <- non-empty of a different size + b = whole; + EXPECT_TRUE(sane(b)); + EXPECT_EQ(b, whole); + + // non-empty <- empty + b = emptyBuffer; + EXPECT_TRUE(sane(b)); + EXPECT_EQ(b, emptyBuffer); +} + +TEST_F(BufferTest, self_assignment_preserves_contents) { - std::uint8_t const data[] = {0xa8, 0xa1, 0x38, 0x45, 0x23, 0xec, 0xe4, 0x23, 0x71, 0x6d, 0x2a, - 0x18, 0xb4, 0x70, 0xcb, 0xf5, 0xac, 0x2d, 0x89, 0x4d, 0x19, 0x9c, - 0xf0, 0x2c, 0x15, 0xd1, 0xf9, 0x9b, 0x66, 0xd2, 0x30, 0xd3}; - - Buffer const b0; - EXPECT_TRUE(sane(b0)); - EXPECT_TRUE(b0.empty()); - - Buffer b1{0}; - EXPECT_TRUE(sane(b1)); - EXPECT_TRUE(b1.empty()); - std::memcpy(b1.alloc(16), data, 16); - EXPECT_TRUE(sane(b1)); - EXPECT_FALSE(b1.empty()); - EXPECT_EQ(b1.size(), 16); - - Buffer b2{b1.size()}; - EXPECT_TRUE(sane(b2)); - EXPECT_FALSE(b2.empty()); - EXPECT_EQ(b2.size(), b1.size()); - std::memcpy(b2.data(), data + 16, 16); - - Buffer b3{data, sizeof(data)}; - EXPECT_TRUE(sane(b3)); - EXPECT_FALSE(b3.empty()); - EXPECT_EQ(b3.size(), sizeof(data)); - EXPECT_EQ(std::memcmp(b3.data(), data, b3.size()), 0); - - // Check equality and inequality comparisons. - // For code readability, we want to use general - // EXPECT_TRUE instead of specific EXPECT_EQ etc. - EXPECT_TRUE(b0 == b0); - EXPECT_TRUE(b0 != b1); - EXPECT_TRUE(b1 == b1); - EXPECT_TRUE(b1 != b2); - EXPECT_TRUE(b2 != b3); - - // Check copy constructors and copy assignments: - { - Buffer x{b0}; - EXPECT_EQ(x, b0); - EXPECT_TRUE(sane(x)); - Buffer y{b1}; - EXPECT_EQ(y, b1); - EXPECT_TRUE(sane(y)); - x = b2; - EXPECT_EQ(x, b2); - EXPECT_TRUE(sane(x)); - x = y; - EXPECT_EQ(x, y); - EXPECT_TRUE(sane(x)); - y = b3; - EXPECT_EQ(y, b3); - EXPECT_TRUE(sane(y)); - x = b0; - EXPECT_EQ(x, b0); - EXPECT_TRUE(sane(x)); #ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wself-assign-overloaded" #endif - x = x; - EXPECT_EQ(x, b0); - EXPECT_TRUE(sane(x)); - y = y; - EXPECT_EQ(y, b3); - EXPECT_TRUE(sane(y)); + Buffer emptyCopy{emptyBuffer}; + emptyCopy = emptyCopy; + EXPECT_TRUE(sane(emptyCopy)); + EXPECT_EQ(emptyCopy, emptyBuffer); + + Buffer wholeCopy{whole}; + wholeCopy = wholeCopy; + EXPECT_TRUE(sane(wholeCopy)); + EXPECT_EQ(wholeCopy, whole); #ifdef __clang__ #pragma clang diagnostic pop #endif - } +} - // Check move constructor & move assignments: - { - static_assert(std::is_nothrow_move_constructible_v); - static_assert(std::is_nothrow_move_assignable_v); - - { // Move-construct from empty buf - Buffer x; - Buffer const y{std::move(x)}; - EXPECT_TRUE(sane(x)); // NOLINT(bugprone-use-after-move) - EXPECT_TRUE(x.empty()); // NOLINT(bugprone-use-after-move) - EXPECT_TRUE(sane(y)); - EXPECT_TRUE(y.empty()); - EXPECT_EQ(x, y); // NOLINT(bugprone-use-after-move) - } - - { // Move-construct from non-empty buf - Buffer x{b1}; - Buffer const y{std::move(x)}; - EXPECT_TRUE(sane(x)); // NOLINT(bugprone-use-after-move) - EXPECT_TRUE(x.empty()); // NOLINT(bugprone-use-after-move) - EXPECT_TRUE(sane(y)); - EXPECT_EQ(y, b1); - } - - { // Move assign empty buf to empty buf - Buffer x; - Buffer y; - - x = std::move(y); - EXPECT_TRUE(sane(x)); - EXPECT_TRUE(x.empty()); - EXPECT_TRUE(sane(y)); // NOLINT(bugprone-use-after-move) - EXPECT_TRUE(y.empty()); // NOLINT(bugprone-use-after-move) - } - - { // Move assign non-empty buf to empty buf - Buffer x; - Buffer y{b1}; - - x = std::move(y); - EXPECT_TRUE(sane(x)); - EXPECT_EQ(x, b1); - EXPECT_TRUE(sane(y)); // NOLINT(bugprone-use-after-move) - EXPECT_TRUE(y.empty()); // NOLINT(bugprone-use-after-move) - } - - { // Move assign empty buf to non-empty buf - Buffer x{b1}; - Buffer y; - - x = std::move(y); - EXPECT_TRUE(sane(x)); - EXPECT_TRUE(x.empty()); - EXPECT_TRUE(sane(y)); // NOLINT(bugprone-use-after-move) - EXPECT_TRUE(y.empty()); // NOLINT(bugprone-use-after-move) - } - - { // Move assign non-empty buf to non-empty buf - Buffer x{b1}; - Buffer y{b2}; - Buffer z{b3}; - - x = std::move(y); - EXPECT_TRUE(sane(x)); - EXPECT_FALSE(x.empty()); - EXPECT_TRUE(sane(y)); // NOLINT(bugprone-use-after-move) - EXPECT_TRUE(y.empty()); // NOLINT(bugprone-use-after-move) - - x = std::move(z); - EXPECT_TRUE(sane(x)); - EXPECT_FALSE(x.empty()); - EXPECT_TRUE(sane(z)); // NOLINT(bugprone-use-after-move) - EXPECT_TRUE(z.empty()); // NOLINT(bugprone-use-after-move) - } - } +TEST_F(BufferTest, move_construct_from_empty) +{ + Buffer source; + Buffer const moved{std::move(source)}; - { - Buffer w{static_cast(b0)}; - EXPECT_TRUE(sane(w)); - EXPECT_EQ(w, b0); - - Buffer x{static_cast(b1)}; - EXPECT_TRUE(sane(x)); - EXPECT_EQ(x, b1); - - Buffer y{static_cast(b2)}; - EXPECT_TRUE(sane(y)); - EXPECT_EQ(y, b2); - - Buffer z{static_cast(b3)}; - EXPECT_TRUE(sane(z)); - EXPECT_EQ(z, b3); - - // Assign empty slice to empty buffer - w = static_cast(b0); - EXPECT_TRUE(sane(w)); - EXPECT_EQ(w, b0); - - // Assign non-empty slice to empty buffer - w = static_cast(b1); - EXPECT_TRUE(sane(w)); - EXPECT_EQ(w, b1); - - // Assign non-empty slice to non-empty buffer - x = static_cast(b2); - EXPECT_TRUE(sane(x)); - EXPECT_EQ(x, b2); - - // Assign non-empty slice to non-empty buffer - y = static_cast(z); - EXPECT_TRUE(sane(y)); - EXPECT_EQ(y, z); - - // Assign empty slice to non-empty buffer: - z = static_cast(b0); - EXPECT_TRUE(sane(z)); - EXPECT_EQ(z, b0); - } + EXPECT_TRUE(sane(source)); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(source.empty()); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(sane(moved)); + EXPECT_TRUE(moved.empty()); +} + +TEST_F(BufferTest, move_construct_from_non_empty) +{ + Buffer source{firstHalf}; + Buffer const moved{std::move(source)}; + + EXPECT_TRUE(sane(source)); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(source.empty()); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(sane(moved)); + EXPECT_EQ(moved, firstHalf); +} + +TEST_F(BufferTest, move_assign_empty_to_empty) +{ + Buffer target; + Buffer source; + + target = std::move(source); + + EXPECT_TRUE(sane(target)); + EXPECT_TRUE(target.empty()); + EXPECT_TRUE(sane(source)); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(source.empty()); // NOLINT(bugprone-use-after-move) +} + +TEST_F(BufferTest, move_assign_non_empty_to_empty) +{ + Buffer target; + Buffer source{firstHalf}; + + target = std::move(source); + + EXPECT_TRUE(sane(target)); + EXPECT_EQ(target, firstHalf); + EXPECT_TRUE(sane(source)); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(source.empty()); // NOLINT(bugprone-use-after-move) +} + +TEST_F(BufferTest, move_assign_empty_to_non_empty) +{ + Buffer target{firstHalf}; + Buffer source; + target = std::move(source); + + EXPECT_TRUE(sane(target)); + EXPECT_TRUE(target.empty()); + EXPECT_TRUE(sane(source)); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(source.empty()); // NOLINT(bugprone-use-after-move) +} + +TEST_F(BufferTest, move_assign_non_empty_to_non_empty) +{ + Buffer target{firstHalf}; + Buffer sameSize{secondHalf}; + Buffer largerSize{whole}; + + target = std::move(sameSize); + EXPECT_TRUE(sane(target)); + EXPECT_EQ(target, secondHalf); + EXPECT_TRUE(sane(sameSize)); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(sameSize.empty()); // NOLINT(bugprone-use-after-move) + + target = std::move(largerSize); + EXPECT_TRUE(sane(target)); + EXPECT_EQ(target, whole); + EXPECT_TRUE(sane(largerSize)); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(largerSize.empty()); // NOLINT(bugprone-use-after-move) +} + +TEST_F(BufferTest, construction_from_slice) +{ + Buffer const fromEmpty{static_cast(emptyBuffer)}; + EXPECT_TRUE(sane(fromEmpty)); + EXPECT_EQ(fromEmpty, emptyBuffer); + + Buffer const fromNonEmpty{static_cast(whole)}; + EXPECT_TRUE(sane(fromNonEmpty)); + EXPECT_EQ(fromNonEmpty, whole); +} + +TEST_F(BufferTest, assignment_from_slice) +{ + Buffer b; + + // empty <- empty slice + b = static_cast(emptyBuffer); + EXPECT_TRUE(sane(b)); + EXPECT_EQ(b, emptyBuffer); + + // empty <- non-empty slice + b = static_cast(firstHalf); + EXPECT_TRUE(sane(b)); + EXPECT_EQ(b, firstHalf); + + // non-empty <- non-empty slice + b = static_cast(secondHalf); + EXPECT_TRUE(sane(b)); + EXPECT_EQ(b, secondHalf); + + // non-empty <- empty slice + b = static_cast(emptyBuffer); + EXPECT_TRUE(sane(b)); + EXPECT_EQ(b, emptyBuffer); +} + +TEST_F(BufferTest, resize_allocates_and_clear_releases) +{ + auto check = [](Buffer const& original, std::size_t size) { + SCOPED_TRACE(::testing::Message() << "size: " << size); + + Buffer b{original}; + + // Resizing to zero is equivalent to clearing. + b(size); + EXPECT_TRUE(sane(b)); + EXPECT_EQ(b.size(), size); + EXPECT_EQ(b.data() == nullptr, size == 0); + + b(size + 1); + EXPECT_TRUE(sane(b)); + EXPECT_EQ(b.size(), size + 1); + EXPECT_NE(b.data(), nullptr); + + b.clear(); + EXPECT_TRUE(sane(b)); + EXPECT_TRUE(b.empty()); + EXPECT_EQ(b.data(), nullptr); + + // clear() is idempotent. + b.clear(); + EXPECT_TRUE(sane(b)); + EXPECT_TRUE(b.empty()); + EXPECT_EQ(b.data(), nullptr); + }; + + for (auto size = 0uz; size < kHalf; ++size) { - auto test = [](Buffer const& b, std::size_t i) { - Buffer x{b}; - - // Try to allocate some number of bytes, possibly - // zero (which means clear) and sanity check - x(i); - EXPECT_TRUE(sane(x)); - EXPECT_EQ(x.size(), i); - EXPECT_EQ((x.data() == nullptr), (i == 0)); - - // Try to allocate some more data (always non-zero) - x(i + 1); - EXPECT_TRUE(sane(x)); - EXPECT_EQ(x.size(), i + 1); - EXPECT_NE(x.data(), nullptr); - - // Try to clear: - x.clear(); - EXPECT_TRUE(sane(x)); - EXPECT_TRUE(x.empty()); - EXPECT_EQ(x.data(), nullptr); - - // Try to clear again: - x.clear(); - EXPECT_TRUE(sane(x)); - EXPECT_TRUE(x.empty()); - EXPECT_EQ(x.data(), nullptr); - }; - - for (std::size_t i = 0; i < 16; ++i) - { - test(b0, i); - test(b1, i); - } + check(emptyBuffer, size); + check(firstHalf, size); } } +TEST_F(BufferTest, fill_sets_every_byte) +{ + Buffer b{4}; + b.fill(0xab); + + EXPECT_EQ(b.size(), 4); + for (auto const byte : Slice{b}) + EXPECT_EQ(byte, 0xab); +} + +TEST_F(BufferTest, fill_overwrites_and_keeps_size) +{ + Buffer b{4}; + b.fill(0xab); + b.fill(0x00); + + EXPECT_EQ(b.size(), 4); + for (auto const byte : Slice{b}) + EXPECT_EQ(byte, 0x00); +} + +TEST_F(BufferTest, fill_on_empty_buffer_is_a_noop) +{ + Buffer empty; + empty.fill(0xff); + + EXPECT_TRUE(empty.empty()); + EXPECT_EQ(empty.data(), nullptr); +} + } // namespace xrpl::test diff --git a/src/tests/libxrpl/basics/IntrusiveShared.cpp b/src/tests/libxrpl/basics/IntrusiveShared.cpp index e798cd1cccc..9659e1e01ee 100644 --- a/src/tests/libxrpl/basics/IntrusiveShared.cpp +++ b/src/tests/libxrpl/basics/IntrusiveShared.cpp @@ -50,11 +50,11 @@ struct Barrier { std::mutex mtx; std::condition_variable cv; - int count; - int const initial; + std::size_t count; + std::size_t const initial; std::size_t generation{0}; - explicit Barrier(int n) : count(n), initial(n) + explicit Barrier(std::size_t n) : count(n), initial(n) { } @@ -92,6 +92,7 @@ class TIBase : public IntrusiveRefCounts static constexpr std::size_t kMaxStates = 128; static std::array, kMaxStates> state; static std::atomic nextId; + static TrackedState getState(std::size_t id) { @@ -100,13 +101,12 @@ class TIBase : public IntrusiveRefCounts return state[id].load(std::memory_order_acquire); } + static void resetStates(bool resetCallback) { for (std::size_t i = 0; i < kMaxStates; ++i) - { state[i].store(TrackedState::Uninitialized, std::memory_order_release); - } nextId.store(0, std::memory_order_release); if (resetCallback) TIBase::tracingCallback = [](TrackedState, std::optional) {}; @@ -120,6 +120,7 @@ class TIBase : public IntrusiveRefCounts { TIBase::resetStates(resetCallback); } + ~ResetStatesGuard() { TIBase::resetStates(resetCallback); @@ -130,6 +131,7 @@ class TIBase : public IntrusiveRefCounts { state[id].store(TrackedState::Alive, std::memory_order_relaxed); } + ~TIBase() override { using enum TrackedState; @@ -217,10 +219,8 @@ TEST(IntrusiveSharedTest, basics) auto id = b->id; EXPECT_EQ(TIBase::getState(id), Alive); EXPECT_EQ(b->useCount(), 1); - for (int i = 0; i < 10; ++i) - { + for (auto i = 0uz; i < 10; ++i) strong.push_back(b); - } b.reset(); EXPECT_EQ(TIBase::getState(id), Alive); strong.resize(strong.size() - 1); @@ -232,7 +232,7 @@ TEST(IntrusiveSharedTest, basics) id = b->id; EXPECT_EQ(TIBase::getState(id), Alive); EXPECT_EQ(b->useCount(), 1); - for (int i = 0; i < 10; ++i) + for (auto i = 0uz; i < 10; ++i) { weak.emplace_back(b); EXPECT_EQ(b->useCount(), 1); @@ -244,8 +244,7 @@ TEST(IntrusiveSharedTest, basics) EXPECT_EQ(TIBase::getState(id), PartiallyDeleted); while (!weak.empty()) { - weak.resize(weak.size() - 1); - if (!weak.empty()) + if (weak.resize(weak.size() - 1); !weak.empty()) { EXPECT_EQ(TIBase::getState(id), PartiallyDeleted); } @@ -280,17 +279,17 @@ TEST(IntrusiveSharedTest, basics) TIBase::ResetStatesGuard const rsg{true}; using enum TrackedState; - using swu = SharedWeakUnion; - swu b = makeSharedIntrusive(); + using SharedWeak = SharedWeakUnion; + SharedWeak b = makeSharedIntrusive(); EXPECT_TRUE(b.isStrong() && b.useCount() == 1); auto id = b.get()->id; EXPECT_EQ(TIBase::getState(id), Alive); - swu w = b; + SharedWeak w = b; EXPECT_TRUE(TIBase::getState(id) == Alive); EXPECT_TRUE(w.isStrong() && b.useCount() == 2); w.convertToWeak(); EXPECT_TRUE(w.isWeak() && b.useCount() == 1); - swu s = w; + SharedWeak s = w; EXPECT_TRUE(s.isWeak() && b.useCount() == 1); s.convertToStrong(); EXPECT_TRUE(s.isStrong() && b.useCount() == 2); @@ -391,7 +390,7 @@ TEST(IntrusiveSharedTest, partial_delete) // given an opportunity to run during partial delete. EXPECT_EQ(cur, PartiallyDeleted); } - if (next == PartiallyDeletedStarted) + else if (next == PartiallyDeletedStarted) { partialDeleteStartedSyncPoint.arrive_and_wait(); using namespace std::chrono_literals; @@ -400,11 +399,11 @@ TEST(IntrusiveSharedTest, partial_delete) // is running. The test is to make sure that doesn't happen. std::this_thread::sleep_for(800ms); } - if (next == PartiallyDeleted) + else if (next == PartiallyDeleted) { EXPECT_FALSE(partialDeleteRan.exchange(true) || destructorRan.load()); } - if (next == Deleted) + else if (next == Deleted) { EXPECT_FALSE(destructorRan.exchange(true)); } @@ -448,7 +447,7 @@ TEST(IntrusiveSharedTest, destructor) { EXPECT_FALSE(partialDeleteRan.exchange(true) || destructorRan.load()); } - if (next == Deleted) + else if (next == Deleted) { EXPECT_FALSE(destructorRan.exchange(true)); } @@ -497,7 +496,7 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_variant) EXPECT_FALSE(partialDeleteRan || destructorRan); setPartialDeleteRan(); } - if (next == Deleted) + else if (next == Deleted) { EXPECT_FALSE(destructorRan); setDestructorRan(); @@ -506,11 +505,11 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_variant) auto createVecOfPointers = [&](auto const& toClone, std::default_random_engine& eng) -> std::vector, WeakIntrusive>> { std::vector, WeakIntrusive>> result; - std::uniform_int_distribution<> toCreateDist(4, 64); + std::uniform_int_distribution toCreateDist(4, 64); std::uniform_int_distribution<> isStrongDist(0, 1); auto numToCreate = toCreateDist(eng); result.reserve(numToCreate); - for (int i = 0; i < numToCreate; ++i) + for (auto i = 0uz; i < numToCreate; ++i) { if (isStrongDist(eng)) { @@ -523,8 +522,8 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_variant) } return result; }; - constexpr int kLoopIters = 2 * 1024; - constexpr int kNumThreads = 16; + constexpr auto kLoopIters = 2uz * 1024; + constexpr auto kNumThreads = 16uz; std::vector> toClone; Barrier loopStartSyncPoint{kNumThreads}; Barrier postCreateToCloneSyncPoint{kNumThreads}; @@ -533,7 +532,7 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_variant) std::random_device rd; std::vector result; result.reserve(kNumThreads); - for (int i = 0; i < kNumThreads; ++i) + for (auto i = 0uz; i < kNumThreads; ++i) result.emplace_back(rd()); return result; }(); @@ -541,8 +540,8 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_variant) // cloneAndDestroy clones the strong pointer into a vector of mixed // strong and weak pointers and destroys them all at once. // threadId==0 is special. - auto cloneAndDestroy = [&](int threadId) { - for (int i = 0; i < kLoopIters; ++i) + auto cloneAndDestroy = [&](std::size_t threadId) { + for (auto i = 0uz; i < kLoopIters; ++i) { // ------ Sync Point ------ loopStartSyncPoint.arriveAndWait(); @@ -582,14 +581,10 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_variant) }; std::vector threads; threads.reserve(kNumThreads); - for (int i = 0; i < kNumThreads; ++i) - { + for (auto i = 0uz; i < kNumThreads; ++i) threads.emplace_back(cloneAndDestroy, i); - } - for (int i = 0; i < kNumThreads; ++i) - { + for (auto i = 0uz; i < kNumThreads; ++i) threads[i].join(); - } } TEST(IntrusiveSharedTest, multithreaded_clear_mixed_union) @@ -628,7 +623,7 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_union) EXPECT_FALSE(partialDeleteRan || destructorRan); setPartialDeleteRan(); } - if (next == Deleted) + else if (next == Deleted) { EXPECT_FALSE(destructorRan); setDestructorRan(); @@ -638,16 +633,16 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_union) [&](auto const& toClone, std::default_random_engine& eng) -> std::vector> { std::vector> result; - std::uniform_int_distribution<> toCreateDist(4, 64); + std::uniform_int_distribution toCreateDist(4, 64); auto numToCreate = toCreateDist(eng); result.reserve(numToCreate); - for (int i = 0; i < numToCreate; ++i) + for (auto i = 0uz; i < numToCreate; ++i) result.emplace_back(SharedIntrusive(toClone)); return result; }; - constexpr int kLoopIters = 2 * 1024; - constexpr int kFlipPointersLoopIters = 256; - constexpr int kNumThreads = 16; + constexpr auto kLoopIters = 2uz * 1024; + constexpr auto kFlipPointersLoopIters = 256uz; + constexpr auto kNumThreads = 16uz; std::vector> toClone; Barrier loopStartSyncPoint{kNumThreads}; Barrier postCreateToCloneSyncPoint{kNumThreads}; @@ -657,7 +652,7 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_union) std::random_device rd; std::vector result; result.reserve(kNumThreads); - for (int i = 0; i < kNumThreads; ++i) + for (auto i = 0uz; i < kNumThreads; ++i) result.emplace_back(rd()); return result; }(); @@ -666,8 +661,8 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_union) // mixed strong and weak pointers, runs a loop that randomly // changes strong pointers to weak pointers, and destroys them // all at once. - auto cloneAndDestroy = [&](int threadId) { - for (int i = 0; i < kLoopIters; ++i) + auto cloneAndDestroy = [&](std::size_t threadId) { + for (auto i = 0uz; i < kLoopIters; ++i) { // ------ Sync Point ------ loopStartSyncPoint.arriveAndWait(); @@ -702,7 +697,7 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_union) postCreateVecOfPointersSyncPoint.arriveAndWait(); std::uniform_int_distribution<> isStrongDist(0, 1); - for (int f = 0; f < kFlipPointersLoopIters; ++f) + for (auto f = 0uz; f < kFlipPointersLoopIters; ++f) { for (auto& p : v) { @@ -725,14 +720,10 @@ TEST(IntrusiveSharedTest, multithreaded_clear_mixed_union) }; std::vector threads; threads.reserve(kNumThreads); - for (int i = 0; i < kNumThreads; ++i) - { + for (auto i = 0uz; i < kNumThreads; ++i) threads.emplace_back(cloneAndDestroy, i); - } - for (int i = 0; i < kNumThreads; ++i) - { + for (auto i = 0uz; i < kNumThreads; ++i) threads[i].join(); - } } TEST(IntrusiveSharedTest, multithreaded_locking_weak) @@ -766,16 +757,16 @@ TEST(IntrusiveSharedTest, multithreaded_locking_weak) EXPECT_FALSE(partialDeleteRan || destructorRan); setPartialDeleteRan(); } - if (next == Deleted) + else if (next == Deleted) { EXPECT_FALSE(destructorRan); setDestructorRan(); } }; - constexpr int kLoopIters = 2 * 1024; - constexpr int kLockWeakLoopIters = 256; - constexpr int kNumThreads = 16; + constexpr auto kLoopIters = 2uz * 1024; + constexpr auto kLockWeakLoopIters = 256uz; + constexpr auto kNumThreads = 16uz; std::vector> toLock; Barrier loopStartSyncPoint{kNumThreads}; Barrier postCreateToLockSyncPoint{kNumThreads}; @@ -784,8 +775,8 @@ TEST(IntrusiveSharedTest, multithreaded_locking_weak) // lockAndDestroy creates weak pointers from the strong pointer // and runs a loop that locks the weak pointer. At the end of the loop // all the pointers are destroyed all at once. - auto lockAndDestroy = [&](int threadId) { - for (int i = 0; i < kLoopIters; ++i) + auto lockAndDestroy = [&](std::size_t threadId) { + for (auto i = 0uz; i < kLoopIters; ++i) { // ------ Sync Point ------ loopStartSyncPoint.arriveAndWait(); @@ -816,7 +807,7 @@ TEST(IntrusiveSharedTest, multithreaded_locking_weak) // Multiple threads all create a weak pointer from the same // strong pointer WeakIntrusive const weak{toLock[threadId]}; - for (int wi = 0; wi < kLockWeakLoopIters; ++wi) + for (auto wi = 0uz; wi < kLockWeakLoopIters; ++wi) { EXPECT_FALSE(weak.expired()); auto strong = weak.lock(); @@ -831,14 +822,10 @@ TEST(IntrusiveSharedTest, multithreaded_locking_weak) }; std::vector threads; threads.reserve(kNumThreads); - for (int i = 0; i < kNumThreads; ++i) - { + for (auto i = 0uz; i < kNumThreads; ++i) threads.emplace_back(lockAndDestroy, i); - } - for (int i = 0; i < kNumThreads; ++i) - { + for (auto i = 0uz; i < kNumThreads; ++i) threads[i].join(); - } } } // namespace xrpl::tests diff --git a/src/tests/libxrpl/basics/MallocTrim.cpp b/src/tests/libxrpl/basics/MallocTrim.cpp index 6ac8957f0eb..52151262b01 100644 --- a/src/tests/libxrpl/basics/MallocTrim.cpp +++ b/src/tests/libxrpl/basics/MallocTrim.cpp @@ -199,7 +199,7 @@ TEST(mallocTrim, repeated_calls) beast::Journal const journal{beast::Journal::getNullSink()}; // Call malloc_trim multiple times to ensure it's safe - for (int i = 0; i < 5; ++i) + for (auto i = 0uz; i < 5; ++i) { MallocTrimReport const report = mallocTrim("iteration_" + std::to_string(i), journal); diff --git a/src/tests/libxrpl/basics/Number.cpp b/src/tests/libxrpl/basics/Number.cpp index 36e1b4a700b..32f93eb1f7a 100644 --- a/src/tests/libxrpl/basics/Number.cpp +++ b/src/tests/libxrpl/basics/Number.cpp @@ -323,11 +323,11 @@ TEST(NumberTest, add) __LINE__, }, { - // Does not round. Mantissas are going to be > maxRep, so if + // Does not round. Mantissas are going to be > kMaxRep, so if // added together as uint64_t's, the result will overflow. // With addition using uint128_t, there's no problem. After // normalizing, the resulting mantissa ends up less than - // maxRep. + // kMaxRep. Number{false, 9'999'999'999'999'999'990ULL, 0, Number::Normalized{}}, Number{false, 9'999'999'999'999'999'990ULL, 0, Number::Normalized{}}, Number{false, 1'999'999'999'999'999'998ULL, 1, Number::Normalized{}}, @@ -1078,14 +1078,6 @@ TEST(NumberTest, root) EXPECT_EQ(result, z) << ss.str(); } }; - /* - auto tests = [&](auto const& cSmall, auto const& cLarge) { - test(cSmall); - if (scale != MantissaRange::mantissa_scale::small) - test(cLarge); - }; - */ - auto const cSmall = std::to_array( {{Number{2}, 2, Number{1414213562373095049, -18}}, {Number{2'000'000}, 2, Number{1414213562373095049, -15}}, @@ -1511,7 +1503,7 @@ TEST(NumberTest, to_string) NumberRoundModeGuard const mg(Number::RoundingMode::TowardsZero); auto const maxMantissa = Number::maxMantissa(); - EXPECT_EQ(maxMantissa, (9'999'999'999'999'999)); + EXPECT_EQ(maxMantissa, 9'999'999'999'999'999); test( Number{false, (maxMantissa * 1000) + 999, -3, Number::Normalized()}, "9999999999999999", @@ -1550,7 +1542,7 @@ TEST(NumberTest, to_string) NumberRoundModeGuard const mg(Number::RoundingMode::TowardsZero); auto const maxMantissa = Number::maxMantissa(); - EXPECT_EQ((maxMantissa), (9'999'999'999'999'999'999ULL)); + EXPECT_EQ(maxMantissa, 9'999'999'999'999'999'999ULL); test( Number{false, maxMantissa, 0, Number::Normalized{}}, "9999999999999999990", diff --git a/src/tests/libxrpl/basics/base58.cpp b/src/tests/libxrpl/basics/base58.cpp index d452453f767..d6b1d2c3f9c 100644 --- a/src/tests/libxrpl/basics/base58.cpp +++ b/src/tests/libxrpl/basics/base58.cpp @@ -151,7 +151,7 @@ randomBigInt(std::uint8_t minSize = 1, std::uint8_t maxSize = 5) auto const numCoeff = numCoeffDist(eng); std::vector coeffs; coeffs.reserve(numCoeff); - for (int i = 0; i < numCoeff; ++i) + for (auto i = 0uz; i < numCoeff; ++i) { coeffs.push_back(dist(eng)); } @@ -167,7 +167,7 @@ TEST(Base58Test, multiprecision) auto eng = randEngine(); std::uniform_int_distribution dist; std::uniform_int_distribution dist1(1); - for (int i = 0; i < kIters; ++i) + for (auto i = 0uz; i < kIters; ++i) { std::uint64_t const d = dist(eng); if (d == 0u) @@ -185,7 +185,7 @@ TEST(Base58Test, multiprecision) EXPECT_EQ(refMod.convert_to(), mod); EXPECT_EQ(foundDiv, refDiv); } - for (int i = 0; i < kIters; ++i) + for (auto i = 0uz; i < kIters; ++i) { std::uint64_t const d = dist(eng); auto bigInt = multiprecision_utils::randomBigInt(/*minSize*/ 2); @@ -204,7 +204,7 @@ TEST(Base58Test, multiprecision) auto const foundAdd = multiprecision_utils::toBoostMP(bigInt); EXPECT_EQ(refAdd, foundAdd); } - for (int i = 0; i < kIters; ++i) + for (auto i = 0uz; i < kIters; ++i) { std::uint64_t const d = dist1(eng); // Force overflow @@ -221,7 +221,7 @@ TEST(Base58Test, multiprecision) auto const foundAdd = multiprecision_utils::toBoostMP(bigInt); EXPECT_NE(refAdd, foundAdd); } - for (int i = 0; i < kIters; ++i) + for (auto i = 0uz; i < kIters; ++i) { std::uint64_t const d = dist(eng); auto bigInt = multiprecision_utils::randomBigInt(/* minSize */ 2); @@ -239,7 +239,7 @@ TEST(Base58Test, multiprecision) auto const foundMul = multiprecision_utils::toBoostMP(bigInt); EXPECT_EQ(refMul, foundMul); } - for (int i = 0; i < kIters; ++i) + for (auto i = 0uz; i < kIters; ++i) { std::uint64_t const d = dist1(eng); // Force overflow @@ -265,7 +265,7 @@ TEST(Base58Test, fast_matches_ref) std::array b256ResultBuf[2]; std::array, 2> b256Result; - for (int i = 0; i < 2; ++i) + for (auto i = 0uz; i < 2; ++i) { std::span const outBuf{b58ResultBuf[i]}; if (i == 0) @@ -297,7 +297,7 @@ TEST(Base58Test, fast_matches_ref) } } - for (int i = 0; i < 2; ++i) + for (auto i = 0uz; i < 2; ++i) { std::span const outBuf{b256ResultBuf[i].data(), b256ResultBuf[i].size()}; if (i == 0) @@ -339,7 +339,7 @@ TEST(Base58Test, fast_matches_ref) std::array b256ResultBuf[2]; std::array, 2> b256Result; - for (int i = 0; i < 2; ++i) + for (auto i = 0uz; i < 2; ++i) { std::span const outBuf{b58ResultBuf[i].data(), b58ResultBuf[i].size()}; if (i == 0) @@ -370,7 +370,7 @@ TEST(Base58Test, fast_matches_ref) } } - for (int i = 0; i < 2; ++i) + for (auto i = 0uz; i < 2; ++i) { std::span const outBuf{b256ResultBuf[i].data(), b256ResultBuf[i].size()}; if (i == 0) @@ -425,7 +425,7 @@ TEST(Base58Test, fast_matches_ref) // test with random data constexpr std::size_t kIters = 100000; - for (int i = 0; i < kIters; ++i) + for (auto i = 0uz; i < kIters; ++i) { std::array b256DataBuf{}; auto const [tokType, b256Data] = randomB256TestData(b256DataBuf); diff --git a/src/tests/libxrpl/basics/base_uint.cpp b/src/tests/libxrpl/basics/base_uint.cpp index 174cc33aa04..43d3002c220 100644 --- a/src/tests/libxrpl/basics/base_uint.cpp +++ b/src/tests/libxrpl/basics/base_uint.cpp @@ -6,6 +6,7 @@ #include +#include #include #include @@ -59,18 +60,21 @@ struct BaseUintTest : public ::testing::Test static void testComparisons() { + using HexPair = std::pair; + { - static constexpr std::array, 6> kTestArgs{ - {{"0000000000000000", "0000000000000001"}, - {"0000000000000000", "ffffffffffffffff"}, - {"1234567812345678", "2345678923456789"}, - {"8000000000000000", "8000000000000001"}, - {"aaaaaaaaaaaaaaa9", "aaaaaaaaaaaaaaaa"}, - {"fffffffffffffffe", "ffffffffffffffff"}}}; - - for (auto const& arg : kTestArgs) + static constexpr auto kTestArgs = std::to_array({ + {"0000000000000000", "0000000000000001"}, + {"0000000000000000", "ffffffffffffffff"}, + {"1234567812345678", "2345678923456789"}, + {"8000000000000000", "8000000000000001"}, + {"aaaaaaaaaaaaaaa9", "aaaaaaaaaaaaaaaa"}, + {"fffffffffffffffe", "ffffffffffffffff"}, + }); + + for (auto const& [smallerText, largerText] : kTestArgs) { - xrpl::BaseUInt<64> const u{arg.first}, v{arg.second}; + xrpl::BaseUInt<64> const u{smallerText}, v{largerText}; // For code readability, we want to use general boolean // expectations instead of specific EXPECT_LT etc. EXPECT_TRUE(u < v); @@ -91,19 +95,18 @@ struct BaseUintTest : public ::testing::Test } { - static constexpr std::array, 6> kTestArgs{ - { - {"000000000000000000000000", "000000000000000000000001"}, - {"000000000000000000000000", "ffffffffffffffffffffffff"}, - {"0123456789ab0123456789ab", "123456789abc123456789abc"}, - {"555555555555555555555555", "55555555555a555555555555"}, - {"aaaaaaaaaaaaaaa9aaaaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaa"}, - {"fffffffffffffffffffffffe", "ffffffffffffffffffffffff"}, - }}; - - for (auto const& arg : kTestArgs) + static constexpr auto kTestArgs = std::to_array({ + {"000000000000000000000000", "000000000000000000000001"}, + {"000000000000000000000000", "ffffffffffffffffffffffff"}, + {"0123456789ab0123456789ab", "123456789abc123456789abc"}, + {"555555555555555555555555", "55555555555a555555555555"}, + {"aaaaaaaaaaaaaaa9aaaaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaa"}, + {"fffffffffffffffffffffffe", "ffffffffffffffffffffffff"}, + }); + + for (auto const& [smallerText, largerText] : kTestArgs) { - xrpl::BaseUInt<96> const u{arg.first}, v{arg.second}; + xrpl::BaseUInt<96> const u{smallerText}, v{largerText}; EXPECT_TRUE(u < v); EXPECT_TRUE(u <= v); EXPECT_TRUE(u != v); @@ -203,125 +206,119 @@ TEST_F(BaseUintTest, base_uint) Blob const raw{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; EXPECT_EQ(BaseUInt96::kBytes, raw.size()); - BaseUInt96 u = BaseUInt96::fromRaw(raw); - uset.insert(u); - EXPECT_EQ(raw.size(), u.size()); - EXPECT_EQ(to_string(u), "0102030405060708090A0B0C"); - EXPECT_EQ(toShortString(u), "01020304..."); - EXPECT_EQ(*u.data(), 1); - EXPECT_EQ(u.signum(), 1); - EXPECT_FALSE(!u); - EXPECT_FALSE(u.isZero()); - EXPECT_TRUE(u.isNonZero()); - unsigned char t = 0; - for (auto& d : u) - { - EXPECT_EQ(d, ++t); - } - - // Test hash_append by "hashing" with a no-op hasher (h) + BaseUInt96 ascending = BaseUInt96::fromRaw(raw); + uset.insert(ascending); + EXPECT_EQ(raw.size(), ascending.size()); + EXPECT_EQ(to_string(ascending), "0102030405060708090A0B0C"); + EXPECT_EQ(toShortString(ascending), "01020304..."); + EXPECT_EQ(*ascending.data(), 1); + EXPECT_EQ(ascending.signum(), 1); + EXPECT_FALSE(!ascending); + EXPECT_FALSE(ascending.isZero()); + EXPECT_TRUE(ascending.isNonZero()); + unsigned char expectedByte = 0; + for (auto& byte : ascending) + EXPECT_EQ(byte, ++expectedByte); + + // Test hash_append by "hashing" with a no-op hasher (hasher) // and then extracting the bytes that were written during hashing - // back into another base_uint (w) for comparison with the original - Nonhash<96> h{}; - hash_append(h, u); - BaseUInt96 const w = - BaseUInt96::fromRaw(std::vector(h.data.begin(), h.data.end())); - EXPECT_EQ(w, u); - - BaseUInt96 v{~u}; - uset.insert(v); - EXPECT_EQ(to_string(v), "FEFDFCFBFAF9F8F7F6F5F4F3"); - EXPECT_EQ(toShortString(v), "FEFDFCFB..."); - EXPECT_EQ(*v.data(), 0xfe); - EXPECT_EQ(v.signum(), 1); - EXPECT_FALSE(!v); - EXPECT_FALSE(v.isZero()); - EXPECT_TRUE(v.isNonZero()); - - t = 0xff; - for (auto& d : v) - { - EXPECT_EQ(d, --t); - } - - EXPECT_LT(u, v); - EXPECT_GT(v, u); - - v = u; - EXPECT_EQ(v, u); - - BaseUInt96 z{beast::kZero}; - uset.insert(z); - EXPECT_EQ(to_string(z), "000000000000000000000000"); - EXPECT_EQ(toShortString(z), "00000000..."); - EXPECT_EQ(*z.data(), 0); - EXPECT_EQ(*z.begin(), 0); - EXPECT_EQ(*std::prev(z.end(), 1), 0); - EXPECT_EQ(z.signum(), 0); - EXPECT_TRUE(!z); - EXPECT_TRUE(z.isZero()); - EXPECT_FALSE(z.isNonZero()); - for (auto& d : z) - { - EXPECT_EQ(d, 0); - } + // back into another base_uint (rehashed) for comparison with the original + Nonhash<96> hasher{}; + hash_append(hasher, ascending); + BaseUInt96 const rehashed = + BaseUInt96::fromRaw(std::vector(hasher.data.begin(), hasher.data.end())); + EXPECT_EQ(rehashed, ascending); + + BaseUInt96 complement{~ascending}; + uset.insert(complement); + EXPECT_EQ(to_string(complement), "FEFDFCFBFAF9F8F7F6F5F4F3"); + EXPECT_EQ(toShortString(complement), "FEFDFCFB..."); + EXPECT_EQ(*complement.data(), 0xfe); + EXPECT_EQ(complement.signum(), 1); + EXPECT_FALSE(!complement); + EXPECT_FALSE(complement.isZero()); + EXPECT_TRUE(complement.isNonZero()); + + expectedByte = 0xff; + for (auto& byte : complement) + EXPECT_EQ(byte, --expectedByte); + + EXPECT_LT(ascending, complement); + EXPECT_GT(complement, ascending); + + complement = ascending; + EXPECT_EQ(complement, ascending); + + BaseUInt96 zero{beast::kZero}; + uset.insert(zero); + EXPECT_EQ(to_string(zero), "000000000000000000000000"); + EXPECT_EQ(toShortString(zero), "00000000..."); + EXPECT_EQ(*zero.data(), 0); + EXPECT_EQ(*zero.begin(), 0); + EXPECT_EQ(*std::prev(zero.end(), 1), 0); + EXPECT_EQ(zero.signum(), 0); + EXPECT_TRUE(!zero); + EXPECT_TRUE(zero.isZero()); + EXPECT_FALSE(zero.isNonZero()); + for (auto& byte : zero) + EXPECT_EQ(byte, 0); { // There are several ways to create a zero. beast::kZero is tested above. Test some // others. - BaseUInt96 const z1; - EXPECT_EQ(z1, z) << to_string(z1); + BaseUInt96 const defaultZero; + EXPECT_EQ(defaultZero, zero) << to_string(defaultZero); - BaseUInt96 const z2{}; - EXPECT_EQ(z2, z) << to_string(z2); + BaseUInt96 const bracedZero{}; + EXPECT_EQ(bracedZero, zero) << to_string(bracedZero); - BaseUInt96 const z3{0u}; - EXPECT_EQ(z3, z) << to_string(z3); + BaseUInt96 const zeroFromUInt{0u}; + EXPECT_EQ(zeroFromUInt, zero) << to_string(zeroFromUInt); } - BaseUInt96 n{z}; - n++; - EXPECT_EQ(n, BaseUInt96(1)); - n--; - EXPECT_EQ(n, beast::kZero); - EXPECT_EQ(n, z); - n--; - EXPECT_EQ(to_string(n), "FFFFFFFFFFFFFFFFFFFFFFFF"); - EXPECT_EQ(toShortString(n), "FFFFFFFF..."); - n = beast::kZero; - EXPECT_EQ(n, z); - - BaseUInt96 zp1{z}; - zp1++; - BaseUInt96 zm1{z}; - zm1--; - BaseUInt96 const x{zm1 ^ zp1}; - uset.insert(x); - EXPECT_EQ(to_string(x), "FFFFFFFFFFFFFFFFFFFFFFFE") << to_string(x); - EXPECT_EQ(toShortString(x), "FFFFFFFF...") << toShortString(x); + BaseUInt96 counter{zero}; + counter++; + EXPECT_EQ(counter, BaseUInt96(1)); + counter--; + EXPECT_EQ(counter, beast::kZero); + EXPECT_EQ(counter, zero); + counter--; + EXPECT_EQ(to_string(counter), "FFFFFFFFFFFFFFFFFFFFFFFF"); + EXPECT_EQ(toShortString(counter), "FFFFFFFF..."); + counter = beast::kZero; + EXPECT_EQ(counter, zero); + + BaseUInt96 zeroPlusOne{zero}; + zeroPlusOne++; + BaseUInt96 zeroMinusOne{zero}; + zeroMinusOne--; + BaseUInt96 const xored{zeroMinusOne ^ zeroPlusOne}; + uset.insert(xored); + EXPECT_EQ(to_string(xored), "FFFFFFFFFFFFFFFFFFFFFFFE") << to_string(xored); + EXPECT_EQ(toShortString(xored), "FFFFFFFF...") << toShortString(xored); EXPECT_EQ(uset.size(), 4); - BaseUInt96 tmp; - EXPECT_TRUE(tmp.parseHex(to_string(u))); - EXPECT_EQ(tmp, u); - tmp = z; + BaseUInt96 parsed; + EXPECT_TRUE(parsed.parseHex(to_string(ascending))); + EXPECT_EQ(parsed, ascending); + parsed = zero; // fails with extra char - EXPECT_FALSE(tmp.parseHex("A" + to_string(u))); - tmp = z; + EXPECT_FALSE(parsed.parseHex("A" + to_string(ascending))); + parsed = zero; // fails with extra char at end - EXPECT_FALSE(tmp.parseHex(to_string(u) + "A")); + EXPECT_FALSE(parsed.parseHex(to_string(ascending) + "A")); // fails with a non-hex character at some point in the string: - tmp = z; + parsed = zero; for (std::size_t i = 0; i != 24; ++i) { - std::string x = to_string(z); - x[i] = ('G' + (i % 10)); - EXPECT_FALSE(tmp.parseHex(x)); + std::string xored = to_string(zero); + xored[i] = ('G' + (i % 10)); + EXPECT_FALSE(parsed.parseHex(xored)); } // Walking 1s: @@ -330,8 +327,8 @@ TEST_F(BaseUintTest, base_uint) std::string s1 = "000000000000000000000000"; s1[i] = '1'; - EXPECT_TRUE(tmp.parseHex(s1)); - EXPECT_EQ(to_string(tmp), s1); + EXPECT_TRUE(parsed.parseHex(s1)); + EXPECT_EQ(to_string(parsed), s1); } // Walking 0s: @@ -340,8 +337,8 @@ TEST_F(BaseUintTest, base_uint) std::string s1 = "111111111111111111111111"; s1[i] = '0'; - EXPECT_TRUE(tmp.parseHex(s1)); - EXPECT_EQ(to_string(tmp), s1); + EXPECT_TRUE(parsed.parseHex(s1)); + EXPECT_EQ(to_string(parsed), s1); } // Constexpr constructors @@ -355,39 +352,27 @@ TEST_F(BaseUintTest, base_uint) // Using the constexpr constructor in a non-constexpr context // with an error in the parsing throws an exception. { - // Invalid length for string. - bool caught = false; - try - { - // Try to prevent constant evaluation. - std::vector str(23, '7'); + // Invalid length for string. The vector keeps this out of a constant + // expression, so the constructor throws instead of failing to compile. + auto tooShort = [] { + std::vector const str(23, '7'); std::string_view const sView(str.data(), str.size()); [[maybe_unused]] BaseUInt96 const t96(sView); - } - catch (std::invalid_argument const& e) - { - EXPECT_EQ(e.what(), std::string("invalid length for hex string")); - caught = true; - } - EXPECT_TRUE(caught); + }; + EXPECT_THAT( + tooShort, + ::testing::ThrowsMessage("invalid length for hex string")); } { // Invalid character in string. - bool caught = false; - try - { - // Try to prevent constant evaluation. + auto badCharacter = [] { std::vector str(23, '7'); str.push_back('G'); std::string_view const sView(str.data(), str.size()); [[maybe_unused]] BaseUInt96 const t96(sView); - } - catch (std::range_error const& e) - { - EXPECT_EQ(e.what(), std::string("invalid hex character")); - caught = true; - } - EXPECT_TRUE(caught); + }; + EXPECT_THAT( + badCharacter, ::testing::ThrowsMessage("invalid hex character")); } // Verify that constexpr base_uints interpret a string the same @@ -401,20 +386,20 @@ TEST_F(BaseUintTest, base_uint) { } }; - constexpr StrBaseUInt kTestCases[] = { + constexpr auto kTestCases = std::to_array({ "000000000000000000000000", "000000000000000000000001", "fedcba9876543210ABCDEF91", "19FEDCBA0123456789abcdef", "800000000000000000000000", "fFfFfFfFfFfFfFfFfFfFfFfF", - }; + }); - for (StrBaseUInt const& t : kTestCases) + for (StrBaseUInt const& expectedByte : kTestCases) { BaseUInt96 t96; - EXPECT_TRUE(t96.parseHex(t.str)); - EXPECT_EQ(t96, t.tst); + EXPECT_TRUE(t96.parseHex(expectedByte.str)); + EXPECT_EQ(t96, expectedByte.tst); } } } diff --git a/src/tests/libxrpl/basics/join.cpp b/src/tests/libxrpl/basics/join.cpp index 66c832678b7..427f0b42bcf 100644 --- a/src/tests/libxrpl/basics/join.cpp +++ b/src/tests/libxrpl/basics/join.cpp @@ -19,11 +19,11 @@ struct JoinTest : public ::testing::Test TEST_F(JoinTest, join) { - auto test = [](auto collectionanddelimiter, std::string expected) { + auto test = [](auto collectionAndDelimiter, std::string expected) { std::stringstream ss; // Put something else in the buffer before and after to ensure that // the << operator returns the stream correctly. - ss << "(" << collectionanddelimiter << ")"; + ss << "(" << collectionAndDelimiter << ")"; auto const str = ss.str(); EXPECT_EQ(str.substr(1, str.length() - 2), expected); EXPECT_EQ(str.front(), '('); diff --git a/src/tests/libxrpl/consensus/CensorshipDetector.cpp b/src/tests/libxrpl/consensus/CensorshipDetector.cpp index aa6b2d086be..2c6b6ec731b 100644 --- a/src/tests/libxrpl/consensus/CensorshipDetector.cpp +++ b/src/tests/libxrpl/consensus/CensorshipDetector.cpp @@ -69,7 +69,7 @@ TEST(CensorshipDetectorTest, censorship_detector) runRound(cdet, ++round, {23, 24, 25, 26}, {25, 27}, {23, 26}, {24}); runRound(cdet, ++round, {23, 26, 28}, {26, 28}, {23}, {}); - for (int i = 0; i != 10; ++i) + for (auto i = 0uz; i != 10; ++i) runRound(cdet, ++round, {23}, {}, {23}, {}); runRound(cdet, ++round, {23, 29}, {29}, {23}, {}); diff --git a/src/tests/libxrpl/csf/TrustGraph.h b/src/tests/libxrpl/csf/TrustGraph.h index d010b954e08..8a804fcd5bc 100644 --- a/src/tests/libxrpl/csf/TrustGraph.h +++ b/src/tests/libxrpl/csf/TrustGraph.h @@ -118,9 +118,9 @@ class TrustGraph std::vector res; // Loop over all pairs of uniqueUNLs - for (int i = 0; i < uniqueUNLs.size(); ++i) + for (auto i = 0uz; i < uniqueUNLs.size(); ++i) { - for (int j = (i + 1); j < uniqueUNLs.size(); ++j) + for (auto j = i + 1; j < uniqueUNLs.size(); ++j) { auto const& unlA = uniqueUNLs[i]; auto const& unlB = uniqueUNLs[j]; diff --git a/src/tests/libxrpl/csf/random.h b/src/tests/libxrpl/csf/random.h index 007bdecb1b6..56838bb2806 100644 --- a/src/tests/libxrpl/csf/random.h +++ b/src/tests/libxrpl/csf/random.h @@ -24,11 +24,12 @@ randomWeightedShuffle(std::vector v, std::vector w, G& g) { using std::swap; - for (int i = 0; i < v.size() - 1; ++i) + for (auto i = 0uz; i + 1 < v.size(); ++i) { - // pick a random item weighted by w - std::discrete_distribution<> dd(w.begin() + i, w.end()); // NOLINT(misc-const-correctness) - auto idx = dd(g); + // Pick a random item from the unplaced tail, weighted by w. + // NOLINTNEXTLINE(misc-const-correctness) + std::discrete_distribution dd(w.begin() + i, w.end()); + auto const idx = i + dd(g); std::swap(v[i], v[idx]); std::swap(w[i], w[idx]); } diff --git a/src/tests/libxrpl/nodestore/Database.cpp b/src/tests/libxrpl/nodestore/Database.cpp index 23087a2f84b..82012ed347e 100644 --- a/src/tests/libxrpl/nodestore/Database.cpp +++ b/src/tests/libxrpl/nodestore/Database.cpp @@ -27,11 +27,11 @@ namespace { std::vector allBackends() { - std::vector types{"memory", "nudb"}; #if XRPL_ROCKSDB_AVAILABLE - types.emplace_back("rocksdb"); + return {"memory", "nudb", "rocksdb"}; +#else + return {"memory", "nudb"}; #endif - return types; } std::vector diff --git a/src/tests/libxrpl/resource/Logic.cpp b/src/tests/libxrpl/resource/Logic.cpp index 1f935ebf4b3..e49627ae344 100644 --- a/src/tests/libxrpl/resource/Logic.cpp +++ b/src/tests/libxrpl/resource/Logic.cpp @@ -17,9 +17,11 @@ #include #include +#include #include #include #include +#include namespace xrpl::Resource { @@ -71,7 +73,7 @@ class ResourceManagerTest : public ::testing::Test static_cast(v + i), }}; item.address = beast::IP::Endpoint{beast::IP::AddressV4{d}}; - gossip.items.push_back(item); + gossip.items.push_back(std::move(item)); } } }; @@ -87,8 +89,8 @@ TEST_F(ResourceManagerTest, limited_warn_drop) Consumer c{logic.newInboundEndpoint(addr)}; // Create load until we get a warning - int n = 10000; - bool warned = false; + auto n = 10000; + auto warned = false; while (--n >= 0) { @@ -97,7 +99,7 @@ TEST_F(ResourceManagerTest, limited_warn_drop) warned = true; break; } - ++logic.clock(); + logic.advance(); } ASSERT_TRUE(warned) << "Loop count exceeded without warning"; @@ -113,7 +115,7 @@ TEST_F(ResourceManagerTest, limited_warn_drop) EXPECT_TRUE(c.disconnect(j_)); break; } - ++logic.clock(); + logic.advance(); } ASSERT_TRUE(dropped) << "Loop count exceeded without dropping"; @@ -135,7 +137,7 @@ TEST_F(ResourceManagerTest, limited_warn_drop) auto n = kSecondsUntilExpiration + 1s; while (--n > 0s) { - ++logic.clock(); + logic.advance(); logic.periodicActivity(); Consumer const c{logic.newInboundEndpoint(addr)}; if (c.disposition() != Disposition::Drop) @@ -167,7 +169,7 @@ TEST_F(ResourceManagerTest, unlimited_warn_drop) warned = true; break; } - ++logic.clock(); + logic.advance(); } EXPECT_FALSE(warned) << "Should loop forever with no warning"; @@ -175,6 +177,8 @@ TEST_F(ResourceManagerTest, unlimited_warn_drop) TEST_F(ResourceManagerTest, charges) { + static constexpr auto kDecayTicks = 128uz; + TestLogic logic{j_}; { @@ -183,7 +187,7 @@ TEST_F(ResourceManagerTest, charges) Charge const fee{1000}; JLOG(j_.info()) << "Charging " << c.toString() << " " << fee << " per second"; c.charge(fee); - for (int i = 0; i < 128; ++i) + for (auto tick = 0uz; tick < kDecayTicks; ++tick) { JLOG(j_.info()) << "Time= " << logic.clock().now().time_since_epoch().count() << ", Balance = " << c.balance(); @@ -196,7 +200,7 @@ TEST_F(ResourceManagerTest, charges) Consumer c{logic.newInboundEndpoint(address)}; Charge const fee{1000}; JLOG(j_.info()) << "Charging " << c.toString() << " " << fee << " per second"; - for (int i = 0; i < 128; ++i) + for (auto tick = 0uz; tick < kDecayTicks; ++tick) { c.charge(fee); JLOG(j_.info()) << "Time= " << logic.clock().now().time_since_epoch().count() @@ -210,13 +214,15 @@ TEST_F(ResourceManagerTest, imports) { TestLogic logic{j_}; - Gossip g[5]; + static constexpr auto kGossipSources = 5uz; + + std::array gossip; - for (auto& i : g) - populateGossip(i); + for (auto& g : gossip) + populateGossip(g); - for (int i = 0; i < 5; ++i) - logic.importConsumers(std::to_string(i), g[i]); + for (auto i = 0uz; i < gossip.size(); ++i) + logic.importConsumers(std::to_string(i), gossip[i]); } TEST_F(ResourceManagerTest, import) @@ -233,7 +239,7 @@ TEST_F(ResourceManagerTest, import) 1, }}; item.address = beast::IP::Endpoint{beast::IP::AddressV4{d}}; - g.items.push_back(item); + g.items.push_back(std::move(item)); logic.importConsumers("g", g); } diff --git a/src/tests/libxrpl/shamap/SHAMap.cpp b/src/tests/libxrpl/shamap/SHAMap.cpp index 238c34bf9c4..c84cdf504f8 100644 --- a/src/tests/libxrpl/shamap/SHAMap.cpp +++ b/src/tests/libxrpl/shamap/SHAMap.cpp @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -113,7 +112,7 @@ class SHAMapTest : public ::testing::TestWithParam intToVuc(std::uint8_t v) { Buffer vuc{32}; - std::fill_n(vuc.data(), vuc.size(), v); + vuc.fill(v); return vuc; } }; @@ -257,12 +256,9 @@ TEST_P(SHAMapTest, add_traverse_snapshot_build_tear_and_iterate) map.invariants(); } - int h = 7; + auto keyIndex = kKeys.size(); for (auto const& k : map) - { - EXPECT_EQ(k.key(), kKeys[h]); - --h; - } + EXPECT_EQ(k.key(), kKeys[--keyIndex]); } } @@ -288,7 +284,11 @@ TEST_F(SHAMapPathProof, verify_proof_path) uint256 rootHash; std::vector goodPath; - for (unsigned char c = 1; c < 100; ++c) + static constexpr unsigned char kFirstKey = 1; + static constexpr unsigned char kKeyCount = 100; + static constexpr unsigned char kLastKey = kKeyCount - 1; + + for (unsigned char c = kFirstKey; c < kKeyCount; ++c) { uint256 k(c); map.addItem(SHAMapNodeType::TnAccountState, makeShamapitem(k, Slice{k.data(), k.size()})); @@ -304,7 +304,7 @@ TEST_F(SHAMapPathProof, verify_proof_path) auto& proofPath = *path; EXPECT_TRUE(map.verifyProofPath(root, k, proofPath)); - if (c == 1) + if (c == kFirstKey) { // extra node proofPath.insert(proofPath.begin(), proofPath.front()); @@ -313,7 +313,7 @@ TEST_F(SHAMapPathProof, verify_proof_path) uint256 const wrongKey(c + 1); EXPECT_FALSE(map.getProofPath(wrongKey)); } - if (c == 99) + if (c == kLastKey) { key = k; rootHash = root; diff --git a/src/tests/libxrpl/shamap/SHAMapSync.cpp b/src/tests/libxrpl/shamap/SHAMapSync.cpp index 5cefbae8a16..be23895626b 100644 --- a/src/tests/libxrpl/shamap/SHAMapSync.cpp +++ b/src/tests/libxrpl/shamap/SHAMapSync.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -34,15 +35,17 @@ class SHAMapSyncTest : public ::testing::Test boost::intrusive_ptr makeRandomAS() { + static constexpr auto kWordsPerState = 3uz; + Serializer s; - for (int d = 0; d < 3; ++d) + for (auto word = 0uz; word < kWordsPerState; ++word) s.add32(randInt(eng_)); return makeShamapitem(s.getSHA512Half(), s.slice()); } bool - confuseMap(SHAMap& map, int count) + confuseMap(SHAMap& map, std::size_t count) { // add a bunch of random states to a map, then remove them // map should be the same @@ -50,7 +53,7 @@ class SHAMapSyncTest : public ::testing::Test std::list items; - for (int i = 0; i < count; ++i) + for (auto i = 0uz; i < count; ++i) { auto item = makeRandomAS(); items.push_back(item->key()); @@ -87,26 +90,30 @@ TEST_F(SHAMapSyncTest, sync) SHAMap source{SHAMapType::FREE, f}; SHAMap destination{SHAMapType::FREE, f2}; - int const items = 10000; - for (int i = 0; i < items; ++i) + static constexpr auto kItemCount = 10000uz; + static constexpr auto kInvariantInterval = 100uz; + static constexpr auto kNodesToConfuse = 500uz; + static constexpr auto kMaxNodesPerRequest = 2048; + + for (auto i = 0uz; i < kItemCount; ++i) { source.addItem(SHAMapNodeType::TnAccountState, makeRandomAS()); - if (i % 100 == 0) + if (i % kInvariantInterval == 0) source.invariants(); } source.invariants(); - ASSERT_TRUE(confuseMap(source, 500)); + ASSERT_TRUE(confuseMap(source, kNodesToConfuse)); source.invariants(); source.setImmutable(); - int count = 0; + std::size_t count = 0; source.visitLeaves([&count]([[maybe_unused]] auto const& item) { ++count; }); - EXPECT_EQ(count, items); + EXPECT_EQ(count, kItemCount); std::vector missingNodes; - source.walkMap(missingNodes, 2048); + source.walkMap(missingNodes, kMaxNodesPerRequest); EXPECT_TRUE(missingNodes.empty()); destination.setSynching(); @@ -127,7 +134,7 @@ TEST_F(SHAMapSyncTest, sync) f.clock().advance(std::chrono::seconds(1)); // get the list of nodes we know we need - auto nodesMissing = destination.getMissingNodes(2048, nullptr); + auto nodesMissing = destination.getMissingNodes(kMaxNodesPerRequest, nullptr); if (nodesMissing.empty()) break;