feat: update integration test script for Dash compatibility #136
Conversation
WalkthroughThe integration test script updates the second regtest node to use dashd instead of bitcoind, adjusts datadir, ports, connection wiring, RPC URL, and cookie paths, removes version-based argument blocks, and updates cleanup to terminate dashd. The test still launches two nodes, runs cargo via RPC, and kills both processes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test Runner
participant N1 as bitcoind (node 1)
participant N2 as dashd (node 2)
T->>N1: Start bitcoind -regtest -rpcport 12348 -server -txindex
T->>N2: Start dashd -regtest -rpcport 12349 -connect 127.0.0.1:12348 -server -txindex
Note over N2: Uses datadir ./2 and regtest cookie
T->>N2: RPC calls via http://localhost:12349
Note over T,N2: Cargo tests exercise RPC on node 2
T-->>N1: On completion/failure: kill -9 PID1
T-->>N2: On completion/failure: kill -9 PID2
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rpc-integration-test/run.sh (1)
21-28: dashd cannot peer with bitcoind; replace the first node or drop the P2P wiring.Dash Core and Bitcoin Core are wire-incompatible;
dashd -connect=127.0.0.1:12348will never handshake with thebitcoindnode. The test won’t form a network and the second node’s p2p won’t connect. Either:
- run both nodes as
dashd, or- simplify to a single
dashdand remove the-connectrequirement if P2P isn’t needed for the RPC test.Minimal change within this hunk (decouple from the Bitcoin peer):
dashd -regtest \ -datadir=${TESTDIR}/2 \ - -connect=127.0.0.1:12348 \ -rpcport=12349 \ -server=1 \ -txindex=1 \ -printtoconsole=0 &If you want a two-node Dash regtest, update the earlier node to
dashdand wire with-addnode/-connectbetween the twodashdinstances. Example (outside selected lines, full context for clarity):# Node A dashd -regtest -datadir="${TESTDIR}/1" -port=12348 -server=0 -printtoconsole=0 & PID1=$! # Node B dashd -regtest -datadir="${TESTDIR}/2" -port=12350 -connect=127.0.0.1:12348 -rpcport=12349 -server=1 -txindex=1 -printtoconsole=0 & PID2=$!
🧹 Nitpick comments (2)
rpc-integration-test/run.sh (2)
3-3: Use a unique temp dir and quote expansions.Hard-coding
/tmp/rust_dashcore_rpc_testrisks collisions across runs/CI shards. Prefermktemp -dand quote${TESTDIR}where used.Example (outside selected lines):
TESTDIR="$(mktemp -d -t rust_dashcore_rpc_test.XXXXXX)" mkdir -p "${TESTDIR}/1" "${TESTDIR}/2"
33-35: Replace fixed sleeps with RPC readiness checks.Polling for readiness is more robust than
sleep. Usedash-cli -rpcwaitif available, or a small curl loop againstgetblockcount.Example (outside selected lines):
# Wait for RPC to be ready dash-cli -regtest -datadir="${TESTDIR}/2" -rpcwait getblockcount >/dev/null 2>&1 || true # Fallback if dash-cli isn’t present: # until curl -s --fail --user "$(cat "${TESTDIR}/2/regtest/.cookie")" \ # --data-binary '{"jsonrpc":"1.0","id":"ping","method":"getblockcount","params":[]}' \ # http://127.0.0.1:12349 >/dev/null; do sleep 0.5; done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
rpc-integration-test/run.sh(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/integration_real_node_test.rs : Integration tests that connect to a live Dash Core node must gracefully skip when the node at 127.0.0.1:9999 is unavailable
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Applied to files:
rpc-integration-test/run.sh
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
rpc-integration-test/run.sh
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/integration_real_node_test.rs : Integration tests that connect to a live Dash Core node must gracefully skip when the node at 127.0.0.1:9999 is unavailable
Applied to files:
rpc-integration-test/run.sh
📚 Learning: 2025-09-01T14:44:53.683Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Applied to files:
rpc-integration-test/run.sh
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Applied to files:
rpc-integration-test/run.sh
⏰ 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). (15)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: Clippy (Non-strict)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: SPV Components Tests
| # To kill any remaining open dashd. | ||
| killall -9 dashd |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid killall -9 dashd; scope cleanup to this script.
killall -9 may nuke a developer’s local node. Remove this and rely on PID-scoped cleanup with TERM + forced KILL fallback in a trap.
-# To kill any remaining open dashd.
-killall -9 dashdAdd a trap after spawning processes (outside selected lines):
cleanup() {
# Try graceful shutdown first
kill -TERM ${PID2:-} ${PID1:-} 2>/dev/null || true
wait ${PID2:-} ${PID1:-} 2>/dev/null || true
}
trap cleanup EXIT INT TERM🤖 Prompt for AI Agents
In rpc-integration-test/run.sh around lines 8 to 9, remove the unconditional
global killall -9 dashd and instead implement PID-scoped cleanup: after you
spawn background processes capture their PIDs, add a trap for EXIT/INT/TERM that
first attempts a graceful kill -TERM on those PIDs, waits for them, and if they
do not exit escalate to kill -9 as a fallback; place the trap setup after the
processes are spawned so it can reference the actual PID variables and suppress
errors on missing PIDs.
Updated the integration test script to support Dash instead of Bitcoin. The changes include renaming variables and commands to reflect the Dash environment, such as changing
bitcoindtodashdand updating the test directory name. This ensures that the integration tests are correctly set up for testing Dash-related functionalities.Summary by CodeRabbit