chore: remove flaky handshake_test.rs integration tests#574
chore: remove flaky handshake_test.rs integration tests#574xdustinface wants to merge 1 commit intov0.42-devfrom
handshake_test.rs integration tests#574Conversation
📝 WalkthroughWalkthroughThe pull request removes the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
All three tests in this file were effectively no-ops in CI: - `test_handshake_timeout` used timing assertions on a connection to an RFC 5737 address. The lower bound (`>= 1s`) fails when the OS returns an immediate ICMP unreachable and the upper bound (`< 5s`) can fail under CI load. The remaining logic just tests tokio's `timeout` wrapper, not project code. - `test_handshake_with_mainnet_peer` and `test_multiple_connect_disconnect_cycles` silently pass when no Dash Core node is running at 127.0.0.1:9999.
73a0745 to
3d2a896
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dash-spv/CLAUDE.md (1)
127-132:⚠️ Potential issue | 🟡 MinorUpdate stale test-category summary to remove handshake/count claims.
The section still says “Network layer: Handshake ... (3/4 passing),” which is now outdated after removing
handshake_test.rs. Please sync this summary with the current test plan state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dash-spv/CLAUDE.md` around lines 127 - 132, Update the stale "Test Categories (from `tests/test_plan.md`)" summary: remove the obsolete "Handshake" mention and the hard-coded "(3/4 passing)" claim from the "Network layer: Handshake, connection management (3/4 passing)" line in CLAUDE.md, and replace it with an accurate, current phrase (e.g., "Network layer: connection management") and an updated pass/fail count that matches the live tests by syncing with tests/test_plan.md; ensure the "Test Categories" section text reflects the current test_plan.md contents.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@dash-spv/CLAUDE.md`:
- Around line 127-132: Update the stale "Test Categories (from
`tests/test_plan.md`)" summary: remove the obsolete "Handshake" mention and the
hard-coded "(3/4 passing)" claim from the "Network layer: Handshake, connection
management (3/4 passing)" line in CLAUDE.md, and replace it with an accurate,
current phrase (e.g., "Network layer: connection management") and an updated
pass/fail count that matches the live tests by syncing with tests/test_plan.md;
ensure the "Test Categories" section text reflects the current test_plan.md
contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48dd95da-5dd1-44d8-a6d0-e1d696b353ff
📒 Files selected for processing (3)
dash-spv/CLAUDE.mddash-spv/tests/handshake_test.rsdash-spv/tests/test_plan.md
💤 Files with no reviewable changes (1)
- dash-spv/tests/handshake_test.rs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #574 +/- ##
=============================================
- Coverage 66.32% 66.30% -0.03%
=============================================
Files 311 311
Lines 64976 64976
=============================================
- Hits 43097 43083 -14
- Misses 21879 21893 +14
|
All three tests in this file were effectively no-ops in CI:
test_handshake_timeoutused timing assertions on a connection to an RFC 5737 address. The lower bound (>= 1s) fails when the OS returns an immediate ICMP unreachable and the upper bound (< 5s) can fail under CI load. The remaining logic just tests tokio'stimeoutwrapper, not project code.test_handshake_with_mainnet_peerandtest_multiple_connect_disconnect_cyclessilently pass when no Dash Core node is running at 127.0.0.1:9999.Summary by CodeRabbit
Tests
Documentation