Skip to content

Commit

Permalink
Improve test coverage (#4)
Browse files Browse the repository at this point in the history 
* Add test case
* Remove redundant check

Since capacity is exactly half of the freel size, it is not possible for freel to be exhaused when size is below capacity
@Bronek
Bronek authored Jan 3, 2024
1 parent 7e7d91c commit e4b5109
Showing 2 changed files with 64 additions and 18 deletions.
18 changes: 10 additions & 8 deletions libs/market/book.hpp
View file
Original file line number Diff line number Diff line change
@@ -224,13 +224,14 @@ namespace market {
ASSERT(freel != nullptr);
ASSERT(side_i[0] + side_i[1] + (size_type)(tail_i + 1) == size_i);
size_type result = npos;
auto& i = side_i[(size_t)Side];
if (i < capacity && tail_i != npos) {
auto& size = side_i[(size_t)Side];
if (size < capacity) {
ASSERT(tail_i != npos);
// Note: must post-decrement tail_i here. Will change to npos if it was 0
const auto l = freel[tail_i--];
levels[l] = std::forward<Type>(a);
sides[(size_t)Side * capacity + i] = l;
result = i++; // Note: must post-increment side_i[Side] here
sides[(size_t)Side * capacity + size] = l;
result = size++; // Note: must post-increment side_i[Side] here
}
return result;
}
@@ -240,13 +241,14 @@ namespace market {
ASSERT(freel != nullptr);
ASSERT(side_i[0] + side_i[1] + (size_type)(tail_i + 1) == size_i);
size_type result = npos;
auto& i = side_i[(size_t)Side];
if (i < capacity && tail_i != npos) {
auto& size = side_i[(size_t)Side];
if (size < capacity) {
ASSERT(tail_i != npos);
// Note: must post-decrement tail_i here. Will change to npos if it was 0
const auto l = freel[tail_i--];
common::emplace(&levels[l], std::forward<Args>(a) ...);
sides[(size_t)Side * capacity + i] = l;
result = i++; // Note: must post-increment side_i[Side] here
sides[(size_t)Side * capacity + size] = l;
result = size++; // Note: must post-increment side_i[Side] here
}
return result;
}
64 changes: 54 additions & 10 deletions tests/book.cpp
View file
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@ struct assert_error {};

namespace {
struct Level {
int ticks; int size;
int ticks = -1; int size = -1;

template <market::side Side>
constexpr static bool compare(const Level& lh, const Level& rh) noexcept;
@@ -165,6 +165,20 @@ TEST_CASE("SmallBook_basics", "[book][data][capacity][size][empty][full][at][pus
}
}

SECTION("emplace_back() allows use of default ctor") {
CHECK(book.emplace_back<side::bid>() == 0);
CHECK(book.size<side::bid>() == 1);
CHECK(not book.empty<side::bid>());
CHECK(not book.full<side::bid>());
CHECK(book.at<side::bid>(0) == Level{});

CHECK(book.emplace_back<side::ask>() == 0);
CHECK(book.size<side::ask>() == 1);
CHECK(not book.empty<side::ask>());
CHECK(not book.full<side::ask>());
CHECK(book.at<side::ask>(0) == Level{});
}

SECTION("emplace_back() different elements on each side to fill the capacity") {
REQUIRE(SmallBook::npos > 3);
CHECK(book.emplace_back<side::bid>(130130, 100) == 0);
@@ -595,22 +609,23 @@ TEST_CASE("AnySizeBook_construction", "[book][capacity][construction][bad_capaci
}

SECTION("ASSERT check") {
CHECK_NOTHROW(ptr.reset(new AnySizeBook(10, std::nothrow)));
CHECK_NOTHROW(ptr.reset(new AnySizeBook(2, std::nothrow)));
SECTION("null freel") {
ptr->base_freel() = nullptr;
CHECK_THROWS_AS(ptr->reset(), assert_error);
CHECK_THROWS_AS(ptr->accept(), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::bid>(Level{1, 1}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::ask>(Level{1, 1}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::bid>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::ask>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::bid>(1, 1), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::ask>(1, 1), assert_error);
CHECK_THROWS_AS(ptr->remove<side::bid>(0), assert_error);
CHECK_THROWS_AS(ptr->remove<side::ask>(0), assert_error);
}

SECTION("bad bid size") {
ptr->base_size_bid() = 1;
CHECK_THROWS_AS(ptr->push_back<side::bid>(Level{1, 1}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::ask>(Level{1, 1}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::bid>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::ask>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::bid>(1, 1), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::ask>(1, 1), assert_error);
CHECK_THROWS_AS(ptr->remove<side::bid>(0), assert_error);
@@ -619,8 +634,8 @@ TEST_CASE("AnySizeBook_construction", "[book][capacity][construction][bad_capaci

SECTION("bad ask size") {
ptr->base_size_ask() = 1;
CHECK_THROWS_AS(ptr->push_back<side::bid>(Level{1, 1}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::ask>(Level{1, 1}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::bid>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::ask>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::bid>(1, 1), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::ask>(1, 1), assert_error);
CHECK_THROWS_AS(ptr->remove<side::bid>(0), assert_error);
@@ -629,8 +644,8 @@ TEST_CASE("AnySizeBook_construction", "[book][capacity][construction][bad_capaci

SECTION("bad tail") {
ptr->base_tail() = 1;
CHECK_THROWS_AS(ptr->push_back<side::bid>(Level{1, 1}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::ask>(Level{1, 1}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::bid>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->push_back<side::ask>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::bid>(1, 1), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::ask>(1, 1), assert_error);
CHECK_THROWS_AS(ptr->remove<side::bid>(0), assert_error);
@@ -645,6 +660,35 @@ TEST_CASE("AnySizeBook_construction", "[book][capacity][construction][bad_capaci
CHECK_THROWS_AS(ptr->remove<side::bid>(0), assert_error);
CHECK_THROWS_AS(ptr->remove<side::ask>(0), assert_error);
}

SECTION("bad tail_i adding element") {
CHECK(ptr->push_back<side::bid>(Level{130140, 10}) == 0);
CHECK(ptr->push_back<side::bid>(Level{130130, 10}) == 1);
CHECK(ptr->push_back<side::ask>(Level{130140, 10}) == 0);
CHECK(ptr->push_back<side::ask>(Level{130150, 10}) == 1);
REQUIRE(ptr->size<side::bid>() == 2);
REQUIRE(ptr->size<side::ask>() == 2);
REQUIRE(ptr->full<side::bid>());
REQUIRE(ptr->full<side::ask>());
REQUIRE(ptr->base_tail() == npos);

// note, sections below intentionally corrupt internal state
// such that there is place to add new level without causing UB
SECTION("push_back() or emplace_back() on ask") {
ptr->base_size_bid() = 3;
ptr->base_size_ask() = 1;
CHECK_THROWS_AS(ptr->push_back<side::ask>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::ask>(), assert_error);
REQUIRE(ptr->push_back<side::bid>(Level{}) == npos);
}
SECTION("push_back() or emplace_back() on bid") {
ptr->base_size_bid() = 1;
ptr->base_size_ask() = 3;
CHECK_THROWS_AS(ptr->push_back<side::bid>(Level{}), assert_error);
CHECK_THROWS_AS(ptr->emplace_back<side::bid>(), assert_error);
REQUIRE(ptr->push_back<side::ask>(Level{}) == npos);
}
}
}
}

0 comments on commit e4b5109

Please sign in to comment.