-
Notifications
You must be signed in to change notification settings - Fork 0
test(docker): add multi-validator test infrastructure and fix healthchecks #7
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
Add comprehensive test suite and Docker configuration for testing multi-validator P2P networks locally: - docker-compose.multi-validator.yml for 4-validator test network - Dockerfile.test-validator for lightweight test builds - test-comprehensive.sh for running full test suite - entrypoint-test.sh for configuring test validators
📝 WalkthroughWalkthroughThese changes introduce comprehensive testing infrastructure for a multi-validator test network. Minor healthcheck updates adjust file existence checks, while new Docker configurations define a test validator image and multi-node network topology. A new comprehensive test script orchestrates seven sequential test phases across the Rust workspace. Changes
Sequence DiagramsequenceDiagram
participant Build as Build System
participant UnitTests as Unit Tests
participant DockerInt as Docker Integration
participant Bittensor as Bittensor Tests
participant Security as Security Tests
participant P2P as P2P Tests
participant Storage as Storage Tests
participant Summary as Summary Report
Build->>Build: cargo build --release
Build-->>UnitTests: ✓ Build Success
UnitTests->>UnitTests: cargo test --workspace --release
UnitTests-->>DockerInt: ✓ Unit Tests Complete
DockerInt->>DockerInt: Check Docker daemon
DockerInt->>DockerInt: cargo test secure-container-runtime
DockerInt->>DockerInt: cargo test challenge-orchestrator
DockerInt-->>Bittensor: ✓ Docker Tests Complete
Bittensor->>Bittensor: cargo test platform-bittensor (120s timeout)
Bittensor-->>Security: ✓ Bittensor Tests Complete
Security->>Security: test_default_policy_blocks_docker_socket
Security->>Security: test_strict_policy_blocks_non_whitelisted_images
Security->>Security: test_policy_enforces_resource_limits
Security-->>P2P: ✓ Security Tests Complete
P2P->>P2P: cargo test platform-p2p-consensus
P2P-->>Storage: ✓ P2P Tests Complete
Storage->>Storage: cargo test platform-storage
Storage->>Storage: cargo test platform-distributed-storage
Storage-->>Summary: ✓ Storage Tests Complete
Summary->>Summary: Calculate pass/fail/skip counts
Summary->>Summary: Print colored summary
Summary-->>Storage: Exit with status code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 `@scripts/test-comprehensive.sh`:
- Around line 17-18: The script currently uses "set -e" but misses "set -o
pipefail", so failures in piped commands like the "cargo test ... | tee ..."
pipelines (used around the cargo test checks) can be masked by tee's success;
add "set -o pipefail" immediately after the existing "set -e" in
scripts/test-comprehensive.sh so that pipeline failures (e.g., in the cargo test
| tee pipelines) correctly propagate non-zero exit codes to the script's
conditionals and cause test failures to be detected.
In `@tests/docker/entrypoint-test.sh`:
- Around line 10-35: The script builds command args into a single string
variable ARGS and then calls exec validator-node $ARGS which causes
word-splitting, glob expansion and drops any extra script args; change to use a
bash array (e.g., args_array) and append elements (like "--data-dir",
"${DATA_DIR:-/data}", "--listen-addr",
"${P2P_LISTEN_ADDR:-/ip4/0.0.0.0/tcp/9000}", conditional pushes for SECRET_KEY,
NETUID, each bootstrap peer, and "--no-bittensor") and finally append "$@"
before calling exec validator-node "${args_array[@]}" so arguments are preserved
and not split by the shell.
| set -e | ||
|
|
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.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and examine its structure
git ls-files | grep -E "scripts/test-comprehensive\.sh"Repository: PlatformNetwork/platform-v2
Length of output: 100
🏁 Script executed:
# Read the file to verify the lines mentioned
wc -l scripts/test-comprehensive.shRepository: PlatformNetwork/platform-v2
Length of output: 104
🏁 Script executed:
# Read the relevant sections of the file
cat -n scripts/test-comprehensive.sh | sed -n '1,30p'Repository: PlatformNetwork/platform-v2
Length of output: 1038
🏁 Script executed:
# Check the pipeline sections at the mentioned lines
cat -n scripts/test-comprehensive.sh | sed -n '85,95p'Repository: PlatformNetwork/platform-v2
Length of output: 592
🏁 Script executed:
# Check around line 109
cat -n scripts/test-comprehensive.sh | sed -n '105,115p'Repository: PlatformNetwork/platform-v2
Length of output: 597
🏁 Script executed:
# Check around line 184
cat -n scripts/test-comprehensive.sh | sed -n '180,190p'Repository: PlatformNetwork/platform-v2
Length of output: 580
🏁 Script executed:
# Search for pipefail in the file
grep -n "pipefail" scripts/test-comprehensive.sh || echo "No pipefail found"Repository: PlatformNetwork/platform-v2
Length of output: 88
Enable pipefail to prevent masked test failures in piped commands.
The pipelines at lines 88, 109, and 184 use cargo test ... | tee ... in if conditions without set -o pipefail. This causes test failures to be masked since the exit code comes from tee (which succeeds) rather than cargo test, allowing failed tests to incorrectly pass the conditional checks.
🔧 Proposed fix
set -e
+set -o pipefailAdd set -o pipefail after line 17 to properly propagate exit codes through pipelines.
📝 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.
| set -e | |
| set -e | |
| set -o pipefail | |
🤖 Prompt for AI Agents
In `@scripts/test-comprehensive.sh` around lines 17 - 18, The script currently
uses "set -e" but misses "set -o pipefail", so failures in piped commands like
the "cargo test ... | tee ..." pipelines (used around the cargo test checks) can
be masked by tee's success; add "set -o pipefail" immediately after the existing
"set -e" in scripts/test-comprehensive.sh so that pipeline failures (e.g., in
the cargo test | tee pipelines) correctly propagate non-zero exit codes to the
script's conditionals and cause test failures to be detected.
| # Build command arguments | ||
| ARGS="--data-dir ${DATA_DIR:-/data}" | ||
| ARGS="$ARGS --listen-addr ${P2P_LISTEN_ADDR:-/ip4/0.0.0.0/tcp/9000}" | ||
|
|
||
| if [ -n "$VALIDATOR_SECRET_KEY" ]; then | ||
| ARGS="$ARGS --secret-key $VALIDATOR_SECRET_KEY" | ||
| fi | ||
|
|
||
| if [ -n "$NETUID" ]; then | ||
| ARGS="$ARGS --netuid $NETUID" | ||
| fi | ||
|
|
||
| if [ -n "$BOOTSTRAP_PEERS" ]; then | ||
| # Split by comma and add each peer | ||
| IFS=',' read -ra PEERS <<< "$BOOTSTRAP_PEERS" | ||
| for peer in "${PEERS[@]}"; do | ||
| ARGS="$ARGS --bootstrap $peer" | ||
| done | ||
| fi | ||
|
|
||
| if [ "$NO_BITTENSOR" = "true" ]; then | ||
| ARGS="$ARGS --no-bittensor" | ||
| fi | ||
|
|
||
| echo "Starting validator-node with args: $ARGS" | ||
| exec validator-node $ARGS |
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.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file
fd -type f -name "entrypoint-test.sh" 2>/dev/null | head -20Repository: PlatformNetwork/platform-v2
Length of output: 53
🏁 Script executed:
# Check if the file exists and examine the relevant lines
if [ -f "tests/docker/entrypoint-test.sh" ]; then
cat -n "tests/docker/entrypoint-test.sh" | head -40
fiRepository: PlatformNetwork/platform-v2
Length of output: 1326
🏁 Script executed:
# Also check the shebang and structure
head -5 "tests/docker/entrypoint-test.sh" 2>/dev/null || echo "File not found"Repository: PlatformNetwork/platform-v2
Length of output: 341
Use array-based argument handling to prevent word-splitting and preserve extra args.
Lines 11–35 build ARGS as a string and execute it unquoted on line 35 (exec validator-node $ARGS). This causes word-splitting and pathname expansion, and ignores $@, dropping any extra arguments passed to the script. Use an array and append "$@":
🔧 Proposed fix
-ARGS="--data-dir ${DATA_DIR:-/data}"
-ARGS="$ARGS --listen-addr ${P2P_LISTEN_ADDR:-/ip4/0.0.0.0/tcp/9000}"
+ARGS=(--data-dir "${DATA_DIR:-/data}" --listen-addr "${P2P_LISTEN_ADDR:-/ip4/0.0.0.0/tcp/9000}")
if [ -n "$VALIDATOR_SECRET_KEY" ]; then
- ARGS="$ARGS --secret-key $VALIDATOR_SECRET_KEY"
+ ARGS+=(--secret-key "$VALIDATOR_SECRET_KEY")
fi
if [ -n "$NETUID" ]; then
- ARGS="$ARGS --netuid $NETUID"
+ ARGS+=(--netuid "$NETUID")
fi
if [ -n "$BOOTSTRAP_PEERS" ]; then
# Split by comma and add each peer
IFS=',' read -ra PEERS <<< "$BOOTSTRAP_PEERS"
for peer in "${PEERS[@]}"; do
- ARGS="$ARGS --bootstrap $peer"
+ ARGS+=(--bootstrap "$peer")
done
fi
if [ "$NO_BITTENSOR" = "true" ]; then
- ARGS="$ARGS --no-bittensor"
+ ARGS+=(--no-bittensor)
fi
echo "Starting validator-node with args: $ARGS"
-exec validator-node $ARGS
+exec validator-node "${ARGS[@]}" "$@"📝 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.
| # Build command arguments | |
| ARGS="--data-dir ${DATA_DIR:-/data}" | |
| ARGS="$ARGS --listen-addr ${P2P_LISTEN_ADDR:-/ip4/0.0.0.0/tcp/9000}" | |
| if [ -n "$VALIDATOR_SECRET_KEY" ]; then | |
| ARGS="$ARGS --secret-key $VALIDATOR_SECRET_KEY" | |
| fi | |
| if [ -n "$NETUID" ]; then | |
| ARGS="$ARGS --netuid $NETUID" | |
| fi | |
| if [ -n "$BOOTSTRAP_PEERS" ]; then | |
| # Split by comma and add each peer | |
| IFS=',' read -ra PEERS <<< "$BOOTSTRAP_PEERS" | |
| for peer in "${PEERS[@]}"; do | |
| ARGS="$ARGS --bootstrap $peer" | |
| done | |
| fi | |
| if [ "$NO_BITTENSOR" = "true" ]; then | |
| ARGS="$ARGS --no-bittensor" | |
| fi | |
| echo "Starting validator-node with args: $ARGS" | |
| exec validator-node $ARGS | |
| # Build command arguments | |
| ARGS=(--data-dir "${DATA_DIR:-/data}" --listen-addr "${P2P_LISTEN_ADDR:-/ip4/0.0.0.0/tcp/9000}") | |
| if [ -n "$VALIDATOR_SECRET_KEY" ]; then | |
| ARGS+=(--secret-key "$VALIDATOR_SECRET_KEY") | |
| fi | |
| if [ -n "$NETUID" ]; then | |
| ARGS+=(--netuid "$NETUID") | |
| fi | |
| if [ -n "$BOOTSTRAP_PEERS" ]; then | |
| # Split by comma and add each peer | |
| IFS=',' read -ra PEERS <<< "$BOOTSTRAP_PEERS" | |
| for peer in "${PEERS[@]}"; do | |
| ARGS+=(--bootstrap "$peer") | |
| done | |
| fi | |
| if [ "$NO_BITTENSOR" = "true" ]; then | |
| ARGS+=(--no-bittensor) | |
| fi | |
| echo "Starting validator-node with args: $ARGS" | |
| exec validator-node "${ARGS[@]}" "$@" |
🤖 Prompt for AI Agents
In `@tests/docker/entrypoint-test.sh` around lines 10 - 35, The script builds
command args into a single string variable ARGS and then calls exec
validator-node $ARGS which causes word-splitting, glob expansion and drops any
extra script args; change to use a bash array (e.g., args_array) and append
elements (like "--data-dir", "${DATA_DIR:-/data}", "--listen-addr",
"${P2P_LISTEN_ADDR:-/ip4/0.0.0.0/tcp/9000}", conditional pushes for SECRET_KEY,
NETUID, each bootstrap peer, and "--no-bittensor") and finally append "$@"
before calling exec validator-node "${args_array[@]}" so arguments are preserved
and not split by the shell.
Summary
Add comprehensive test suite and Docker configuration for testing multi-validator P2P networks locally.
Changes
Bug Fixes
test -einstead oftest -fsincedistributed.dbis a directory (sled database), not a fileNew Features
docker-compose.multi-validator.ymlfor spinning up a 4-validator test networkDockerfile.test-validatorfor lightweight test buildstest-comprehensive.shscript for running the full test suiteentrypoint-test.shfor configuring test validatorsTesting
All tests pass:
Local Testing Instructions
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores