fix(#80,#103): rewrite test_orchestrator vs modular layout + register in CI#131
Merged
Conversation
… register in CI #80 core (prologue guard, pid_max fixtures, summary line) already landed; this finishes it and does the #103 rewrite: - Retarget stale structural asserts to the LOADED symbols (they moved into modules/orchestrator.sh, so grepping minecraftSplitscreen.sh always failed): T6.2 -> declare -p _WATCHDOG_PID; T6.6 -> declare -f cleanup | grep; T6.7 -> declare -f main (+ keep the launcher BASH_SOURCE guard grep). - Retarget behavioral tests to the function that OWNS the behavior: SLOT_DIED (T6.3) and DISPLAY_MODE_CHANGE (T6.5) are driven through _handle_msg directly — deterministic, no acquisition/loop/timing races. T6.5 also corrected: the old test asserted a handheld->docked path the modular orchestrator doesn't implement; it now tests the real docked->handheld switch (tear down non-P1, return 1). - Fix the persistent-stderr bug: exec 9>&- 2>/dev/null permanently silenced the suite's stderr from the first test on -> exec 9>&-. - Kill-scoping: every test that runs a real flow now mocks teardown_instance + start_watchdog so no real process-GROUP kill path ever sees suite state; state JSON PIDs derived from the FIXTURE_* constants the pid_max guard checks. - Speed/determinism: pre-set the guarded ORCHESTRATOR_* constants before sourcing (acquisition always burns the full timeout) + neutralize env-touching helpers. - Register tests/test_orchestrator.sh=8 in the CI baseline map. Validated in CI (isolated); Deck smoke at HW-2. Never run raw in an un-namespaced dev container. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hm7DmagTa2EttH8aHxGji2
The first CI run wedged on 'Run CI-safe test suites' — a real flow driven by a test failed to exit and the bare `wait $pid` blocked forever. A unit suite must never hang: add _wait_bounded (poll up to N s, then SIGKILL + return 124) and use it for every wait on a backgrounded flow/handler (T6.3, T6.4, T6.5, T6.8). A flow that misbehaves now FAILS visibly instead of hanging the whole job. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hm7DmagTa2EttH8aHxGji2
… capture pipe Root cause of the CI hang (not the bounded waits): CI runs `out=$(bash suite 2>&1)`, and $(...) blocks until EVERY process holding the stdout pipe exits. T6.8's docked_flow starts the REAL start_watchdog as a grandchild; killing the docked_flow subshell orphans it (SIGKILL doesn't propagate), and it loops forever holding the inherited pipe fd -> $(...) never returns. Proven: an unredirected bg child makes $(...) wait for it; a redirected one returns immediately. Fix: - Mock start_watchdog INSIDE T6.8's docked_flow subshell so it doesn't spawn an orphanable real watchdog (the test runs its own real watchdog separately as wd_pid). - Redirect every backgrounded flow subshell '>/dev/null 2>&1' so no flow descendant can ever hold the capture pipe (belt-and-suspenders across T6.3/T6.4/T6.5/T6.8). Flows communicate via sentinels/state files/the FIFO, not stdout, so the redirect loses nothing the assertions need. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hm7DmagTa2EttH8aHxGji2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the pre-HW-2 test-harness batch. CI-gated — the suite is validated in the isolated GitHub Actions runner (the safe place for a suite whose failure mode is escaped process-group kills), plus a Deck smoke at HW-2. Never run raw in an un-namespaced dev container (kill hazard).
Background
#80's core already landed (prologue guard, pid_max fixtures,
N/M tests passed.summary). What remained: the persistent-stderr bug + CI registration (#80), and the #103 rewrite against the modular layout.Changes
minecraftSplitscreen.shfor_WATCHDOG_PID/cleanup()/main(), which moved intomodules/orchestrator.sh→ always failed. Now assert the loaded definitions (declare -p/declare -f); keep theBASH_SOURCEguard grep on the launcher entry._handle_msgdirectly — deterministic, no acquisition/loop/timing races. T6.5 corrected: the old test asserted a handheld→docked path the modular orchestrator doesn't implement; it now tests the real docked→handheld switch (tear down non-P1, return 1).exec 9>&- 2>/dev/nullsilenced the whole suite's stderr from the first test on) →exec 9>&-.teardown_instance+start_watchdog; state-JSON PIDs derived from theFIXTURE_*constants the pid_max guard checks.ORCHESTRATOR_*constants before sourcing (acquisition always burns the full timeout) + neutralize env-touching helpers.[tests/test_orchestrator.sh]=8added to the baseline map.Closes #80 and #103 (on CI-green merge; Deck smoke confirms kill-scoping on a real session).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Hm7DmagTa2EttH8aHxGji2