Skip to content

test: behavioral host-agent install poll + bats resolver/preflight + pre-commit shellcheck#1102

Draft
yasinBursali wants to merge 1 commit intoLight-Heart-Labs:mainfrom
yasinBursali:test/bats-coverage-batch
Draft

test: behavioral host-agent install poll + bats resolver/preflight + pre-commit shellcheck#1102
yasinBursali wants to merge 1 commit intoLight-Heart-Labs:mainfrom
yasinBursali:test/bats-coverage-batch

Conversation

@yasinBursali
Copy link
Copy Markdown
Contributor

What

Adds BATS + pytest test coverage and pre-commit shellcheck guards across four areas:

  • Behavioral pytest coverage for the host-agent's install state-poll loop.
  • Pre-commit shellcheck hooks (errors-only + SC2006-only) plus a custom heredoc-backtick guard for installer scripts.
  • BATS coverage for the macOS Docker Desktop sharing probe in preflight.
  • BATS coverage for the resolver's user-extension loop (5 scenarios + GPU filter edge cases).

Why

  • Existing TestInstallRunningStateVerification is 100% source-inspection (asserts substrings appear in the function body). A refactor preserving substrings while breaking runtime semantics — wrong terminal status, missing error-progress, swallowed timeouts — would have shipped silently.
  • .pre-commit-config.yaml had no shellcheck hook; CI's lint-shell.yml only fails on --severity=error, so SC2006 (legacy backtick command-substitution) was silently allowed. SC2006 inside an unquoted heredoc body is doubly dangerous because shellcheck doesn't lint heredoc bodies at all — needed both a top-level SC2006 catch and a heredoc-specific guard.
  • test_docker_desktop_sharing() in installers/macos/lib/preflight-fs.sh had no test; a regression that flipped DOCKER_SHARE_OK=true on a bind-mount denial would silently pass preflight.
  • resolve-compose-stack.sh's user-extension loop had no fixture-based tests; the apple-deadlock guard, GPU-backend filter, and overlay-discovery edge cases would all have been hand-verified at most.

How

Added files:

  • dream-server/tests/bats-tests/preflight-docker-desktop.bats — 6 cases. Stubs docker via BATS_TEST_TMPDIR/bin/docker (chmod +x) and prepends to PATH. Covers Mounts-denied, not-shared-from-host, success, missing-docker-CLI, mount succeeds despite warning, and unrelated-error must-not-flip-OK.
  • dream-server/tests/bats-tests/resolver-user-ext.bats — 11 cases combining the GPU-filter edge cases and the 5 user-ext loop scenarios (AMD-only excluded on NVIDIA, legacy-no-manifest included, local-mode overlay handling, apple-deadlock guard, multigpu overlay at gpu-count > 1, plus gpu_backends=[all]/[none] and overlay discovery).
  • dream-server/scripts/check-heredoc-backticks.awk — POSIX-awk state machine that flags backticks inside unquoted heredoc bodies. Wired through pre-commit as a language: system local hook.

Modified:

  • .pre-commit-config.yaml — adds 3 hooks: shellcheck --severity=error (matches CI's failing pass), shellcheck --include=SC2006 (catches legacy backticks regardless of base severity), and the local heredoc-backtick guard.
  • dream-server/extensions/services/dashboard-api/tests/test_host_agent.py — appended TestInstallStatePollBehavior class with 4 wire-tests. Uses types.SimpleNamespace time stub on _mod.time so the no-op'd time.sleep doesn't break the test helper's own time.sleep (helper imports import time as _real_time to bypass).

Deferred (collision with another in-flight branch):

  • tests/bats-tests/preflight-fs-detection.bats (FS personality detection — nfs/cifs/exfat) is already shipping in another in-flight PR with substantially the same scope. Dropping rather than duplicating.

Testing

  • pytest tests/test_host_agent.py::TestInstallStatePollBehavior -v — 4/4 pass.
  • bash tests/run-bats.sh tests/bats-tests/preflight-docker-desktop.bats — 6/6 pass.
  • bash tests/run-bats.sh tests/bats-tests/resolver-user-ext.bats — 7 pass / 4 fail-as-expected (the four cases that depend on the resolver-python-hygiene branch's gpu_backends filter and apple-guard expansion).
  • pre-commit run --all-files — 3 new hooks pass cleanly on the existing tree (zero SC2006 violations, zero heredoc-backtick violations).

Review

Critique Guardian: APPROVED WITH WARNINGS (DRAFT note + deferred deliverable note required in description — both included here, no code blockers).

Known Considerations

  • DRAFT until the resolver-python-hygiene branch lands: 4 of the 11 resolver tests fail on current upstream/main because they reference resolver behavior that's added by that branch (the gpu_backends filter and the apple-guard on compose.local.yaml). Mechanical safeguard.
  • preflight-fs-detection.bats is intentionally deferred — another in-flight branch ships exactly that file with 7 cases for nfs/cifs/exfat/ext2-3/apfs/smbfs on both BSD and GNU stat. Adding our own would duplicate work.
  • SC2006 severity calibration — SC2006 is style-level, not warning-level, so --severity=warning would NOT catch it. The hook uses --include=SC2006 to catch the symbol regardless of base severity.
  • Test 8 (apple-deadlock guard) passes vacuously on current upstream/main because the resolver doesn't yet discover compose.local.yaml on the user-ext loop at all; once that discovery lands (alongside the apple guard) in the prerequisite branch, the test becomes a real regression shield.
  • _make_docker_stub uses an unquoted heredoc — $message is shell-expanded into the stub at write time. Inline note in the file flags the constraint; current test inputs are all literal strings.

Platform Impact

  • macOS (BSD userland, Apple Silicon): BATS preflight-docker-desktop.bats and resolver-user-ext.bats run identically; PATH-stub mocks (docker, stat) are POSIX-shell. check-heredoc-backticks.awk uses POSIX awk only — no GNU-isms (gensub, \b).
  • Linux (GNU userland): same as macOS for these tests.
  • Windows-WSL2 (Linux bash inside WSL): same — pytest and BATS run identically in WSL.

…pre-commit shellcheck

- TestInstallStatePollBehavior (test_host_agent.py): four wire-tests
  for the install state-poll loop — error progress on never-running,
  startup_check=false skipping inspect, started on starting->running
  transition, and TimeoutExpired tolerance. Uses types.SimpleNamespace
  to stub _mod.time so the test helper's own time.sleep stays real
  (helper imports `import time as _real_time` to bypass the stub).
- preflight-docker-desktop.bats: six cases stubbing docker via
  $BATS_TEST_TMPDIR/bin/docker for Mounts-denied, not-shared, success,
  missing-CLI, warning-allowed, and unrelated-error preservation paths.
- resolver-user-ext.bats: eleven cases for the user-ext loop scenarios
  and gpu_backends edge cases. Four cases are fail-as-expected pending
  the resolver-python-hygiene branch (gpu_backends filter + apple guard
  on compose.local.yaml). Test 8 inline note explains the vacuous-pass
  mechanic.
- .pre-commit-config.yaml: shellcheck error-only (matches CI), SC2006
  catch (--include=SC2006 since SC2006 is style-level, not warning),
  and a local heredoc-backtick guard backed by check-heredoc-backticks.awk.
- check-heredoc-backticks.awk: POSIX-awk state machine for the guard.

Skips tests/bats-tests/preflight-fs-detection.bats because another
in-flight branch ships an equivalent preflight-fs-networked.bats with
the same scope.

PR is DRAFT until the resolver-python-hygiene branch merges; the four
fail-as-expected resolver cases are the mechanical safeguard.
@Lightheartdevs
Copy link
Copy Markdown
Collaborator

Merge-pass recheck: holding this as draft/red.

The test coverage is useful, but the PR body explicitly says 4 resolver tests fail until the resolver prerequisite lands, and the live integration-smoke check is currently failing. Please rebase after the resolver branch is in, make the full test set pass against current main, and then undraft. I am not treating fail-as-expected tests as mergeable for the backlog cleanup pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants