chore(server): Extended SerializerBase test#7179
Conversation
🤖 Augment PR SummarySummary: This PR adds a more comprehensive unit test suite around Changes:
Technical Notes: The tests track “baseline-before-journal” ordering per key and validate invariants by counting baseline emissions vs journal mutations while injecting randomized yields to stress scheduling/interleavings. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| auto entry = q_.top(); | ||
| q_.pop(); | ||
|
|
||
| util::ThisFiber::SleepUntil(entry.first); |
There was a problem hiding this comment.
Because the entry is popped from q_ before SleepUntil(), calling Pause() while the fiber is sleeping won’t prevent the already-dequeued item from being resolved, which makes the pause/resume semantics unreliable for tests that depend on delaying resolution.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| Change([](TestDriver& d) { d.delay_driver_.Pause(); }); | ||
|
|
||
| // Let all values to be expire deleted | ||
| TEST_current_time_ms = TEST_current_time_ms + 100; |
There was a problem hiding this comment.
This modifies the global TEST_current_time_ms without restoring it; once the test is un-skipped, this can leak the simulated time shift into subsequent tests and cause hard-to-debug order-dependent failures.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| private: | ||
| friend struct TestDriver; |
There was a problem hiding this comment.
Making DelayedEntryHandler befriend a test-only type (TestDriver) in a widely included production header effectively grants private access to any TU that defines dfly::TestDriver, which weakens encapsulation beyond just the test target.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Pull request overview
Adds a new C++ test that exercises SerializerBase with a more “consumer-like” driver that immediately validates baseline/journal ordering invariants (similar to a replica/cluster consumer), and wires the test into the server test suite.
Changes:
- Introduces
serializer_base_test.ccwith aTestDriver(extendsSerializerBase+ journal consumer) and multiple concurrent test scenarios. - Adds
TestDriverfriendship toDelayedEntryHandlerso the test driver can inject/inspect delayed-entry state. - Registers the new test target in
src/server/CMakeLists.txt.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/server/serializer_base_test.cc | New extended SerializerBase tests using a custom driver, background delayed-resolution fiber, and journal parsing to assert invariants. |
| src/server/serializer_base.h | Adds forward declaration + friend struct TestDriver to allow the new test driver to access DelayedEntryHandler internals. |
| src/server/CMakeLists.txt | Adds serializer_base_test to the helio C++ test targets. |
| TEST_F(SerializerBaseTest, DelayedAllDeleted) { | ||
| GTEST_SKIP() << "To be fixed"; |
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
dranikpg
force-pushed
the
serializer-base-test
branch
from
f8d57a8 to
066c1c1
Compare
dranikpg
force-pushed
the
serializer-base-test
branch
from
066c1c1 to
cef349c
Compare
Introduce extended serializer base test with more intricate testing scenarions
The TestDriver extends SerializerBase and becomes one more "consumer" akin to a replica or cluster node. However instead of sending the data, it processes it immediately and verifies additional invariants