Skip to content

Conversation

@ZocoLini
Copy link
Collaborator

@ZocoLini ZocoLini commented Jan 26, 2026

Summary by CodeRabbit

  • Tests
    • Added tracking of outbound network messages in the shared test network mock with a new sent_messages accessor.
    • Replaced duplicated in-file mocks with a consolidated shared MockNetworkManager across tests.
    • Updated tests to use the synchronous message inspection API instead of async message retrieval.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 26, 2026

📝 Walkthrough

Walkthrough

The PR updates the test utilities' MockNetworkManager to record outbound messages and refactors two tests to use this shared mock, replacing their local mock implementations and switching to the new synchronous sent_messages() accessor.

Changes

Cohort / File(s) Summary
MockNetworkManager Enhancement
dash-spv/src/test_utils/network.rs
Added sent_messages: Vec<NetworkMessage> field and pub fn sent_messages(&self) -> &Vec<NetworkMessage> accessor. send_message() now appends processed outgoing messages to sent_messages for inspection.
Test Refactoring to Use Shared Mock
dash-spv/tests/edge_case_filter_sync_test.rs, dash-spv/tests/filter_header_verification_test.rs
Removed in-file mock network implementations and imports; now import and use dash_spv::test_utils::MockNetworkManager. Updated tests to read synchronous sent_messages() instead of prior async/local APIs; test logic and assertions preserved.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 I hopped through code with nimble paws,
I tucked each message in my laws.
One cozy mock, no copies more,
I store the sends behind the door.
Tests nibble happily, encore! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main objective: consolidating multiple mock network implementations into a single unified MockNetworkManager used across test files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the merge-conflict The PR conflicts with the target branch. label Jan 27, 2026
@github-actions
Copy link

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

@ZocoLini ZocoLini force-pushed the chore/mock-network-unified branch from 6484b36 to 2d14540 Compare January 27, 2026 14:53
@github-actions github-actions bot removed the merge-conflict The PR conflicts with the target branch. label Jan 27, 2026
@xdustinface xdustinface merged commit 4fcf5e4 into v0.42-dev Jan 27, 2026
53 checks passed
@xdustinface xdustinface deleted the chore/mock-network-unified branch January 27, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants