Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicola-cab
nicola-cab committed Aug 1, 2024
1 parent 195716c commit 1629181
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 49 deletions.
19 changes: 7 additions & 12 deletions test/test_string_compression.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ using namespace realm;
TEST(StringInterner_Basic_Creation)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);
parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);
StringData my_string = "aaaaaaaaaaaaaaa";
Expand All @@ -50,12 +51,12 @@ TEST(StringInterner_Basic_Creation)
CHECK_EQUAL(my_string, origin_string);

CHECK(interner.compare(*stored_id, id) == 0); // compare agaist self.
parent.destroy_deep();
}

TEST(StringInterner_InternMultipleStrings)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);
parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);

Expand All @@ -72,12 +73,12 @@ TEST(StringInterner_InternMultipleStrings)
CHECK_EQUAL(*stored_id, id);
CHECK_EQUAL(interner.compare(str, id), 0);
}
parent.destroy_deep();
}

TEST(StringInterner_TestLookup)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);
parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);

Expand All @@ -96,13 +97,12 @@ TEST(StringInterner_TestLookup)
CHECK(id);
CHECK(interner.compare(StringData(s), *id) == 0);
}

parent.destroy_deep();
}

TEST(StringInterner_VerifyComparison)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);
parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);

Expand Down Expand Up @@ -173,13 +173,12 @@ TEST(StringInterner_VerifyComparison)
res = interner.compare(test_upper_case_id, test_lower_case_id);
CHECK_LESS(interner.get(test_upper_case_id), interner.get(test_lower_case_id));
CHECK_EQUAL(res, -1);

parent.destroy_deep();
}

TEST(StringInterner_VerifyInterningNull)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);
parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);
auto null_id = interner.intern({});
Expand All @@ -203,13 +202,12 @@ TEST(StringInterner_VerifyInterningNull)
CHECK_LESS(StringData{}, interner.get(str_id)); // compare via StringData
CHECK_EQUAL(interner.compare(StringData{"test"}, null_id), 1);
CHECK_GREATER(StringData{"test"}, interner.get(null_id));

parent.destroy_deep();
}

TEST(StringInterner_VerifyLongString)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);
parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);

Expand All @@ -221,13 +219,12 @@ TEST(StringInterner_VerifyLongString)
const auto stored_id = interner.lookup(StringData(long_string));
CHECK_EQUAL(stored_id, 1);
CHECK(interner.compare(StringData(long_string), *stored_id) == 0);

parent.destroy_deep();
}

TEST(StringInterner_VerifyExpansionFromSmallStringToLongString)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);
parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);

Expand All @@ -249,6 +246,4 @@ TEST(StringInterner_VerifyExpansionFromSmallStringToLongString)
stored_id = interner.lookup(StringData(long_string));
CHECK_EQUAL(stored_id, id);
CHECK(interner.compare(StringData(long_string), *stored_id) == 0);

parent.destroy_deep();
}
62 changes: 25 additions & 37 deletions test/test_utf8.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <realm/unicode.hpp>
#include <realm/array.hpp>
#include <realm/string_interner.hpp>
#include <realm/impl/destroy_guard.hpp>

#include "test.hpp"

Expand Down Expand Up @@ -88,10 +89,22 @@ const char* u16sur2 = "\xF0\xA0\x9C\xB1"; // same as above, with larger unicode

TEST(UTF8_Compare_Strings)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);

parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);

// Useful line for creating new unit test cases:
// bool ret = std::locale("us_EN")(string("a"), std::string("b"));
auto str_compare = [](StringData a, StringData b) {
return a < b;

auto str_compare = [&interner, this](StringData a, StringData b) {
auto id1 = interner.intern(a);
auto id2 = interner.intern(b);
const auto ret_interner_cmp = interner.compare(id1, id2) < 0;
const auto ret_cmp = a < b;
CHECK_EQUAL(ret_cmp, ret_interner_cmp);
return ret_cmp;
};

// simplest test
Expand Down Expand Up @@ -144,27 +157,20 @@ TEST(UTF8_Compare_Strings)
TEST(UTF8_Compare_Core_utf8)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);

parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);

auto str_compare = [&interner, this](StringData a, StringData b) {
auto id1 = interner.lookup(a);
auto id2 = interner.lookup(b);
CHECK_EQUAL((id1 && id2), true);
const auto ret_interner_cmp = interner.compare(*id1, *id2) < 0;
auto id1 = interner.intern(a);
auto id2 = interner.intern(b);
const auto ret_interner_cmp = interner.compare(id1, id2) < 0;
const auto ret_cmp = a < b;
CHECK_EQUAL(ret_cmp, ret_interner_cmp);
return ret_cmp;
};

// intern all the utf8 strings
interner.intern(uae);
interner.intern(uAE);
interner.intern(ua);
interner.intern(uA);
interner.intern(u16sur);
interner.intern(u16sur2);

// single utf16 code points (tests mostly Windows)
CHECK_EQUAL(false, str_compare(uae, uae));
CHECK_EQUAL(false, str_compare(uAE, uAE));
Expand All @@ -188,8 +194,6 @@ TEST(UTF8_Compare_Core_utf8)
CHECK_EQUAL(true, str_compare(u16sur, u16sur2));
CHECK_EQUAL(false, str_compare(u16sur2, u16sur2));
CHECK_EQUAL(false, str_compare(u16sur2, u16sur2));

parent.destroy_deep();
}

TEST(UTF8_Compare_Core_utf8_invalid)
Expand Down Expand Up @@ -219,13 +223,13 @@ TEST(UTF8_Compare_Core_utf8_invalid)

// the same applies if the strings are interned.
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);
parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);
auto id1 = interner.intern(invalid1);
auto id2 = interner.intern(invalid2);
bool ret_interned = interner.compare(id1, id2) < 0;
CHECK_EQUAL(ret_interned, ret);
parent.destroy_deep();
}

TEST(Compare_Core_utf8_invalid_crash)
Expand All @@ -252,28 +256,14 @@ TEST(Compare_Core_utf8_invalid_crash)
TEST(UTF8_Compare_Core_utf8_zero)
{
Array parent(Allocator::get_default());
_impl::DeepArrayDestroyGuard dg(&parent);
parent.create(NodeHeader::type_HasRefs, false, 1, 0);
StringInterner interner(Allocator::get_default(), parent, ColKey(0), true);

// intern all the utf8 strings
const char* empty = "\0";
const char* a = "a";
const char* aa = "a\0a";
const char* ab = "a\0b";
const char* a0 = "a\0";
const char* a00 = "a\0\0";
interner.intern(StringData(empty, 1));
interner.intern(StringData(a));
interner.intern(StringData(aa, 3));
interner.intern(StringData(ab, 3));
interner.intern(StringData(a0, 2));
interner.intern(StringData(a00, 3));

auto str_compare = [&interner, this](StringData a, StringData b) {
auto id1 = interner.lookup(a);
auto id2 = interner.lookup(b);
CHECK_EQUAL((id1 && id2), true);
const auto ret_interner_cmp = interner.compare(*id1, *id2) < 0;
auto id1 = interner.intern(a);
auto id2 = interner.intern(b);
const auto ret_interner_cmp = interner.compare(id1, id2) < 0;
const auto ret_cmp = a < b;
CHECK_EQUAL(ret_cmp, ret_interner_cmp);
return ret_cmp;
Expand All @@ -292,8 +282,6 @@ TEST(UTF8_Compare_Core_utf8_zero)
// Number of trailing 0 makes a difference
CHECK_EQUAL(true, str_compare(StringData("a\0", 2), StringData("a\0\0", 3)));
CHECK_EQUAL(false, str_compare(StringData("a\0\0", 3), StringData("a\0", 2)));

parent.destroy_deep();
}
} // anonymous namespace

Expand Down

0 comments on commit 1629181

Please sign in to comment.