Conversation
- Migrate all 38 follower_state tests from d-engine-server to d-engine-core - Use mock_raft_context_with_temp helper for consistent setup - Improve test naming following Rust best practices - Add comprehensive comments for each test scenario - Preserve all original test intent (100% coverage) - Execution time: 0.06s (10x faster than integration setup) Tests migrated: - Initialization & Tick (3) - VoteRequest handling (3) - ClusterConf & ClusterConfUpdate (7) - AppendEntries (3) - Client requests (6) - RaftLogCleanUp (7) - Cluster management (2) - Log purge safety (4) - Role violation (3) All tests passing ✅
Split monolithic leader_state_test.rs (~5.2k lines) into focused modules for better maintainability and test organization. Migration results: - 95 tests migrated from d-engine-server to d-engine-core - 124 tests passing, 9 tests marked #[ignore] (require integration test environment) - Test modules created: * client_write_test.rs (3 tests) * event_handling_test.rs (13 tests) * snapshot_test.rs (10 tests) * replication_test.rs (17 tests) * state_management_test.rs (10 tests) * membership_change_test.rs (42 tests) Changes: - Improved test names (case1_1 → descriptive names like reject_same_term) - Inline test helpers per module (no shared test_utils needed) - Preserved all test logic and assertions - 9 async tests marked #[ignore] pending integration test infrastructure Tests: cargo nextest run --all-features leader_state_test
…with modular structure - Migrated all 62 replication handler tests (31 implemented + 31 TODO) - Reorganized into 8 domain-specific modules for maintainability - Created mock helpers in core to eliminate server dependencies - Renamed tests for clarity with Arrange-Act-Assert structure - Deleted monolithic full_test.rs (4,200 lines) - All tests maintain original assertion logic and purposes - Verification: 31 passed, 31 ignored (TODO), 0 failed
…straction - Moved BufferedRaftLog<T> to d-engine-core (generic, no concrete dependencies) - Updated imports from d_engine_core:: to crate:: within core - Added re-export in server: pub use d_engine_core::BufferedRaftLog - Changed BufferedRaftLog::new from pub(crate) to pub for external access - Tests remain in server (use FileStorageEngine for integration testing) - Optimized test timeouts: 2s→200ms, 3s→500ms, 5s→500ms (33s test now <1s) Architecture improvement: - BufferedRaftLog is truly generic (T: TypeConfig) with zero server dependencies - Can be reused with any storage backend (File, Memory, S3, etc.) - Follows proper layering: core=abstractions, server=implementations
…ne-core Align benchmark location with tested code following Rust best practices. LeaderState is a core module, its performance tests should live in d-engine-core/benches/ rather than d-engine-server/benches/. Changes: - Move leader_state_bench.rs to d-engine-core/benches/ (git mv preserves history) - Add criterion to d-engine-core dev-dependencies - Add [[bench]] config to d-engine-core/Cargo.toml with required-features = ["test-utils"] - Remove leader_state_bench config from d-engine-server/Cargo.toml
Migrate BufferedRaftLog unit tests to core following Rust best practices. This is Phase 0-2 of 10-phase migration plan. Changes: - Add test-utils APIs to BufferedRaftLog: * durable_index() - accessor for test verification * is_all_workers_finished() - worker state checking - Create test infrastructure: * BufferedRaftLogTestContext for test context management * Mock entry generators and command simulators * Crash recovery simulation support - Migrate 35 unit tests (27% complete): * basic_operations_test.rs - 25 tests (CRUD, range, conflicts, purge) * flush_strategy_test.rs - 10 tests (DiskFirst/MemFirst/Batched) - Create modular test structure: * 9 test modules with clear documentation * Self-documenting test names Test results: 35/35 passing Next: Phases 3-10 (95 unit tests + 17 integration tests remaining)
Continue migration of BufferedRaftLog unit tests to core. This is Phase 3 of 10-phase migration plan. Changes: - Add test-utils API: * next_id() accessor for ID allocation verification - Migrate ID allocation tests (9 tests): * Sequential allocation verification * Batch range allocation * Zero-count edge cases * Concurrent allocation safety * Mixed allocation strategies Test results: 44/44 passing (34% complete) - basic_operations_test: 25 tests - flush_strategy_test: 10 tests - id_allocation_test: 9 tests Next: Phases 4-10 (86 unit tests + 17 integration tests remaining)
Continue migration of BufferedRaftLog unit tests to core. This is Phase 4 of 10-phase migration plan. Changes: - Migrate term index tests (5 tests): * first_index_for_term verification * last_index_for_term verification * Term index after purge * Term index with concurrent writes * Performance with large dataset Test results: 49/49 passing (38% complete) - basic_operations_test: 25 tests - flush_strategy_test: 10 tests - id_allocation_test: 9 tests - term_index_test: 5 tests Next: Phases 5-10 (81 unit tests + 17 integration tests remaining)
- Migrate 2 concurrent operation tests from server to core - test_remove_range_with_concurrent_reads: verify safety during concurrent reads/removes - test_concurrent_append_and_purge: verify consistency under mixed concurrent operations - All tests use mock storage (MockStorageEngine) for unit testing - Total migrated: 51/100 tests
- Migrate 6 remove_range operation tests from server to core - test_remove_middle_range: verify middle range removal correctness - test_remove_from_start/test_remove_to_end: verify boundary updates - test_remove_empty_range/test_remove_entire_log: verify edge cases - test_remove_single_entry: verify single entry removal - All tests use mock storage for algorithm verification - Total migrated: 57/100 tests
- Migrate 4 edge case tests from server to core - test_empty_log_operations: verify empty log boundary behavior - test_single_entry_operations: verify single entry lifecycle - test_gap_handling_in_indexes: verify non-contiguous index handling after removals - test_extreme_boundary_conditions: verify u64::MAX boundary correctness - All tests use mock storage for algorithm verification - Total migrated: 61/100 tests
- Migrate 2 Raft protocol property tests from server to core - test_log_matching_property: verify Raft Log Matching Property - test_leader_completeness_property: verify Leader Completeness Property - Add append_entries helper to BufferedRaftLogTestContext for test convenience - All tests use mock storage for algorithm verification - Total migrated: 63/100 tests
- Migrate 6 calculate_majority_matched_index tests from server to core - Test all scenarios for Raft commit index calculation algorithm - case0-4: verify correctness under various term/index combinations - case5: stress test with 100K entries to verify performance - All tests use mock storage for algorithm verification - Total migrated: 69/100 tests
- Migrate 4 shutdown behavior tests from server to core - test_shutdown_closes_channel_properly: verify channel closure on drop - test_shutdown_awaits_worker_completion: verify graceful worker termination - test_shutdown_handles_slow_workers: verify timeout handling - test_shutdown_with_multiple_flushes: verify cleanup with pending work - All tests use mock storage for algorithm verification - Total migrated: 73/100 tests
…rker) - Migrate 3 durable_index and worker tests from server to core - test_durable_index_monotonic_under_concurrency: verify durable_index monotonicity - test_durable_index_with_non_contiguous_entries: verify non-contiguous handling - test_worker_retry_survives_transient_errors: verify worker resilience - All tests use mock storage for algorithm verification - Total migrated: 76/100 tests
…istory) - Move buffered_raft_log_test.rs to tests/integration/storage_buffered_raft_log/ - Create module structure: crash_recovery, performance, stress, storage_integration - Extract shared TestContext to mod.rs - Remove empty cfg(test) block from server/src - Integration tests now organized by test category - Git history preserved via 'git mv'
…n and failures **Compilation fixes:** - Replace MockStateMachine with FileStateMachine in all integration tests - Use public test-utils methods (len(), durable_index()) instead of private fields - Add missing imports (FileStateMachine, RaftLog trait, etc.) - Fix type mismatches (usize → u64) **Test migration:** - Move 3 mock-based performance tests to core unit tests: * test_reset_performance_during_active_flush * test_filter_conflicts_performance_during_flush * test_fresh_cluster_performance_consistency - These require controllable delays and belong in core unit tests **Test fixes:** - Split test_partial_flush_after_crash into two tests: * test_partial_flush_with_graceful_shutdown: Verifies Drop flushes all data (75 entries) * test_partial_flush_after_crash: Uses mem::forget to simulate crash (50 entries) - Fix test_high_concurrency_mixed_operations expected count (10000) - Properly simulate crash by adding remaining entries after first batch flush **Test results:** - Server integration tests: 18/18 passed - Core performance tests: 3/3 passed - Integration test count: 18 (was 21, 3 moved to core) **Root cause analysis:** - FileLogStore Drop implementation correctly flushes on graceful shutdown - mem::forget properly simulates crash (kill -9/power loss) by skipping Drop
**Test extraction:** - Extract test_last_entry_id_performance from legacy_all_tests.rs - Extract test_performance_benchmarks from legacy_all_tests.rs - Extract test_read_performance_remains_lockfree from legacy_all_tests.rs - Mark heavy performance tests with #[ignore] for explicit execution **Compilation fix:** - Remove useless comparison durable >= 0 (u64 is always >= 0) - Fix clippy::absurd_extreme_comparisons error **Verification results:** - Core unit tests: 79 tests - Server integration tests: 21 tests (18 + 3 performance) - Total: 100/100 tests ✅ - All 18 non-ignored integration tests pass - 3 performance tests marked #[ignore] for manual execution
**Test utilities reorganization:** - Move MockNode and MockRpcService from d-engine-core to d-engine-server - Create server test_utils with mock modules: * mock_rpc.rs - Mock RPC implementations * mock_rpc_service.rs - Mock service handlers * snapshot.rs - Snapshot utilities * stream.rs - Stream utilities - Update all test imports to use server test_utils **Test fixes:** - Fix CI environment check in linearizable_read_batching_embedded * Change is_err() to is_ok() to skip benchmark in CI * Performance tests should skip in CI (unstable machine performance) - Delete legacy_all_tests.rs (3907 lines removed) * All tests extracted to proper modules **Files changed:** - src/membership/raft_membership_test.rs: Use server test_utils - src/network/*_test.rs: Use server test_utils - tests/consistent_reads/linearizable_read_batching_embedded.rs: Fix CI check - tests/storage_buffered_raft_log/: Remove legacy file **Rationale:** - Server tests should not depend on core test_utils - Each crate maintains its own test utilities - Follows separation of concerns principle **Test results:** - All server tests compile and pass - Performance test correctly skips in CI environment
…ntation visibility
📝 WalkthroughWalkthroughThis PR refactors the test infrastructure by renaming the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors the test-utils feature to improve test infrastructure and reduce public API surface area.
Changes:
- Removed
test-utilsfeature flag in favor of standard#[cfg(test)]compilation - Introduced
__test_supportfeature for specialized test exports (e.g., dev-dependencies) - Reduced visibility of test helper methods from
pubtopub(crate) - Reorganized test module structure with comprehensive BufferedRaftLog and ReplicationHandler test suites
Reviewed changes
Copilot reviewed 74 out of 114 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| d-engine-server/Cargo.toml | Removed test-utils feature, moved test dependencies to dev-dependencies |
| d-engine-server/src/lib.rs | Changed test_utils visibility from pub to pub(crate) |
| d-engine-core/Cargo.toml | Renamed test-utils feature to __test_support for dev-dependencies |
| d-engine-core/src/storage/buffered_raft_log.rs | Changed BufferedRaftLog::new visibility from pub(crate) to pub, added test helper methods |
| d-engine-core/src/storage/buffered_raft_log_test/* | Added comprehensive test suites for BufferedRaftLog functionality |
| d-engine-core/src/replication/replication_handler_test/* | Added comprehensive test suites for ReplicationHandler functionality |
| d-engine-core/src/test_utils/* | Added test helper modules for BufferedRaftLog and ReplicationHandler testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…e Makefile docs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
d-engine-core/src/raft_role/learner_state_test.rs (1)
28-31: Usemock_raft_context_with_tempfor portability and consistency.These two tests use hardcoded
/tmp/paths, which are not portable to Windows and inconsistent with all other tests in this file (starting at line 109). Replacemock_raft_contextwithmock_raft_context_with_tempand use the returnedTempDir.♻️ Proposed fix for test_learner_rejects_flush_read_buffer_event
#[tokio::test] async fn test_learner_rejects_flush_read_buffer_event() { let (shutdown_tx, shutdown_rx) = watch::channel(()); - let ctx = mock_raft_context("/tmp/test_learner_flush", shutdown_rx, None); + let (ctx, _temp_dir) = mock_raft_context_with_temp(shutdown_rx, None); let mut state = LearnerState::<MockTypeConfig>::new(1, ctx.node_config.clone());♻️ Proposed fix for test_learner_drain_read_buffer_returns_error
#[tokio::test] async fn test_learner_drain_read_buffer_returns_error() { - let mut state = - LearnerState::<MockTypeConfig>::new(1, Arc::new(node_config("/tmp/test_learner_drain"))); + let (shutdown_tx, shutdown_rx) = watch::channel(()); + let (ctx, _temp_dir) = mock_raft_context_with_temp(shutdown_rx, None); + let mut state = LearnerState::<MockTypeConfig>::new(1, ctx.node_config.clone());Also applies to: 68-70
d-engine-core/src/maybe_clone_oneshot.rs (1)
181-201: Dead code: cfg attributes inside excluded block will never compile.Lines 191-197 contain
#[cfg(any(test, feature = "__test_support"))]attributes, but they're inside a block that's only compiled whennot(any(test, feature = "__test_support")). These cfg attributes and their associatedtest_inner: Nonefields will never be included in any build.This appears to be leftover from a refactor. The
test_innerfield initialization is unreachable in this context.🔧 Suggested fix
#[cfg(not(any(test, feature = "__test_support")))] impl<T: Send> RaftOneshot<T> for MaybeCloneOneshot { type Sender = MaybeCloneOneshotSender<T>; type Receiver = MaybeCloneOneshotReceiver<T>; fn new() -> (Self::Sender, Self::Receiver) { let (tx, rx) = oneshot::channel(); ( MaybeCloneOneshotSender { inner: tx, - #[cfg(any(test, feature = "__test_support"))] - test_inner: None, }, MaybeCloneOneshotReceiver { inner: rx, - #[cfg(any(test, feature = "__test_support"))] - test_inner: None, }, ) } }d-engine-server/src/storage/adaptors/file/file_storage_engine.rs (1)
241-260: Remove#[allow(dead_code)]annotation—function is actively used in tests.The
reset_syncfunction is called in integration tests (d-engine-server/src/test_utils/integration/mod.rs:176), so the#[allow(dead_code)]attribute is incorrect and misleading. Remove it.The scope change from
#[cfg(any(test, feature = "test-utils"))]to#[cfg(test)]is acceptable since the function is used internally within the same crate's test suite and no longer requires feature-gating.d-engine-core/src/raft_role/leader_state.rs (1)
2172-2218: Benchmark will fail to compile without enabling__test_supportfeature.The
LeaderState::newcall in./d-engine-core/benches/leader_state_bench.rs:273is now gated behind#[cfg(any(test, feature = "__test_support"))]. Since benchmarks are compiled separately from tests (withoutcfg(test)active), runningcargo benchwill fail with a missing function error unless the user explicitly runscargo bench --features __test_support. This is a breaking change for the benchmark suite.
🤖 Fix all issues with AI agents
In `@d-engine-core/src/raft_role/candidate_state_test.rs`:
- Around line 576-623: The test
test_handle_append_entries_steps_down_to_follower can hang because the
reprocessed RoleEvent may keep resp_tx alive; replace the final direct await on
resp_rx with a timeout-based assertion to avoid deadlock: use
tokio::time::timeout (e.g., Duration::from_millis(100)) around resp_rx.recv()
and assert that the timeout completes with an Err (meaning channel closed) or
that the timed recv returns Err; update the assertion at the end (currently
resp_rx.recv().await.is_err()) to use tokio::time::timeout and assert
timeout/Err accordingly so the test fails fast instead of hanging.
In `@d-engine-core/src/replication/replication_handler_test/mod.rs`:
- Around line 17-18: The crate-level/doc comment references a test module named
`batch_handling_test` but no `mod batch_handling_test;` is declared; either
remove the `batch_handling_test` reference from the documentation or add a
module declaration `mod batch_handling_test;` in this file and ensure the
corresponding `batch_handling_test` module file exists (or is annotated with
`#[cfg(test)]` if it’s a test-only module) so the documentation and module list
remain consistent.
In `@d-engine-core/src/storage/buffered_raft_log_test/basic_operations_test.rs`:
- Around line 248-271: The test name is misleading because
simulate_insert_command uses pre_allocate_raft_logs_next_index to assign
sequential indexes (so the payload value max-1 is not the entry index); either
rename the test to reflect it asserts index 1 (e.g.,
test_last_entry_with_payload_u64_max_minus_one) or change the test to actually
create an entry with a high index by directly inserting/appending an entry with
index u64::MAX-1 (use the raft log API that accepts an explicit index or set the
next index before insert), then assert last.index equals u64::MAX-1; update the
test name and assertions accordingly and keep simulate_insert_command references
only if payload-based behavior is intended.
In `@d-engine-core/src/storage/buffered_raft_log_test/flush_strategy_test.rs`:
- Around line 44-48: Update the doc comment for the test titled "Test DiskFirst
concurrent writes remain consistent" to use Rust/Tokio terminology: replace the
word "goroutines" with "tasks" (or "Tokio tasks") so the scenario reads e.g. "10
concurrent tasks each append 100 entries"; keep the rest of the comment
unchanged.
In `@d-engine-core/src/storage/buffered_raft_log_test/helper_functions_test.rs`:
- Line 1: The placeholder test file should be removed or completed: either
delete the empty buffered_raft_log_test/helper_functions_test.rs placeholder and
open a new tracking issue for the pending migration, or finish migrating the
helper functions into that test file now; if you choose to defer, replace the
placeholder with a TODO comment referencing the new issue number and a brief
rationale so the repo has a clear record of why migration is pending.
In `@d-engine-core/src/storage/buffered_raft_log_test/id_allocation_test.rs`:
- Around line 218-229: Add a test that verifies the code panics on ID overflow:
update or add a test (e.g., a new #[tokio::test] with #[should_panic(expected =
"ID overflow")] or use std::panic::catch_unwind) that sets raft_log.next_id() to
a value such that calling raft_log.pre_allocate_id_range(...) would require IDs
> u64::MAX and assert the panic; reference the existing test_edge_cases setup
and the methods pre_allocate_id_range and next_id to locate where to insert this
assertion.
In `@d-engine-core/src/test_utils/mod.rs`:
- Around line 3-7: The test helper modules buffered_raft_log_test_helpers,
replication_test_helpers and the snapshot module are not feature-gated and will
be compiled in production; add the same gate used elsewhere (e.g.
#[cfg(any(test, feature = "__test_support"))]) above the mod declarations for
buffered_raft_log_test_helpers, replication_test_helpers, and snapshot so they
are only compiled for tests or when the __test_support feature is enabled,
keeping other modules (like pub mod mock) unchanged.
🧹 Nitpick comments (44)
d-engine-core/src/storage/buffered_raft_log_test/crash_recovery_test.rs (1)
1-1: Consider tracking pending migration work through issues rather than placeholder files.Placeholder files with no executable code can be overlooked and may cause confusion about test coverage. Since Issue
#245already tracks the migration effort, you could either:
- Remove this file and track crash recovery test migration in the issue checklist
- Add a TODO comment in the parent module instead
- Implement the crash recovery tests now
Would you like me to help draft crash recovery test cases based on common patterns for this type of testing, or open a new issue to track this specific test migration?
d-engine-core/src/replication/replication_handler_test/command_handling_test.rs (1)
7-47: Implement placeholder tests or track in an issue.All six test functions are currently placeholders with no implementation. While the test structure and naming are well-designed, the missing implementations leave gaps in test coverage for critical command handling and batching scenarios.
Would you like me to:
- Generate implementations for these test cases based on the documented scenarios?
- Open a tracking issue to ensure these tests are implemented in a follow-up PR?
The test scenarios to implement are:
- Single command replication
- Multiple commands replication
- Large batch splitting
- Empty commands (single-node and multi-node)
- Heartbeat with peer success
d-engine-server/src/network/grpc/grpc_transport_test.rs (2)
40-43: Imports forcreate_test_chunkandcreate_test_snapshot_streamappear to be unused in active code.These imports are only used by
create_failing_stream(lines 1087-1116), which is marked#[allow(unused)]. If the function is intentionally kept for future use, consider adding#[allow(unused_imports)]to these imports as well for consistency. Otherwise, consider removing both the unused function and these imports.
1087-1116: Consider removing the unusedcreate_failing_streamhelper function or documenting its intended use.This function is marked
#[allow(unused)]and is not called anywhere in the test file. The importscreate_test_chunkandcreate_test_snapshot_streamare used within this function, but since the function itself is unused, these represent indirect dead code. If this is intentionally preserved for future snapshot streaming failure test scenarios, add a comment clarifying when it will be used. Otherwise, remove the function along with any imports that become truly unused.d-engine-core/src/storage/buffered_raft_log_test/flush_strategy_test.rs (1)
182-187: Consider strengthening the assertion.The assertion
durable_index() > 0is weak for a test that appends 5 entries and explicitly flushes. After a successful flush,durable_index()should be5. The current assertion would pass even if only 1 entry was flushed.If the intent is to avoid flakiness due to timing, consider documenting this, or use a more precise assertion:
💡 Suggested improvement
// Assert: Entries now durable assert!( - ctx.raft_log.durable_index() > 0, - "Entries should be durable after flush" + ctx.raft_log.durable_index() == 5, + "All 5 entries should be durable after explicit flush" );d-engine-core/src/replication/replication_handler_test/index_tracking_test.rs (1)
1-48: Placeholder tests acknowledged.Six test stubs are properly scaffolded with
#[ignore = "TODO: Implement test"]attributes. The module documentation provides good categorization (E1, E2, E3) for the intended test coverage.Consider tracking these TODOs in an issue to ensure they get implemented. With 6 ignored tests here, it would be helpful to have visibility on when they'll be completed.
Would you like me to open an issue to track the implementation of these index tracking tests?
d-engine-core/src/maybe_clone_oneshot.rs (1)
107-138: Test-only busy-polling Future implementation.The
Futureimplementation for test mode usescx.waker().wake_by_ref()followed byPoll::Pendingwhen the broadcast channel is empty (line 123-124). This creates a spin loop that will consume CPU while waiting.This is acceptable for test-only code, but if tests become slow or CPU-bound, consider using
tokio::sync::broadcast::Receiver::recv()with proper async handling instead oftry_recv()polling.d-engine-core/src/event.rs (2)
140-141: Redundant#[cfg_attr(test, ...)]inside#[cfg(test)]block.Since the enum is already gated by
#[cfg(test)]on line 140, the conditional derive on line 141 is redundant—it will only be compiled in test mode anyway. Simplify to:#[cfg(test)] -#[cfg_attr(test, derive(Debug, Clone))] +#[derive(Debug, Clone)] #[allow(unused)] pub enum TestEvent {
201-208: Placeholder mappings may cause misleading test behavior.Returning
TestEvent::CreateSnapshotEventas a placeholder forStepDownSelfRemovedandMembershipAppliedcould cause confusion if tests inadvertently rely on these mappings. Consider either:
- Adding dedicated
TestEventvariants for these events, or- Using a clearly-named placeholder like
TestEvent::IgnoredorTestEvent::InternalEventThe current comments explain the intent, but the mapping itself could lead to false positive test assertions.
d-engine-core/src/replication/replication_handler_test/log_retrieval_test.rs (1)
40-42:Arc::get_mutpattern is fragile.Using
Arc::get_mutworks here because theArchas a single reference immediately after creation. However, this pattern is fragile—ifsetup_mock_replication_test_contextimplementation changes to clone theArcinternally, these tests will panic.Consider having the setup function return the mock directly (not wrapped in
Arc) and letting tests wrap it after configuration, or providing a builder pattern for mock setup.d-engine-core/benches/leader_state_bench.rs (1)
160-161: Consider extracting the timeout configuration.The hardcoded
100mstimeout forverify_leadership_persistent_timeoutcould be extracted to a constant for clarity and easier adjustment during benchmark tuning.Suggested improvement
+const BENCH_LEADERSHIP_VERIFY_TIMEOUT: Duration = Duration::from_millis(100); + impl BenchFixture { async fn new(test_name: &str) -> Self { // ... node_config.raft.membership.verify_leadership_persistent_timeout = - Duration::from_millis(100); + BENCH_LEADERSHIP_VERIFY_TIMEOUT;d-engine-core/src/test_utils/replication_test_helpers.rs (1)
77-110: Consider adding bounds validation to prevent silent failures.The
mock_log_entries_existfunction handles empty entries gracefully withunwrap_or(0), but if an empty vector is passed, the mockedget_entries_rangeandentrymethods will always return empty results without any indication. This may lead to silent test failures if callers mistakenly pass empty entries.Also, the three clones (
entries_clone1,entries_clone2,entries_clone3) are necessary due to Rust's move semantics with closures, but a brief inline comment explaining this would improve readability.Suggested improvement
pub fn mock_log_entries_exist( raft_log: &mut MockRaftLog, entries: Vec<Entry>, ) { + // Note: Multiple clones are needed because each closure moves its captured value let entries_clone1 = entries.clone(); let entries_clone2 = entries.clone(); let entries_clone3 = entries.clone(); let last_index = entries.last().map(|e| e.index).unwrap_or(0);d-engine-core/src/election/election_handler_test.rs (2)
822-928: Excellent documentation of false positive test.The detailed FIXME comment thoroughly explains why this test is a false positive, documents the original behavior, intended behavior, and required fix. Properly marking it with
#[ignore]prevents silent CI failures while preserving the test for future correction.Consider creating a tracking issue for these false positive tests so they don't get forgotten.
Would you like me to help create a GitHub issue to track the fix for these three false positive tests (
test_broadcast_vote_requests_majority_reject_due_to_log_conflict,test_broadcast_vote_requests_peer_has_higher_term,test_broadcast_vote_requests_peer_has_higher_log_index)?
1319-1372: Consider simplifying test setup as noted in TODO.The TODO comment correctly identifies that
check_vote_request_is_legalis a pure function that doesn't need file system setup. ThetempdirandRaftNodeConfigcreation are unnecessary overhead. This applies to multiple tests in this section.Since this is a test refactoring PR, consider addressing these simplifications now or creating a follow-up task.
Simplified test setup example
#[tokio::test] async fn test_check_vote_request_is_legal_rejects_when_current_term_not_lower() { - // TODO: Simplify to just `let election_handler = ElectionHandler::<MockTypeConfig>::new(1);` - let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); - let mut node_config = RaftNodeConfig::new().expect("Should create default config"); - node_config.cluster.db_root_dir = temp_dir.path().to_path_buf(); - let _node_config = node_config.validate().expect("Should validate config"); let election_handler = ElectionHandler::<MockTypeConfig>::new(1);d-engine-core/src/raft_role/candidate_state_test.rs (1)
45-83: Prefer tempdir-backed contexts over hard-coded/tmp/...in async tests.This reduces flakiness under parallel test runs and avoids relying on host FS layout. You already use
tempfilein this file; consider reusing a tempdir formock_raft_contextinputs as well.Also applies to: 190-245
d-engine-core/src/raft_role/follower_state_test.rs (1)
73-112: Consider switching the remaining/tmp/...context usages to tempdir for consistency.Most of the file already uses
mock_raft_context_with_temp; aligning these two tests reduces environment coupling.Also applies to: 113-143
d-engine-core/src/replication/replication_handler_test/edge_cases_test.rs (1)
1-39: Placeholder tests noted - consider tracking via issues.The ignored placeholder tests are a valid way to document planned test scenarios. However, you may want to consider creating GitHub issues for these instead, as it provides better visibility for tracking progress and assigning work.
Current placeholders:
test_network_partitiontest_all_peers_timeouttest_extreme_large_batchtest_log_generation_failuretest_term_change_during_replicationWould you like me to help create GitHub issues for these test cases, or generate initial implementations for any of them?
d-engine-core/src/storage/buffered_raft_log_test/term_index_test.rs (1)
249-282: Timing assertion may be flaky in CI environments.The
assert!(elapsed.as_millis() < 100, ...)check could fail intermittently on slow or overloaded CI runners. Consider:
- Marking this test with
#[ignore]for manual/perf runs only- Increasing the threshold with margin (e.g., 500ms)
- Moving to a proper benchmark using
criterionOption: Mark as ignored performance test
#[tokio::test] +#[ignore = "Performance test - run manually"] async fn test_term_index_performance_large_dataset() {d-engine-core/src/storage/buffered_raft_log_test/remove_range_test.rs (1)
1-36: Good test coverage for remove_range operations.The tests comprehensively cover important scenarios: middle range, boundaries, empty range, entire log, and single entry removal.
Consider extracting the common context setup into a helper to reduce duplication:
♻️ Optional: Extract common test setup
+fn setup_test_context(name: &str) -> BufferedRaftLogTestContext { + BufferedRaftLogTestContext::new( + PersistenceStrategy::MemFirst, + FlushPolicy::Batch { + threshold: 1, + interval_ms: 1, + }, + name, + ) +} + #[tokio::test] async fn test_remove_middle_range() { - let ctx = BufferedRaftLogTestContext::new( - PersistenceStrategy::MemFirst, - FlushPolicy::Batch { - threshold: 1, - interval_ms: 1, - }, - "test_remove_middle_range", - ); + let ctx = setup_test_context("test_remove_middle_range");d-engine-core/src/test_utils/mod.rs (1)
16-20: Re-exports should match module gating.If the modules are gated behind test configuration, the corresponding
pub usestatements should also be gated to avoid compilation errors in non-test builds.♻️ Conditional re-exports
+#[cfg(any(test, feature = "__test_support"))] pub use buffered_raft_log_test_helpers::*; pub use common::*; pub use entry_builder::*; pub use mock::*; +#[cfg(any(test, feature = "__test_support"))] pub use replication_test_helpers::*; pub use snapshot::*;d-engine-core/src/storage/buffered_raft_log_test/id_allocation_test.rs (1)
34-36: Consider using a more idiomatic empty range check.The sentinel value
MAX..=MAXfor empty ranges is implementation-specific. If the underlying implementation changes, this helper would silently break.♻️ Alternative using RangeInclusive::is_empty()
fn is_empty_range(range: &RangeInclusive<u64>) -> bool { - range.start() == &u64::MAX && range.end() == &u64::MAX + range.is_empty() || (range.start() == &u64::MAX && range.end() == &u64::MAX) }Note:
RangeInclusive::is_empty()returns true whenstart > end, which may differ from theMAX..=MAXsentinel convention used here.d-engine-core/src/replication/replication_handler_test/quorum_calculation_test.rs (2)
46-148: Well-structured test with clear documentation.The test effectively validates quorum achievement in a two-node cluster. The arrange-act-assert pattern is clear and the assertions are meaningful.
Consider extracting the duplicated
NodeMetacreation (appears at lines 75-80, 83-88, and 121-126):♻️ Extract NodeMeta helper
+fn create_peer_node_meta(peer_id: u32) -> NodeMeta { + NodeMeta { + id: peer_id, + address: format!("http://127.0.0.1:{}", 55000 + peer_id), + role: Follower.into(), + status: NodeStatus::Active.into(), + } +} // Then use: - membership.expect_replication_peers().returning(move || { - vec![NodeMeta { - id: peer2_id, - address: "http://127.0.0.1:55001".to_string(), - role: Follower.into(), - status: NodeStatus::Active.into(), - }] - }); + let peer_meta = create_peer_node_meta(peer2_id); + membership.expect_replication_peers().returning(move || vec![peer_meta.clone()]);
150-223: TODO tests provide good scaffolding.The ignored tests outline a comprehensive test matrix covering:
- B1: Quorum achieved scenarios (3-node, 5-node)
- B2: Quorum not achieved scenarios
- B3: Special response handling (higher term, stale term, RPC failures)
This is a good pattern for tracking planned test coverage.
Would you like me to help implement any of these TODO tests or open issues to track them?
d-engine-core/src/storage/buffered_raft_log_test/shutdown_test.rs (1)
44-83: Timing-based shutdown tests may be flaky in CI.The assertions with fixed duration thresholds (e.g.,
< Duration::from_millis(500)) can fail under CI load. Consider either:
- Using more generous timeouts (e.g., 2-5 seconds)
- Adding
#[ignore]with a note for manual/dedicated runs- Using a retry mechanism
♻️ Use more generous timeout for CI stability
assert!( - shutdown_duration < Duration::from_millis(500), + shutdown_duration < Duration::from_secs(2), "Shutdown took too long: {shutdown_duration:?}", );d-engine-core/src/storage/buffered_raft_log_test/durable_index_test.rs (2)
36-45: Consider using deterministic synchronization instead of sleep.The 200ms sleep may not be sufficient on slower CI systems, potentially causing flaky tests. Consider using a polling mechanism with a timeout or exposing a flush completion signal from the BufferedRaftLog for test purposes.
Additionally, the assertion
durable >= 1000assumes all entries are flushed, but the test spawns concurrent appends without ordering guarantees. If some appends fail or are delayed, this could intermittently fail.
85-94: Test doesn't verify durable_index behavior for non-contiguous entries.The variable
_durableis fetched but never used in assertions. The test verifies entries exist but doesn't actually test howdurable_indexhandles non-contiguous entries. Consider adding meaningful assertions about the expected durable_index value, or rename the test to reflect what it actually verifies.💡 Suggested improvement
// Verify durable_index handles non-contiguous entries correctly // With mock storage, durable_index behavior depends on flush completion - let _durable = ctx.raft_log.durable_index(); - // Note: durable_index returns u64, which is always >= 0 by type definition + let durable = ctx.raft_log.durable_index(); + // Verify durable_index reflects the highest contiguous or flushed index + assert!(durable >= 4, "durable_index should be at least 4 after flush");d-engine-core/src/storage/buffered_raft_log_test/concurrent_operations_test.rs (1)
4-4: Remove unused import.The
use tokio;import is unused since the code usestokio::spawnandtokio::time::sleepwith full paths, and#[tokio::test]is an attribute.🧹 Suggested fix
use futures::future::join_all; -use tokio;d-engine-core/src/raft_role/leader_state_test/membership_change_test.rs (4)
562-592: Tests don't verify error conditions are actually handled.Both
test_trigger_background_snapshot_peer_channel_missingandtest_trigger_background_snapshot_snapshot_stream_errorassertresult.is_ok()becausetrigger_background_snapshotreturns immediately before the background task completes. This means the tests don't actually verify the error paths are triggered or logged correctly.Consider either:
- Adding a mechanism to await the background task completion
- Using a logging spy to verify error logs are emitted
- Renaming tests to clarify they only test the fire-and-forget pattern
1005-1031: Clarify threshold boundary behavior in test documentation.The test checks boundary conditions with entries at 31s (over), 30s (exactly at), and 29s (under) threshold. The assertion
len() == 2suggests entries at or over the threshold are purged (using>=comparison), but the comment on line 1028 says "node 101, 102" should be purged, leaving only node 103.Consider adding a comment clarifying whether the threshold comparison is inclusive (
>=) or exclusive (>) to help future maintainers understand the expected behavior.
1449-1490: Test fixture function has side effects beyond setup.
verify_internal_quorum_achieved_contextnot only creates the context but also runsverify_internal_quorumwith assertions (lines 1480-1487). While this validates the mock setup works, it mixes setup concerns with test validation. Consider separating these or documenting the intentional side effects.
721-722: Consider documenting why tests are ignored.Several tests in
batch_promote_learners_testare marked#[ignore]without explanation. Adding a brief comment about why (e.g., "requires real network", "long-running", "manual verification needed") would help future maintainers understand when to run these tests.Also applies to: 743-745, 763-765, 783-785
d-engine-core/src/storage/buffered_raft_log_test/performance_test.rs (2)
27-32: Consider usingtokio::time::sleepinstead ofstd::thread::sleepin async context.Using
std::thread::sleepinside a closure that may be called from an async context blocks the entire executor thread. While this is intentional here to simulate blocking I/O, it can cause issues if the Tokio runtime uses a limited thread pool. Since this is test code simulating disk delays, the current approach works, but be aware it blocks the runtime thread.If you want non-blocking behavior while still simulating delay:
Alternative using spawn_blocking
log_store.expect_persist_entries().returning(move |_| { let delay = Duration::from_millis(delay_ms); - std::thread::sleep(delay); + // Note: For tests, blocking sleep is acceptable to simulate disk I/O + // If non-blocking is needed, wrap in spawn_blocking Ok(()) });
38-93: Timing-based tests may be flaky - consider adding#[ignore]or documenting flakiness.This test relies on timing thresholds (50ms/500ms) that can fail on heavily loaded CI machines even with the CI adjustment. The race between
barrier.wait()and the actual flush starting introduces non-determinism.Consider:
- Adding
#[ignore]attribute with a comment for manual runs- Increasing CI thresholds further as a safety margin
- Using a more deterministic synchronization (e.g., a notification when flush actually starts)
Example: Mark as ignored for CI with manual run option
+#[ignore = "Performance test - may be flaky in CI, run manually with --ignored"] #[tokio::test] async fn test_reset_performance_during_active_flush() {d-engine-core/src/storage/buffered_raft_log_test/edge_cases_test.rs (1)
60-68: Consider batching entries for efficiency.Appending 100 entries one at a time is inefficient. While acceptable for tests, batching would be faster and more representative of typical usage.
Batch append for efficiency
- // Create entries 1..=100 - for i in 1..=100 { - let entry = Entry { - index: i, - term: 1, - payload: None, - }; - ctx.raft_log.append_entries(vec![entry]).await.unwrap(); - } + // Create entries 1..=100 in batch + let entries: Vec<Entry> = (1..=100) + .map(|i| Entry { + index: i, + term: 1, + payload: None, + }) + .collect(); + ctx.raft_log.append_entries(entries).await.unwrap();d-engine-core/src/raft_role/leader_state_test/snapshot_test.rs (1)
30-35: Duplicate helper functioncreate_mock_membershipacross test modules.This helper is duplicated in
snapshot_test.rs,event_handling_test.rs,state_management_test.rs,membership_change_test.rs, andreplication_test.rs. Consider extracting to a shared test utility module.Extract to shared test utilities
// In d-engine-core/src/test_utils/mock/mod.rs or similar pub fn create_mock_membership() -> MockMembership<MockTypeConfig> { let mut membership = MockMembership::new(); membership.expect_is_single_node_cluster().returning(|| false); membership }Based on the relevant code snippets showing this pattern repeated in multiple files.
d-engine-core/src/raft_role/leader_state_test/event_handling_test.rs (1)
708-718: Pattern match variable is unused - consider removing or using.The pattern captures
_expect_new_commit_indexbut doesn't use it for validation. Either remove it or add an assertion.Option 1: Use the captured value for validation
assert!(matches!( event, RoleEvent::NotifyNewCommitIndex(NewCommitData { - new_commit_index: _expect_new_commit_index, + new_commit_index: 3, role: _, current_term: _ }) ));Option 2: Use wildcard if value doesn't matter
assert!(matches!( event, RoleEvent::NotifyNewCommitIndex(NewCommitData { - new_commit_index: _expect_new_commit_index, + new_commit_index: _, role: _, current_term: _ }) ));d-engine-core/src/raft_role/leader_state_test/state_management_test.rs (1)
105-114: Redundant assertions - both test the same condition.Lines 105-108 and 111-114 test identical conditions (
last_purge=90,snapshot=99). The second assertion doesn't add value.Remove redundant assertion
// Valid purge window (last_purge=90 < snapshot=99 < commit=100) assert!(state.can_purge_logs( Some(LogId { index: 90, term: 1 }), // last_purge_index LogId { index: 99, term: 1 } // last_included_in_snapshot )); - // Boundary check: 99 == commit_index - 1 (valid gap) - assert!(state.can_purge_logs( - Some(LogId { index: 90, term: 1 }), - LogId { index: 99, term: 1 } - )); - // Violate gap rule: 100 not < 100d-engine-core/src/replication/replication_handler_test/basic_scenarios_test.rs (4)
31-37: Helper function returns misleading configuration for varied cluster sizes.The helper
create_mock_membership_multi_nodehardcodesinitial_cluster_sizeto 3, but it's used across tests for 2, 3, and 5-node clusters. While this may not affect current test behavior ifinitial_cluster_sizeisn't actively used, it could cause confusion or subtle bugs if the implementation starts relying on it.Consider parameterizing or renaming for clarity:
♻️ Suggested improvement
-/// Helper: Create mock membership for multi-node cluster -fn create_mock_membership_multi_node() -> MockMembership<MockTypeConfig> { +/// Helper: Create mock membership for multi-node cluster +fn create_mock_membership_multi_node(cluster_size: usize) -> MockMembership<MockTypeConfig> { let mut membership = MockMembership::new(); membership.expect_is_single_node_cluster().returning(|| false); - membership.expect_initial_cluster_size().returning(|| 3); + membership.expect_initial_cluster_size().returning(move || cluster_size); membership }
88-104: Inconsistent mock configuration between membership and ClusterMetadata.The test uses
create_mock_membership_multi_node()which setsis_single_node_cluster()to returnfalse, but then passesClusterMetadata { single_voter: true, ... }. This inconsistency could mask bugs if the implementation checks both sources differently.For a single-voter test, consider creating a dedicated single-node membership mock:
♻️ Suggested fix
- // Arrange: Mock membership with no replication peers - let mut membership = create_mock_membership_multi_node(); + // Arrange: Mock membership for single-node cluster + let mut membership = MockMembership::new(); + membership.expect_is_single_node_cluster().returning(|| true); + membership.expect_initial_cluster_size().returning(|| 1); membership.expect_replication_peers().returning(Vec::new); context.membership = Arc::new(membership);
202-207: Consider adding quorum boundary tests for comprehensive coverage.These tests verify quorum when all peers respond successfully, which is valid. However, the Raft quorum only requires a majority (2/3 for 3-node, 3/5 for 5-node). Consider adding tests that verify quorum is achieved with exactly the minimum required responses while others fail/timeout.
This would better validate the quorum calculation logic at its boundaries.
Also applies to: 299-304
46-53: Hardcoded/tmppaths may cause test isolation issues.All tests use hardcoded
/tmp/test_*paths. If tests run in parallel, they could interfere with each other. Consider using unique paths per test run:♻️ Example using tempdir or unique suffix
// Option 1: Use tempfile crate let tmp_dir = tempfile::tempdir().unwrap(); let path = tmp_dir.path().to_str().unwrap(); // Option 2: Add unique suffix let path = format!("/tmp/test_single_voter_achieves_quorum_immediately_{}", nanoid!());Also applies to: 128-135, 209-216, 306-313
d-engine-core/src/raft_role/leader_state_test/client_write_test.rs (1)
282-292: Consider verifying both NotifyNewCommitIndex events.The test expects
handle_raft_request_in_batchto be called twice, but only verifies oneNotifyNewCommitIndexevent. If each batch produces an event, consider draining the channel to verify both events (or confirming that only one event is expected).d-engine-core/src/test_utils/buffered_raft_log_test_helpers.rs (2)
46-49: Replace sleep-based synchronization with proper async waiting.Using
std::thread::sleepto wait for the background processor is fragile and can cause flaky tests on slower CI systems. Consider adding a ready signal or using a proper synchronization mechanism.♻️ Suggested approach
// Option 1: Add a ready method to BufferedRaftLog that can be awaited // Option 2: Use tokio::time::sleep in async context for consistency tokio::time::sleep(Duration::from_millis(10)).await; // Option 3: Best - add an initialization barrier or ready signal // In BufferedRaftLog::start(): // let (ready_tx, ready_rx) = oneshot::channel(); // // signal ready_tx when processor loop starts // Then await ready_rx here instead of sleepingAlso applies to: 93-96
160-202: Consider consistent API for simulate commands.
simulate_insert_commandtakesVec<u64>whilesimulate_delete_commandtakesRangeInclusive<u64>. For API consistency, consider using the same input type or providing overloads for both.♻️ Optional: Consistent API example
// Option 1: Both take iterators pub async fn simulate_insert_command( raft_log: &Arc<BufferedRaftLog<MockTypeConfig>>, ids: impl IntoIterator<Item = u64>, term: u64, ) pub async fn simulate_delete_command( raft_log: &Arc<BufferedRaftLog<MockTypeConfig>>, ids: impl IntoIterator<Item = u64>, term: u64, )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockexamples/client-usage-standalone/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (112)
Cargo.tomld-engine-core/Cargo.tomld-engine-core/benches/leader_state_bench.rsd-engine-core/src/commit_handler/mod.rsd-engine-core/src/election/election_handler_test.rsd-engine-core/src/election/mod.rsd-engine-core/src/event.rsd-engine-core/src/lib.rsd-engine-core/src/maybe_clone_oneshot.rsd-engine-core/src/membership.rsd-engine-core/src/network/mod.rsd-engine-core/src/purge/mod.rsd-engine-core/src/raft.rsd-engine-core/src/raft_context.rsd-engine-core/src/raft_role/candidate_state.rsd-engine-core/src/raft_role/candidate_state_test.rsd-engine-core/src/raft_role/follower_state_test.rsd-engine-core/src/raft_role/leader_state.rsd-engine-core/src/raft_role/leader_state_test/client_read_test.rsd-engine-core/src/raft_role/leader_state_test/client_write_test.rsd-engine-core/src/raft_role/leader_state_test/cluster_metadata_test.rsd-engine-core/src/raft_role/leader_state_test/event_handling_test.rsd-engine-core/src/raft_role/leader_state_test/membership_change_test.rsd-engine-core/src/raft_role/leader_state_test/mod.rsd-engine-core/src/raft_role/leader_state_test/replication_test.rsd-engine-core/src/raft_role/leader_state_test/snapshot_test.rsd-engine-core/src/raft_role/leader_state_test/state_management_test.rsd-engine-core/src/raft_role/learner_state_test.rsd-engine-core/src/raft_role/mod.rsd-engine-core/src/replication/mod.rsd-engine-core/src/replication/replication_handler_test/basic_scenarios_test.rsd-engine-core/src/replication/replication_handler_test/command_handling_test.rsd-engine-core/src/replication/replication_handler_test/edge_cases_test.rsd-engine-core/src/replication/replication_handler_test/helper_functions_test.rsd-engine-core/src/replication/replication_handler_test/index_tracking_test.rsd-engine-core/src/replication/replication_handler_test/learner_management_test.rsd-engine-core/src/replication/replication_handler_test/log_retrieval_test.rsd-engine-core/src/replication/replication_handler_test/mod.rsd-engine-core/src/replication/replication_handler_test/quorum_calculation_test.rsd-engine-core/src/state_machine_handler/default_state_machine_handler.rsd-engine-core/src/state_machine_handler/mod.rsd-engine-core/src/state_machine_handler/snapshot_policy/mod.rsd-engine-core/src/storage/buffered_raft_log.rsd-engine-core/src/storage/buffered_raft_log_test/basic_operations_test.rsd-engine-core/src/storage/buffered_raft_log_test/concurrent_operations_test.rsd-engine-core/src/storage/buffered_raft_log_test/crash_recovery_test.rsd-engine-core/src/storage/buffered_raft_log_test/durable_index_test.rsd-engine-core/src/storage/buffered_raft_log_test/edge_cases_test.rsd-engine-core/src/storage/buffered_raft_log_test/flush_strategy_test.rsd-engine-core/src/storage/buffered_raft_log_test/helper_functions_test.rsd-engine-core/src/storage/buffered_raft_log_test/id_allocation_test.rsd-engine-core/src/storage/buffered_raft_log_test/mod.rsd-engine-core/src/storage/buffered_raft_log_test/performance_test.rsd-engine-core/src/storage/buffered_raft_log_test/raft_properties_test.rsd-engine-core/src/storage/buffered_raft_log_test/remove_range_test.rsd-engine-core/src/storage/buffered_raft_log_test/shutdown_test.rsd-engine-core/src/storage/buffered_raft_log_test/term_index_test.rsd-engine-core/src/storage/buffered_raft_log_test/worker_test.rsd-engine-core/src/storage/mod.rsd-engine-core/src/storage/raft_log.rsd-engine-core/src/storage/state_machine.rsd-engine-core/src/storage/storage_engine.rsd-engine-core/src/test_utils/buffered_raft_log_test_helpers.rsd-engine-core/src/test_utils/mock/mod.rsd-engine-core/src/test_utils/mod.rsd-engine-core/src/test_utils/replication_test_helpers.rsd-engine-core/src/watch/manager.rsd-engine-server/Cargo.tomld-engine-server/src/lib.rsd-engine-server/src/membership/raft_membership_test.rsd-engine-server/src/network/connection_cache_test.rsd-engine-server/src/network/grpc/grpc_transport_test.rsd-engine-server/src/network/health_checker.rsd-engine-server/src/network/health_checker_test.rsd-engine-server/src/node/builder.rsd-engine-server/src/storage/adaptors/file/file_storage_engine.rsd-engine-server/src/storage/buffered/buffered_raft_log_test.rsd-engine-server/src/storage/buffered/mod.rsd-engine-server/src/test_utils/integration/mod.rsd-engine-server/src/test_utils/mock/mock_rpc.rsd-engine-server/src/test_utils/mock/mock_rpc_service.rsd-engine-server/src/test_utils/mock/mod.rsd-engine-server/src/test_utils/mock/snapshot.rsd-engine-server/src/test_utils/mock/stream.rsd-engine-server/src/test_utils/mod.rsd-engine-server/tests/components/buffered_raft_log_perf_test.rsd-engine-server/tests/components/election/election_handler_test.rsd-engine-server/tests/components/election/mod.rsd-engine-server/tests/components/mod.rsd-engine-server/tests/components/raft_role/candidate_state_test.rsd-engine-server/tests/components/raft_role/follower_state_test.rsd-engine-server/tests/components/raft_role/leader_state_test.rsd-engine-server/tests/components/raft_role/learner_state_test.rsd-engine-server/tests/components/raft_role/mod.rsd-engine-server/tests/components/replication/mod.rsd-engine-server/tests/components/replication/replication_handler_test.rsd-engine-server/tests/consistent_reads/linearizable_read_batching_embedded.rsd-engine-server/tests/integration_test.rsd-engine-server/tests/snapshot_and_recovery/snapshot_concurrent_replication_embedded.rsd-engine-server/tests/storage_buffered_raft_log/crash_recovery_test.rsd-engine-server/tests/storage_buffered_raft_log/mod.rsd-engine-server/tests/storage_buffered_raft_log/performance_test.rsd-engine-server/tests/storage_buffered_raft_log/storage_integration_test.rsd-engine-server/tests/storage_buffered_raft_log/stress_test.rsexamples/README.mdexamples/client-usage-standalone/.gitignoreexamples/client-usage-standalone/Cargo.tomlexamples/client-usage-standalone/README.mdexamples/client-usage-standalone/src/main.rsexamples/client_usage/README.mdexamples/sled-cluster/Cargo.tomlexamples/three-nodes-standalone/Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (31)
d-engine-core/src/raft_role/candidate_state.rs (4)
d-engine-core/src/raft_role/mod.rs (1)
new(131-154)d-engine-server/src/test_utils/mock/mock_node_builder.rs (1)
new(139-160)d-engine-core/src/raft_role/follower_state.rs (1)
new(615-637)d-engine-core/src/raft_role/learner_state.rs (1)
new(622-631)
d-engine-core/src/raft_role/learner_state_test.rs (2)
d-engine-core/src/test_utils/mock/mod.rs (2)
mock_raft_context(57-71)mock_raft_context_with_temp(89-100)d-engine-core/src/raft_role/mod.rs (5)
state(237-244)tick(360-371)handle_raft_event(373-383)current_term(178-180)current_term(321-323)
d-engine-core/src/raft_role/leader_state_test/membership_change_test.rs (1)
d-engine-core/src/raft_role/leader_state.rs (3)
trigger_background_snapshot(2219-2263)handle_stale_learner(2558-2596)calculate_safe_batch_size(2811-2823)
d-engine-core/src/storage/buffered_raft_log_test/basic_operations_test.rs (3)
d-engine-core/src/test_utils/buffered_raft_log_test_helpers.rs (3)
mock_empty_entries(126-138)simulate_delete_command(186-202)simulate_insert_command(164-180)d-engine-core/src/storage/buffered_raft_log.rs (8)
len(1182-1184)entry(150-155)last_entry_id(161-163)is_empty(186-188)is_empty(1219-1221)last_log_id(174-184)drop(1228-1235)last_entry(165-172)d-engine-core/src/raft_context.rs (1)
raft_log(55-57)
d-engine-core/src/replication/replication_handler_test/log_retrieval_test.rs (2)
d-engine-core/src/test_utils/replication_test_helpers.rs (3)
mock_insert_log_entries(130-145)mock_log_entries_exist(77-110)setup_mock_replication_test_context(50-57)d-engine-server/tests/common/mod.rs (1)
generate_insert_commands(459-468)
d-engine-core/src/storage/buffered_raft_log_test/id_allocation_test.rs (1)
d-engine-core/src/storage/buffered_raft_log.rs (5)
new(535-628)pre_allocate_raft_logs_next_index(246-249)next_id(1208-1210)start(631-646)len(1182-1184)
d-engine-core/src/storage/buffered_raft_log_test/term_index_test.rs (1)
d-engine-core/src/storage/buffered_raft_log.rs (4)
new(535-628)first_index_for_term(232-237)last_index_for_term(239-244)start(631-646)
d-engine-core/src/storage/buffered_raft_log_test/shutdown_test.rs (3)
d-engine-core/src/raft_context.rs (1)
raft_log(55-57)d-engine-core/src/test_utils/buffered_raft_log_test_helpers.rs (1)
new(29-58)d-engine-proto/src/exts/common_ext.rs (1)
command(10-14)
d-engine-core/src/test_utils/replication_test_helpers.rs (2)
d-engine-server/tests/common/mod.rs (1)
generate_insert_commands(459-468)d-engine-proto/src/exts/common_ext.rs (1)
command(10-14)
d-engine-core/src/raft_role/leader_state_test/state_management_test.rs (7)
d-engine-core/src/test_utils/mock/mod.rs (1)
mock_raft_context(57-71)d-engine-core/src/raft_role/leader_state_test/event_handling_test.rs (1)
create_mock_membership(47-51)d-engine-core/src/raft_role/leader_state_test/membership_change_test.rs (2)
create_mock_membership(44-48)new(1408-1447)d-engine-core/src/raft_role/leader_state_test/replication_test.rs (1)
create_mock_membership(46-50)d-engine-core/src/raft_role/leader_state_test/snapshot_test.rs (1)
create_mock_membership(31-35)d-engine-core/src/raft_context.rs (1)
membership(82-84)d-engine-core/src/raft_role/mod.rs (4)
new(131-154)state(237-244)current_term(178-180)current_term(321-323)
d-engine-core/src/test_utils/buffered_raft_log_test_helpers.rs (1)
d-engine-core/src/storage/buffered_raft_log.rs (4)
new(535-628)append_entries(276-349)start(631-646)entry(150-155)
d-engine-core/src/storage/buffered_raft_log_test/durable_index_test.rs (2)
d-engine-core/src/raft_context.rs (1)
raft_log(55-57)d-engine-core/src/test_utils/buffered_raft_log_test_helpers.rs (1)
new(29-58)
d-engine-core/src/storage/buffered_raft_log_test/raft_properties_test.rs (2)
d-engine-core/src/raft_context.rs (1)
raft_log(55-57)d-engine-core/src/test_utils/buffered_raft_log_test_helpers.rs (2)
simulate_insert_command(164-180)new(29-58)
d-engine-core/src/replication/replication_handler_test/helper_functions_test.rs (3)
d-engine-core/src/replication/replication_handler.rs (1)
client_command_to_entry_payloads(683-695)d-engine-core/src/test_utils/replication_test_helpers.rs (2)
mock_insert_log_entries(130-145)setup_mock_replication_test_context(50-57)d-engine-proto/src/exts/replication_ext.rs (2)
is_success(53-58)is_conflict(61-66)
d-engine-core/src/storage/buffered_raft_log_test/performance_test.rs (3)
d-engine-core/src/storage/buffered_raft_log.rs (2)
new(535-628)start(631-646)d-engine-core/src/test_utils/buffered_raft_log_test_helpers.rs (1)
new(29-58)d-engine-proto/src/exts/common_ext.rs (1)
command(10-14)
d-engine-server/src/network/connection_cache_test.rs (1)
d-engine-server/src/test_utils/mock/mock_rpc_service.rs (1)
mock_listener(36-114)
d-engine-core/src/storage/buffered_raft_log_test/remove_range_test.rs (1)
d-engine-core/src/raft_context.rs (1)
raft_log(55-57)
d-engine-core/src/raft_role/follower_state_test.rs (2)
d-engine-core/src/test_utils/mock/mod.rs (2)
mock_raft_context(57-71)mock_raft_context_with_temp(89-100)d-engine-core/src/raft_context.rs (5)
election_handler(70-72)membership(82-84)replication_handler(66-68)state_machine_handler(74-76)purge_executor(86-88)
d-engine-server/src/membership/raft_membership_test.rs (3)
d-engine-server/src/test_utils/mock/mock_rpc_service.rs (1)
mock_listener(36-114)d-engine-client/src/mock_rpc_service.rs (1)
mock_listener(23-80)d-engine-core/src/test_utils/mock/mock_rpc_service.rs (1)
mock_listener(35-113)
d-engine-core/src/storage/buffered_raft_log_test/flush_strategy_test.rs (3)
d-engine-core/src/storage/buffered_raft_log.rs (5)
new(535-628)durable_index(1191-1193)entry(150-155)len(1182-1184)drop(1228-1235)d-engine-core/src/test_utils/buffered_raft_log_test_helpers.rs (1)
new(29-58)d-engine-proto/src/exts/common_ext.rs (1)
command(10-14)
d-engine-server/src/network/health_checker_test.rs (1)
d-engine-server/src/test_utils/mock/mock_rpc_service.rs (1)
mock_listener(36-114)
d-engine-core/src/raft_role/candidate_state_test.rs (2)
d-engine-core/src/raft_role/candidate_state.rs (1)
can_vote_myself(571-585)d-engine-core/src/maybe_clone_oneshot.rs (1)
recv(86-93)
d-engine-core/src/storage/mod.rs (2)
d-engine-core/src/raft_context.rs (2)
raft_log(55-57)state_machine(59-61)d-engine-server/src/node/builder.rs (2)
state_machine(200-206)storage_engine(191-197)
d-engine-core/src/storage/buffered_raft_log_test/worker_test.rs (1)
d-engine-core/src/raft_context.rs (1)
raft_log(55-57)
d-engine-core/src/raft_role/mod.rs (3)
d-engine-core/src/raft_role/leader_state.rs (3)
voted_for(299-301)match_index(342-347)next_index(312-321)d-engine-core/src/raft_role/role_state.rs (5)
voted_for(267-269)commit_index(220-222)match_index(65-71)node_id(42-44)next_index(49-55)d-engine-core/src/raft_role/learner_state.rs (1)
voted_for(136-140)
d-engine-core/src/raft_role/leader_state_test/client_write_test.rs (4)
d-engine-core/src/replication/replication_handler.rs (1)
client_command_to_entry_payloads(683-695)d-engine-core/src/raft_role/mod.rs (3)
new(131-154)state(237-244)commit_index(332-334)d-engine-core/src/raft_role/leader_state_test/membership_change_test.rs (1)
new(1408-1447)d-engine-core/src/raft_context.rs (2)
replication_handler(66-68)raft_log(55-57)
d-engine-core/src/storage/buffered_raft_log.rs (1)
d-engine-core/src/storage/state_machine.rs (1)
len(68-68)
d-engine-core/src/raft.rs (1)
d-engine-core/src/event.rs (1)
raft_event_to_test_event(181-212)
d-engine-core/benches/leader_state_bench.rs (1)
d-engine-core/src/raft_context.rs (8)
node_config(78-80)raft_log(55-57)state_machine(59-61)transport(63-65)replication_handler(66-68)state_machine_handler(74-76)purge_executor(86-88)membership(82-84)
d-engine-core/src/storage/buffered_raft_log_test/concurrent_operations_test.rs (3)
d-engine-core/src/raft_context.rs (1)
raft_log(55-57)d-engine-core/src/test_utils/buffered_raft_log_test_helpers.rs (2)
simulate_insert_command(164-180)new(29-58)d-engine-core/src/storage/buffered_raft_log.rs (6)
new(535-628)len(1182-1184)entry(150-155)start(631-646)first_entry_id(157-159)last_entry_id(161-163)
d-engine-core/src/election/election_handler_test.rs (2)
d-engine-core/src/election/election_handler.rs (1)
check_vote_request_is_legal(281-323)d-engine-core/src/election/mod.rs (1)
check_vote_request_is_legal(79-86)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
d-engine-core/src/storage/buffered_raft_log_test/basic_operations_test.rs
Outdated
Show resolved
Hide resolved
d-engine-core/src/storage/buffered_raft_log_test/helper_functions_test.rs
Outdated
Show resolved
Hide resolved
JoshuaChi
force-pushed
the
feature/245_refactor_server_ut
branch
from
2c6ea0e to
39d8280
Compare
Type
Related Issues
Checklist
Summary by CodeRabbit
Release Notes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.