test: add comprehensive Docker Compose stack validation#375
test: add comprehensive Docker Compose stack validation#375bugman-007 wants to merge 2 commits intoLight-Heart-Labs:mainfrom
Conversation
Lightheartdevs
left a comment
There was a problem hiding this comment.
Review: test/compose-stack-validation
Good motivation -- closing the gap between what CI validates vs. what actually runs is valuable. The test suite is behavioral (runs real commands, checks exit codes), not just static grep checks. A few issues need fixing before merge.
1. CLAUDE.md violations: 2>/dev/null, || true, &>/dev/null
CLAUDE.md is explicit: "Never || true or 2>/dev/null."
.github/workflows/validate-compose.yml (new "Validate extension compose files" step):
ext_files=$(find extensions/services -name "compose.yaml" -type f 2>/dev/null || true)
This silences find errors AND swallows failures. If extensions/services/ is missing, this silently produces an empty list instead of crashing visibly. Use find ... || warn "find failed (non-fatal)", or just let it crash -- if the directory doesn't exist in CI, that's worth knowing.
dream-server/tests/test-validate-compose-stack.sh:
- Line 59:
command -v docker &>/dev/null-- suppresses all output including stderr. - Line 60:
docker compose version &>/dev/null 2>&1-- double stderr redirect (&>already includes stderr, then2>&1is redundant). Same suppression concern. - Line 162:
find extensions/services -name "compose.yaml" -o -name "compose.*.yaml" 2>/dev/null | wc -l-- silences find errors.
Fix: Replace 2>/dev/null with explicit handling. For capability checks (command -v), redirecting just stdout to /dev/null while letting stderr through is acceptable.
2. CI paths filter misses extension compose files
.github/workflows/validate-compose.yml triggers on:
paths:
- "dream-server/docker-compose*.yml"
- "dream-server/compose/**"But the new "Validate extension compose files" step validates extensions/services/*/compose.yaml. Changes to those files won't trigger this workflow, so the new validation step will never catch regressions in extension compose files. Add "dream-server/extensions/services/**/compose*.yaml" to the paths filter.
3. Merge conflict with PR #376
Both this PR and #376 (compose healthcheck audit) add new steps to .github/workflows/test-linux.yml at the same insertion point (after "Health Check Tests"). Whichever merges second will need a rebase. Coordinate merge order.
4. Minor: set +e / set -e toggling
The test file toggles set +e and set -e around every command whose exit code is checked (lines 93-96, 106-109, 119-122, etc.). This is functional but fragile -- if someone adds code inside a set +e block later, failures would be silently swallowed. Consider capturing exit codes inline instead:
resolve_exit=0
bash scripts/resolve-compose-stack.sh ... || resolve_exit=$?This keeps set -e active throughout and is the idiomatic CLAUDE.md-compliant approach ("If you must tolerate a failure, log it: some_command || warn "failed (non-fatal)"").
Summary
The test coverage itself is solid and well-structured. Main blockers are the 2>/dev/null || true violations (item 1) and the paths filter gap (item 2) which means the extension validation would effectively never run on relevant changes. Items 3-4 are lower severity but worth addressing.
|
What's needed to get this merged:
Tests themselves are actually behavioral (not grep-based) which is good. Just needs the error handling cleaned up. |
|
I addressed review feedback for compose stack validation |
Summary
tests/test-validate-compose-stack.shwith 13 test casesresolve-compose-stack.shoutput for all GPU backendsMotivation
Current CI only validates single compose files, not the layered stacks that actually run in production:
base.yml+ GPU overlay + extension compose filesvalidate-compose-stack.shexists but has no tests and isn't used in CIresolve-compose-stack.shdynamically builds stacks but output isn't validatedThis PR closes that gap by testing the actual runtime architecture.
Test Coverage
The new test suite validates:
validate-compose-stack.sh,resolve-compose-stack.sh)resolve-compose-stack.shworks for all backends (nvidia, amd, apple)validate-compose-stack.shaccepts valid flagsvalidate-compose-stack.shrejects missing--compose-flagsCI Enhancements
test-linux.yml
Added "Compose Stack Validation Tests" step to run the test suite on every PR.
validate-compose.yml
Enhanced to validate:
extensions/services/*/compose*.yamldream-server/compose/Test Results (Local)
$ bash tests/test-validate-compose-stack.sh ╔═══════════════════════════════════════════════╗ ║ Compose Stack Validation Test Suite ║ ╚═══════════════════════════════════════════════╝ ✓ PASS validate-compose-stack.sh exists ✓ PASS resolve-compose-stack.sh exists ⊘ SKIP docker not available - skipping validation tests Result: 2 passed, 0 failed (docker required for full tests)Note: Full validation requires Docker, which will run in CI.
Impact
resolve-compose-stack.shfor all GPU backendsRelated