From f3397d48c03dfbbe50d121b9861743ea5f2dcd0a Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 21 Jan 2016 16:15:25 -0800 Subject: [PATCH] Add IndexSet tests And fix a bug that resulted in ranges not being merged. --- src/index_set.cpp | 11 ++-- tests/CMakeLists.txt | 8 ++- tests/index_set.cpp | 153 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 6 deletions(-) create mode 100644 tests/index_set.cpp diff --git a/src/index_set.cpp b/src/index_set.cpp index b7e4961c..c244f76a 100644 --- a/src/index_set.cpp +++ b/src/index_set.cpp @@ -43,11 +43,12 @@ void IndexSet::do_add(iterator it, size_t index) else if (more_before && (it - 1)->second == index) { // index is immediately after an existing range ++(it - 1)->second; - } - else if (more_before && valid && (it - 1)->second == it->first) { - // index joins two existing ranges - (it - 1)->second = it->second; - m_ranges.erase(it); + + if (valid && (it - 1)->second == it->first) { + // index joins two existing ranges + (it - 1)->second = it->second; + m_ranges.erase(it); + } } else if (valid && it->first == index + 1) { // index is immediately before an existing range diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a7b20889..269301c9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,5 +1,11 @@ include_directories(../external/catch/single_include) -add_executable(tests main.cpp parser.cpp) + +set(SOURCES + index_set.cpp + main.cpp + parser.cpp) + +add_executable(tests ${SOURCES}) target_link_libraries(tests realm-object-store) add_custom_target(run-tests USES_TERMINAL DEPENDS tests COMMAND ./tests) diff --git a/tests/index_set.cpp b/tests/index_set.cpp new file mode 100644 index 00000000..50ae889c --- /dev/null +++ b/tests/index_set.cpp @@ -0,0 +1,153 @@ +#include "index_set.hpp" + +#include + +// Catch doesn't have an overload for std::pair, so define one ourselves +// The declaration needs to be before catch.hpp is included for it to be used, +// but the definition needs to be after since it uses Catch's toString() +namespace Catch { +template +std::string toString(std::pair const& value); +} + +#include "catch.hpp" + +namespace Catch { +template + std::string toString(std::pair const& value) { + return "{" + toString(value.first) + ", " + toString(value.second) + "}"; +} +} + +#define REQUIRE_RANGES(index_set, ...) do { \ + std::initializer_list> expected = {__VA_ARGS__}; \ + REQUIRE(index_set.size() == expected.size()); \ + auto begin = index_set.begin(), end = index_set.end(); \ + for (auto range : expected) { \ + REQUIRE(*begin++ == range); \ + } \ +} while (0) + +TEST_CASE("index set") { + realm::IndexSet set; + + SECTION("add() extends existing ranges") { + set.add(1); + REQUIRE_RANGES(set, {1, 2}); + + set.add(2); + REQUIRE_RANGES(set, {1, 3}); + + set.add(0); + REQUIRE_RANGES(set, {0, 3}); + } + + SECTION("add() with gaps") { + set.add(0); + REQUIRE_RANGES(set, {0, 1}); + + set.add(2); + REQUIRE_RANGES(set, {0, 1}, {2, 3}); + } + + SECTION("add() is idempotent") { + set.add(0); + set.add(0); + REQUIRE_RANGES(set, {0, 1}); + } + + SECTION("add() merges existing ranges") { + set.add(0); + set.add(2); + set.add(4); + + set.add(1); + REQUIRE_RANGES(set, {0, 3}, {4, 5}); + } + + SECTION("set() from empty") { + set.set(5); + REQUIRE_RANGES(set, {0, 5}); + } + + SECTION("set() discards existing data") { + set.add(8); + set.add(9); + + set.set(5); + REQUIRE_RANGES(set, {0, 5}); + } + + SECTION("insert_at() extends ranges containing the target index") { + set.add(5); + set.add(6); + + set.insert_at(5); + REQUIRE_RANGES(set, {5, 8}); + + set.insert_at(4); + REQUIRE_RANGES(set, {4, 5}, {6, 9}); + + set.insert_at(9); + REQUIRE_RANGES(set, {4, 5}, {6, 10}); + } + + SECTION("insert_at() does not modify ranges entirely before it") { + set.add(5); + set.add(6); + + set.insert_at(8); + REQUIRE_RANGES(set, {5, 7}, {8, 9}); + } + + SECTION("insert_at() shifts ranges after it") { + set.add(5); + set.add(6); + + set.insert_at(3); + REQUIRE_RANGES(set, {3, 4}, {6, 8}); + } + + SECTION("insert_at() cannot join ranges") { + set.add(5); + set.add(7); + + set.insert_at(6); + REQUIRE_RANGES(set, {5, 7}, {8, 9}); + } + + SECTION("add_shifted() on an empty set is just add()") { + set.add_shifted(5); + REQUIRE_RANGES(set, {5, 6}); + } + + SECTION("add_shifted() before the first range is just add()") { + set.add(10); + set.add_shifted(5); + REQUIRE_RANGES(set, {5, 6}, {10, 11}); + } + + SECTION("add_shifted() on first index of range extends range") { + set.add(5); + set.add_shifted(5); + REQUIRE_RANGES(set, {5, 7}); + + set.add_shifted(5); + REQUIRE_RANGES(set, {5, 8}); + + set.add_shifted(6); + REQUIRE_RANGES(set, {5, 8}, {9, 10}); + } + + SECTION("add_shifted() after ranges shifts by the size of those ranges") { + set.add(5); + set.add_shifted(6); + REQUIRE_RANGES(set, {5, 6}, {7, 8}); + + set.add_shifted(6); // bumped into second range + REQUIRE_RANGES(set, {5, 6}, {7, 9}); + + set.add_shifted(8); + REQUIRE_RANGES(set, {5, 6}, {7, 9}, {11, 12}); + } +}