-
Notifications
You must be signed in to change notification settings - Fork 0
test(docker): add local multi-validator test script #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adds a shell script for testing multiple validators locally without Docker containers. This enables quick validation of P2P network formation and validator initialization.
📝 WalkthroughWalkthroughA new Bash integration test script that orchestrates a multi-validator P2P network. It builds the validator binary, starts a bootstrap validator, launches additional validators with distinct keys and ports, waits for initialization, verifies all validators are running, checks for distributed.db files, and validates P2P connections and peer discovery in logs. Changes
Sequence DiagramsequenceDiagram
participant Script as Test Script
participant Binary as Validator Binary
participant Bootstrap as Bootstrap Validator
participant Others as Additional Validators
participant FileSystem as File System
participant Logs as Log Files
Script->>Script: Check/Build validator binary
Script->>Bootstrap: Start bootstrap validator (port 9100)
activate Bootstrap
Script->>FileSystem: Create test directory
Loop i = 1 to NUM_VALIDATORS-1
Script->>Others: Start validator with unique key/port
activate Others
end
Script->>Script: Wait for initialization
Script->>Bootstrap: Verify process running
Script->>Others: Verify all processes running
Script->>FileSystem: Check distributed.db exists
Bootstrap->>Logs: Write initialization logs
Others->>Logs: Write initialization logs
Others->>Logs: Write P2P peer connection logs
Script->>Logs: Scan for P2P initialization
Script->>Logs: Scan for peer connections
Script->>Script: Run network (stabilization period)
Script->>Script: Count healthy validators
Script->>Script: Return status (0=all healthy, 1=failures)
deactivate Others
deactivate Bootstrap
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tests/docker/test-multi-validator.sh`:
- Around line 131-136: The test is checking for a file but uses the directory
test operator (-d), causing false negatives; change the check that evaluates
DB_FILE (the variable set as DB_FILE="$TEST_DIR/validator-$i/distributed.db")
from -d to -f (or -e if both file/dir are possible) and update the conditional
that calls log_success/log_warning accordingly; apply the same replacement for
the similar check around lines that reference the same DB_FILE logic later in
the script (the block that also uses log_success/log_warning for validators).
- Around line 86-90: The validator key construction in the loop yields
non‑padded hex for two‑digit indexes; update the VALIDATOR_KEY assignment inside
the for loop (where NUM_VALIDATORS and VALIDATOR_KEY are used) to generate a
64‑hex‑char, zero‑padded value (and prefix with 0x) by formatting the numeric
index with printf to produce a 64‑character hex string (e.g., use printf "%064x"
on the index) instead of concatenating the index to a fixed short prefix.
🧹 Nitpick comments (1)
tests/docker/test-multi-validator.sh (1)
74-110: Replace fixed sleeps with bounded polling to reduce flakiness.Fixed sleeps can make the test flaky across slower machines. Consider a retry loop with timeout (e.g., wait for log markers or DB file existence) to make readiness checks deterministic.
♻️ Example pattern (optional)
+wait_for_log() { + local file="$1" pattern="$2" timeout="${3:-20}" + for ((t=0; t<timeout; t++)); do + grep -q "$pattern" "$file" 2>/dev/null && return 0 + sleep 1 + done + return 1 +}Also applies to: 155-157
| for i in $(seq 2 $NUM_VALIDATORS); do | ||
| VALIDATOR_DIR="$TEST_DIR/validator-$i" | ||
| mkdir -p "$VALIDATOR_DIR" | ||
| VALIDATOR_KEY="0x000000000000000000000000000000000000000000000000000000000000000$i" | ||
| VALIDATOR_PORT=$((BASE_PORT + i - 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validator key formatting breaks for NUM_VALIDATORS ≥ 10.
Appending $i to a fixed hex prefix yields non‑padded hex (and non‑64‑byte) keys for two‑digit indexes, which can make keys invalid. Use a zero‑padded 64‑hex format.
🛠️ Suggested fix
- VALIDATOR_KEY="0x000000000000000000000000000000000000000000000000000000000000000$i"
+ VALIDATOR_KEY=$(printf '0x%064x' "$i")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for i in $(seq 2 $NUM_VALIDATORS); do | |
| VALIDATOR_DIR="$TEST_DIR/validator-$i" | |
| mkdir -p "$VALIDATOR_DIR" | |
| VALIDATOR_KEY="0x000000000000000000000000000000000000000000000000000000000000000$i" | |
| VALIDATOR_PORT=$((BASE_PORT + i - 1)) | |
| for i in $(seq 2 $NUM_VALIDATORS); do | |
| VALIDATOR_DIR="$TEST_DIR/validator-$i" | |
| mkdir -p "$VALIDATOR_DIR" | |
| VALIDATOR_KEY=$(printf '0x%064x' "$i") | |
| VALIDATOR_PORT=$((BASE_PORT + i - 1)) |
🤖 Prompt for AI Agents
In `@tests/docker/test-multi-validator.sh` around lines 86 - 90, The validator key
construction in the loop yields non‑padded hex for two‑digit indexes; update the
VALIDATOR_KEY assignment inside the for loop (where NUM_VALIDATORS and
VALIDATOR_KEY are used) to generate a 64‑hex‑char, zero‑padded value (and prefix
with 0x) by formatting the numeric index with printf to produce a 64‑character
hex string (e.g., use printf "%064x" on the index) instead of concatenating the
index to a fixed short prefix.
| DB_FILE="$TEST_DIR/validator-$i/distributed.db" | ||
| if [ -d "$DB_FILE" ]; then | ||
| log_success "Validator $i: distributed.db exists" | ||
| else | ||
| log_warning "Validator $i: distributed.db not found yet" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use -f/-e for distributed.db; -d will mis-detect files.
If distributed.db is a file (as the summary implies), -d will always fail and incorrectly mark validators unhealthy. Switch to -f (or -e if it can be file/dir).
🧩 Suggested fix
- if [ -d "$DB_FILE" ]; then
+ if [ -f "$DB_FILE" ]; then
log_success "Validator $i: distributed.db exists"
else
log_warning "Validator $i: distributed.db not found yet"
fi- if [ -d "$DB_FILE" ]; then
+ if [ -f "$DB_FILE" ]; then
log_success "Validator $i: HEALTHY (running + DB initialized)"
((HEALTHY++))
else
log_warning "Validator $i: RUNNING (no DB yet)"
fiAlso applies to: 168-174
🤖 Prompt for AI Agents
In `@tests/docker/test-multi-validator.sh` around lines 131 - 136, The test is
checking for a file but uses the directory test operator (-d), causing false
negatives; change the check that evaluates DB_FILE (the variable set as
DB_FILE="$TEST_DIR/validator-$i/distributed.db") from -d to -f (or -e if both
file/dir are possible) and update the conditional that calls
log_success/log_warning accordingly; apply the same replacement for the similar
check around lines that reference the same DB_FILE logic later in the script
(the block that also uses log_success/log_warning for validators).
Changes
Testing
Verified the following tests pass:
Local Multi-Validator Test:
Docker Multi-Validator Test:
Unit/Integration Tests:
Verification
Summary by CodeRabbit