Page MenuHomePhabricator
Differential D11571 Diff 33855 src/test/shortidprocessor_tests.cpp

Changeset View

Standalone View

View Options

src/test/shortidprocessor_tests.cpp

  • This file was added.
// Copyright (c) 2022 The Bitcoin developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <shortidprocessor.h>
#include <boost/test/unit_test.hpp>
#include <algorithm>
#include <utility>
BOOST_AUTO_TEST_SUITE(shortidprocessor_tests)
namespace {
struct PrefilledTestItem {
uint32_t index;
std::shared_ptr<uint32_t> item;
PrefilledTestItem(uint32_t indexIn)
: index(indexIn), item(std::make_shared<uint32_t>(indexIn)) {}
};
using ItemType = decltype(PrefilledTestItem::item);
struct PrefilledTestItemAdapter {
uint32_t getIndex(const PrefilledTestItem &i) const { return i.index; }
ItemType getItem(const PrefilledTestItem &i) const { return i.item; }
};
struct TestItemCompare {
bool operator()(const ItemType &lhs, const ItemType &rhs) const {
return *lhs == *rhs;
}
};
} // namespace
BOOST_AUTO_TEST_CASE(processing_items) {
using TestItemShortIdProcessor =
ShortIdProcessor<PrefilledTestItem, PrefilledTestItemAdapter,
TestItemCompare>;
{
TestItemShortIdProcessor p({}, {}, 0);
BOOST_CHECK_EQUAL(p.getItemCount(), 0);
BOOST_CHECK_EQUAL(p.getShortIdCount(), 0);
BOOST_CHECK(p.isEvenlyDistributed());
BOOST_CHECK(!p.hasShortIdCollision());
BOOST_CHECK(!p.hasOutOfBoundIndex());
}
{
// Trigger the bucket uneven distribution
TestItemShortIdProcessor p({}, {0}, 0);
sdulfariUnsubmitted
Not Done
Inline Actions

Using 0 as the bucket size seems wrong for both of the above cases since it's not intuitive, even though it certainly makes writing it easy. It raises questions like: What does a size 0 mean? Is size 0 related to any real world case that is conceivable?

I would also consider disallowing size 0 entirely but that should not block this diff.

sdulfari: Using 0 as the bucket size seems wrong for both of the above cases since it's not intuitive…
FabienAuthorUnsubmitted
Done
Inline Actions

0 is not a realistic value but is the only value that is guaranteed to trigger the flag all the time, since the number of bucket is set to the number of shortids. Other values depends on statistics and would make the test flaky. I'll add a comment for that.

Fabien: 0 is not a realistic value but is the only value that is guaranteed to trigger the flag all the…
sdulfariUnsubmitted
Not Done
Inline Actions

I got confused on the bucket size vs number of buckets. This makes sense.

sdulfari: I got confused on the bucket size vs number of buckets. This makes sense.
BOOST_CHECK_EQUAL(p.getItemCount(), 1);
BOOST_CHECK_EQUAL(p.getShortIdCount(), 1);
BOOST_CHECK(!p.isEvenlyDistributed());
BOOST_CHECK(!p.hasShortIdCollision());
BOOST_CHECK(!p.hasOutOfBoundIndex());
}
{
// Trigger the short id collision
TestItemShortIdProcessor p({}, {0, 0}, 1);
BOOST_CHECK_EQUAL(p.getItemCount(), 2);
BOOST_CHECK_EQUAL(p.getShortIdCount(), 1);
BOOST_CHECK(p.isEvenlyDistributed());
BOOST_CHECK(p.hasShortIdCollision());
BOOST_CHECK(!p.hasOutOfBoundIndex());
}
{
// Trigger the out of bound index
TestItemShortIdProcessor p({1}, {}, 1);
sdulfariUnsubmitted
Not Done
Inline Actions

This tests one specific case where the first index is out of bounds, but does not test for invalid indices following valid ones and vice versa.

sdulfari: This tests one specific case where the first index is out of bounds, but does not test for…
BOOST_CHECK_EQUAL(p.getItemCount(), 1);
BOOST_CHECK_EQUAL(p.getShortIdCount(), 0);
BOOST_CHECK(p.isEvenlyDistributed());
BOOST_CHECK(!p.hasShortIdCollision());
BOOST_CHECK(p.hasOutOfBoundIndex());
sdulfariUnsubmitted
Not Done
Inline Actions

What about shortid out of bounds indices?

sdulfari: What about shortid out of bounds indices?
FabienAuthorUnsubmitted
Done
Inline Actions

I will not create a vector with max(uint32_t) elements to check this case :)

Fabien: I will not create a vector with max(uint32_t) elements to check this case :)
sdulfariUnsubmitted
Not Done
Inline Actions

touche

sdulfari: touche
}
{
const std::vector<PrefilledTestItem> prefilledItems{0, 1, 2, 5};
const std::vector<uint64_t> shortids{3, 4, 6, 7, 8, 9};
TestItemShortIdProcessor p(prefilledItems, shortids, 10);
BOOST_CHECK_EQUAL(p.getItemCount(), 10);
BOOST_CHECK_EQUAL(p.getShortIdCount(), 6);
BOOST_CHECK(p.isEvenlyDistributed());
BOOST_CHECK(!p.hasShortIdCollision());
BOOST_CHECK(!p.hasOutOfBoundIndex());
BOOST_CHECK(p.getItem(0) == prefilledItems[0].item);
BOOST_CHECK(p.getItem(1) == prefilledItems[1].item);
BOOST_CHECK(p.getItem(2) == prefilledItems[2].item);
BOOST_CHECK(p.getItem(3) == nullptr);
BOOST_CHECK(p.getItem(4) == nullptr);
BOOST_CHECK(p.getItem(5) == prefilledItems[3].item);
BOOST_CHECK(p.getItem(6) == nullptr);
BOOST_CHECK(p.getItem(7) == nullptr);
BOOST_CHECK(p.getItem(8) == nullptr);
BOOST_CHECK(p.getItem(9) == nullptr);
// Add a missing shortid
auto item3 = std::make_shared<uint32_t>(3);
BOOST_CHECK_EQUAL(p.matchKnownItem(3, item3), 1);
BOOST_CHECK(p.getItem(3) == item3);
// If the same item is added again, it has no effect
for (size_t i = 0; i < 10; i++) {
BOOST_CHECK_EQUAL(
p.matchKnownItem(3, std::make_shared<uint32_t>(3)), 0);
BOOST_CHECK(p.getItem(3) == item3);
}
// Shortid collision, the item is removed from the available list so it
// will be requested.
BOOST_CHECK_EQUAL(p.matchKnownItem(3, std::make_shared<uint32_t>(30)),
-1);
BOOST_CHECK(p.getItem(3) == nullptr);
// Now that collision occurred, adding other items as no effect
for (size_t i = 0; i < 10; i++) {
BOOST_CHECK_EQUAL(
p.matchKnownItem(3, std::make_shared<uint32_t>(i)), 0);
BOOST_CHECK(p.getItem(3) == nullptr);
}
}
}
BOOST_AUTO_TEST_SUITE_END()