chore: drop unused integration tests #362
Conversation
📝 WalkthroughWalkthroughThis PR removes integration test code, test guides, and documentation referencing tests that require a real Dash Core node. Deleted items include two Rust test files, a large integration test module, a 192-line run guide, and related documentation updates — totaling ~1,039 lines removed. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
💤 Files with no reviewable changes (6)
🚧 Files skipped from review as they are similar to previous changes (1)
✏️ Tip: You can disable this entire section by setting 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
dash-spv/CLAUDE.md (2)
125-137: Remove or update outdated integration test documentation.This section still references "Real network tests" requiring a "Dash Core node at
127.0.0.1:9999for integration tests" and claims "Integration: Real node connectivity and performance (6/6 passing)". However, this PR explicitly removes integration tests that depend on actual networks. This documentation is now inconsistent with the codebase state and will mislead developers.Please update this section to reflect that the real-node integration tests have been removed, or clarify the distinction between different test categories if some integration tests remain.
📝 Suggested documentation update
If all real-node integration tests were removed, consider updating the "Test Data Requirements" section to:
### Test Data Requirements -- Dash Core node at `127.0.0.1:9999` for integration tests -- Tests gracefully handle node unavailability -- Performance benchmarks expect 50-200+ headers/second sync rates +- Tests use in-memory or mock implementations and don't require external nodes +- Real network tests with live Dash Core nodes will be added in a future PR (`#257`)And update the test status in line 133:
-4. **Integration**: Real node connectivity and performance (6/6 passing) +4. **Integration**: Mock-based tests (status: to be added in PR `#257`)
206-208: Update debugging guidance to reflect removed integration tests.The "Network Debugging" section still instructs developers to "Check if Dash Core node is running at
127.0.0.1:9999", which assumes the presence of real-node integration tests that are being removed in this PR. This guidance is no longer applicable.📝 Suggested fix
**Network Debugging:** -- Connection issues: Check if Dash Core node is running at `127.0.0.1:9999` -- Handshake failures: Verify network (mainnet/testnet/devnet) matches node -- Timeout issues: Node may be syncing or under load +- Connection issues: Verify network connectivity and peer addresses +- Handshake failures: Check that network parameters (mainnet/testnet/devnet) are configured correctly +- Timeout issues: Network conditions may affect peer connectivity
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
AGENTS.mdCLAUDE.mddash-spv/CLAUDE.mddash-spv/run_integration_tests.mddash-spv/tests/integration_real_node_test.rsdash-spv/tests/test_plan.md
💤 Files with no reviewable changes (5)
- CLAUDE.md
- AGENTS.md
- dash-spv/tests/test_plan.md
- dash-spv/run_integration_tests.md
- dash-spv/tests/integration_real_node_test.rs
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase, built on tokio runtime
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*test*.rs : Test both mainnet and testnet configurations
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*integration*.rs : Write integration tests for network operations
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Run 'cargo fmt --check' to verify formatting, 'cargo clippy --all-targets --all-features -- -D warnings' for linting, and 'cargo check --all-features' to verify all features compile
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*.rs : Write unit tests for new functionality in Rust
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Mark network-dependent or long-running tests with `#[ignore]` and run with `-- --ignored`
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/*.rs : Unit tests should live alongside code with `#[cfg(test)]` annotation; integration tests use the `tests/` directory
Applied to files:
dash-spv/CLAUDE.md
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use deterministic testing with known test vectors and fixed seeds for reproducible results
Applied to files:
dash-spv/CLAUDE.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: Pre-commit (windows-latest)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: Pre-commit (ubuntu-latest)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: Pre-commit (macos-latest)
🔇 Additional comments (1)
dash-spv/CLAUDE.md (1)
190-190: No action needed. The testtest_handshake_with_mainnet_peerexists atdash-spv/tests/handshake_test.rs:11and correctly handles the case when a Dash Core node is unavailable at 127.0.0.1:9999 by logging a message rather than failing the test (lines 41–48). The documentation reference is accurate and appropriate.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
xdustinface
force-pushed
the
chore/drop-integration-tests
branch
from
fc6c17b to
66ea8d2
Compare
|
Do we currently have a way to test the client against a network other than running the examples?? |
We have the examples and the CLI. Not a test which runs in the actual network but looking at those tests they are not very helpful either even if they would be enabled and they remove some clutter for the sync migration. I think we should eventually have some proper full sync to the network which runs nightly or so but for now i would like to drop those. |
ZocoLini
left a comment
ZocoLini
left a comment
There was a problem hiding this comment.
If they are being an obstacle for the new sync I can approve this but we definetly need good tests for the full sync process. Maybe simulate a blockchsin with idk, 1000 blocks using the dummy constructors I introduce in the test-utils PR
See PR description :) |
|
oh, okey nice |
They don't run on CI, don't test much and probably don't work really anyway and they imo never will run on CI since they depend on actual networks. I have better tests coming in testing with local test blockchains in #257.
Summary by CodeRabbit
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.