Skip to content

Commit 2a8806a

Browse files
authored
chore(server): Better hash tests (Part 1) (#5922)
1 parent 6399e7f commit 2a8806a

File tree

3 files changed

+84
-24
lines changed

3 files changed

+84
-24
lines changed

src/server/hset_family_test.cc

Lines changed: 82 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ extern "C" {
1717
using namespace testing;
1818
using namespace std;
1919
using namespace util;
20-
using namespace boost;
2120
using namespace facade;
2221

2322
namespace dfly {
@@ -58,13 +57,76 @@ TEST_F(HSetFamilyTest, Basic) {
5857
}
5958

6059
TEST_F(HSetFamilyTest, HSet) {
61-
string val(1024, 'b');
60+
// Simulate HSET on mirror map
61+
{
62+
absl::flat_hash_map<string, string> mirror; // mirror
63+
64+
// Generate HSET commands and check how many new entries were added
65+
absl::InsecureBitGen gen{};
66+
while (mirror.size() < 600) {
67+
vector<string> cmd = {"HSET", "hash"};
68+
size_t new_values = 0;
69+
for (int i = 0; i < 20; i++) {
70+
string key = GetRandomHex(gen, 3);
71+
string value = GetRandomHex(gen, 20, 10);
72+
new_values += mirror.contains(key) ? 0 : 1;
73+
mirror[key] = value;
74+
75+
cmd.emplace_back(key);
76+
cmd.emplace_back(value);
77+
}
78+
79+
EXPECT_THAT(Run(cmd), IntArg(new_values));
80+
}
81+
82+
// Verify consistency
83+
EXPECT_THAT(Run({"HLEN", "hash"}), IntArg(mirror.size()));
84+
for (const auto& [key, value] : mirror)
85+
EXPECT_EQ(Run({"HGET", "hash", key}), mirror[key]);
86+
}
87+
88+
// HSet with same key twice
89+
Run({"HSET", "hash", "key1", "value1", "key1", "value2"});
90+
EXPECT_EQ(Run({"HGET", "hash", "key1"}), "value2");
91+
92+
// Wrong value cases
93+
EXPECT_THAT(Run({"HSET", "key"}), ErrArg("wrong number of arguments"));
94+
EXPECT_THAT(Run({"HSET", "key", "key"}), ErrArg("wrong number of arguments"));
95+
EXPECT_THAT(Run({"HSET", "key", "key", "value", "key2"}), ErrArg("wrong number of arguments"));
96+
}
97+
98+
TEST_F(HSetFamilyTest, HSetNX) {
99+
// Should create new field
100+
EXPECT_THAT(Run({"HSETNX", "hash", "key1", "value1"}), IntArg(1));
101+
EXPECT_EQ(Run({"HGET", "hash", "key1"}), "value1");
102+
103+
// Should not overwrite
104+
EXPECT_THAT(Run({"HSETNX", "hash", "key1", "value2"}), IntArg(0));
105+
EXPECT_EQ(Run({"HGET", "hash", "key1"}), "value1");
106+
107+
// Wrong value cases
108+
EXPECT_THAT(Run({"HSETNX", "key"}), ErrArg("wrong number of arguments"));
109+
EXPECT_THAT(Run({"HSET", "key", "key"}), ErrArg("wrong number of arguments"));
110+
}
111+
112+
// Listpack handles integers separately, so create a mix of different types
113+
TEST_F(HSetFamilyTest, MixedTypes) {
114+
absl::flat_hash_set<string> str_keys, int_keys;
115+
for (int i = 0; i < 100; i++) {
116+
auto key1 = absl::StrCat("s", i);
117+
auto key2 = absl::StrCat("i", i);
118+
Run({"HSET", "hash", key1, "VALUE", key2, "123456"});
119+
str_keys.emplace(key1);
120+
int_keys.emplace(key2);
121+
}
62122

63-
EXPECT_EQ(1, CheckedInt({"hset", "large", "a", val}));
64-
EXPECT_EQ(1, CheckedInt({"hlen", "large"}));
65-
EXPECT_EQ(1024, CheckedInt({"hstrlen", "large", "a"}));
123+
for (string_view key : str_keys)
124+
EXPECT_EQ(Run({{"HGET", "hash", key}}), "VALUE");
66125

67-
EXPECT_EQ(1, CheckedInt({"hset", "small", "", "565323349817"}));
126+
for (string_view key : int_keys) {
127+
EXPECT_EQ(Run({{"HGET", "hash", key}}), "123456");
128+
EXPECT_EQ(CheckedInt({"hincrby", "hash", key, "1"}), 123456 + 1);
129+
}
68130
}
69131

70132
TEST_P(HestFamilyTestProtocolVersioned, Get) {
@@ -101,24 +163,22 @@ TEST_P(HestFamilyTestProtocolVersioned, Get) {
101163
EXPECT_THAT(resp.GetVec(), ElementsAre("a", "1", "b", "2", "c", "3"));
102164
}
103165

104-
TEST_F(HSetFamilyTest, HSetNx) {
105-
EXPECT_EQ(1, CheckedInt({"hsetnx", "key", "field", "val"}));
106-
EXPECT_EQ(Run({"hget", "key", "field"}), "val");
107-
108-
EXPECT_EQ(0, CheckedInt({"hsetnx", "key", "field", "val2"}));
109-
EXPECT_EQ(Run({"hget", "key", "field"}), "val");
110-
111-
EXPECT_EQ(1, CheckedInt({"hsetnx", "key", "field2", "val2"}));
112-
EXPECT_EQ(Run({"hget", "key", "field2"}), "val2");
113-
114-
// check dict path
115-
EXPECT_EQ(0, CheckedInt({"hsetnx", "key", "field2", string(512, 'a')}));
116-
EXPECT_EQ(Run({"hget", "key", "field2"}), "val2");
117-
}
166+
TEST_F(HSetFamilyTest, HIncrBy) {
167+
int total = 10;
168+
// Check new field is created
169+
EXPECT_EQ(CheckedInt({"hincrby", "key", "field", "10"}), 10);
170+
EXPECT_EQ(Run({"hget", "key", "field"}), "10");
171+
// Simulate multiple additions
172+
for (int i = -100; i < 100; i += 7) {
173+
total += i;
174+
EXPECT_EQ(CheckedInt({"hincrby", "key", "field", to_string(i)}), total);
175+
}
118176

119-
TEST_F(HSetFamilyTest, HIncr) {
120-
EXPECT_EQ(10, CheckedInt({"hincrby", "key", "field", "10"}));
177+
// Overflow
178+
Run({"hset", "key", "field2", to_string(numeric_limits<int64_t>::max() - 1)});
179+
EXPECT_THAT(Run({"hincrby", "key", "field2", "2"}), ErrArg("would overflow"));
121180

181+
// Error case
122182
Run({"hset", "key", "a", " 1"});
123183
auto resp = Run({"hincrby", "key", "a", "10"});
124184
EXPECT_THAT(resp, ErrArg("hash value is not an integer"));

src/server/test_utils.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ RespExpr BaseFamilyTest::RunPrivileged(std::initializer_list<const std::string_v
429429
return res;
430430
}
431431

432-
RespExpr BaseFamilyTest::Run(absl::Span<std::string> span) {
432+
RespExpr BaseFamilyTest::Run(absl::Span<const std::string> span) {
433433
vector<string_view> sv_vec(span.size());
434434
for (unsigned i = 0; i < span.size(); ++i) {
435435
sv_vec[i] = span[i];

src/server/test_utils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ class BaseFamilyTest : public ::testing::Test {
8181
RespExpr RunPrivileged(std::initializer_list<const std::string_view> list);
8282

8383
RespExpr Run(ArgSlice list);
84-
RespExpr Run(absl::Span<std::string> list);
84+
RespExpr Run(absl::Span<const std::string> list);
8585

8686
RespExpr Run(std::string_view id, ArgSlice list);
8787

0 commit comments

Comments
 (0)