Skip to content

fix: correct worker shutdown waits and test portability - #12

Open
cisrd wants to merge 8 commits into
mainfrom
fm/fm-lot-tests-fiables-portabilite
Open

cisrd wants to merge 8 commits into
mainfrom
fm/fm-lot-tests-fiables-portabilite

Conversation

@cisrd

@cisrd cisrd commented Sep 8, 2026 •

Copy link
Copy Markdown
Owner

Intent

Le capitaine demande de lancer encore une ou deux tâches en attente, avec sa préférence durable de regrouper les petites corrections afin d’éviter des pipelines et des PR inutiles. Ce lot rassemble deux petits défauts établis qui rendent des suites de tests trompeuses : une assertion conditionnée à Ruby échoue lorsque Ruby est absent, et une suite entièrement verte sort avec le code 1 parce que son propre nettoyage temporaire échoue.

What Changed

  • Isolate the Herdr CI timeout assertion into a registered test that explicitly skips when Ruby is absent.
  • Correct worker-tree liveness checks so shutdown waits for the process group before escalating to KILL.
  • Stop remote worker trees before public-followup fixture cleanup, with regression coverage for suite exit status, leaked resources, and graceful versus forced shutdown.

Risk Assessment

✅ Low: The changes are bounded, implement the accepted cleanup and Ruby-skip requirements, and introduce no substantiated material defects.

Testing

Verified Ruby execution and visible skip accounting, real worker cleanup, shutdown escalation, and EXIT-status preservation. Locale and PID-1 assumptions blocked initial commands; a locale-adjusted retry and isolated shutdown checks supplied targeted evidence. The shutdown regression failed against the base helper as expected.

Evidence: Runner visibly accounts for absent Ruby

Source: Runner visibly accounts for absent Ruby

FM_TEST_BEGIN 2026-09-08T16:57:36Z tests/fm-ci-herdr-timeout.test.sh family=pure-contract-unit expected_gate_skip=none
skip: ruby absent; YAML assertion not run
fm-test-run: gate skip: tests/fm-ci-herdr-timeout.test.sh: ruby absent; YAML assertion not run
FM_TEST_END 2026-09-08T16:57:36Z tests/fm-ci-herdr-timeout.test.sh exit=0 duration_ms=23 gate_skip=true
FM_TEST_SUMMARY total=1 failed=0 skipped_gate=1 duration_ms=109
FM_TEST_SUMMARY_FAMILY family=pure-contract-unit count=1 duration_ms=23 failed=0
FM_TEST_SLOWEST rank=1 script=tests/fm-ci-herdr-timeout.test.sh duration_ms=23
fm-test-run: wrote timing artifact: ~/.no-mistakes/evidence/01M20Y3NK7YVT8QNY821YRCBBQ/ruby-absent.json
Evidence: Persisted Ruby-absent runner result

Source: Persisted Ruby-absent runner result

{
  "families": [
    {
      "count": 1,
      "duration_ms": 23,
      "failed": 0,
      "name": "pure-contract-unit"
    }
  ],
  "finished_at": "2026-09-08T16:57:36Z",
  "run_id": "fm-test-run-1788886656885-1407881",
  "scripts": [
    {
      "duration_ms": 23,
      "exit": 0,
      "expected_gate_skip": "none",
      "family": "pure-contract-unit",
      "gate_skip": true,
      "gate_skip_reason": "ruby absent; YAML assertion not run",
      "path": "tests/fm-ci-herdr-timeout.test.sh"
    }
  ],
  "selection": "scripts;jobs=1",
  "started_at": "2026-09-08T16:57:36Z",
  "summary": {
    "duration_ms": 109,
    "failed": 0,
    "skipped_gate": 1,
    "total": 1
  }
}
Evidence: Persisted Ruby-present runner result

Source: Persisted Ruby-present runner result

{
  "families": [
    {
      "count": 1,
      "duration_ms": 693,
      "failed": 0,
      "name": "pure-contract-unit"
    }
  ],
  "finished_at": "2026-09-08T16:57:36Z",
  "run_id": "fm-test-run-1788886656042-1405884",
  "scripts": [
    {
      "duration_ms": 693,
      "exit": 0,
      "expected_gate_skip": "none",
      "family": "pure-contract-unit",
      "gate_skip": false,
      "gate_skip_reason": "",
      "path": "tests/fm-ci-herdr-timeout.test.sh"
    }
  ],
  "selection": "scripts;jobs=1",
  "started_at": "2026-09-08T16:57:36Z",
  "summary": {
    "duration_ms": 770,
    "failed": 0,
    "skipped_gate": 0,
    "total": 1
  }
}
Evidence: Real suite exit and immediate process-group cleanup verification

Source: Real suite exit and immediate process-group cleanup verification

ok - remote worker cleanup fixture completed
Command: FM_TEST_ONLY=test_remote_worker_cleanup_fixture bash tests/fm-public-followup.test.sh
Suite exit status: 0
Authoritative worker process group: 1444096
Immediate group probe: absent
Fixture root: removed
Evidence: Cleanup fault-injection exit-status matrix

Source: Cleanup fault-injection exit-status matrix

errexit enabled; assertion exit=0; worker-stop return=0; temp-cleanup return=1; suite exit=0
errexit enabled; assertion exit=37; worker-stop return=0; temp-cleanup return=1; suite exit=37
not ok - the remote job worker survived suite cleanup
errexit enabled; assertion exit=0; worker-stop return=1; temp-cleanup return=1; suite exit=1
not ok - the remote job worker survived suite cleanup
errexit enabled; assertion exit=37; worker-stop return=1; temp-cleanup return=1; suite exit=37

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed ✅
  • ⚠️ tests/fm-test-run.test.sh:1511 - The accepted decision requires: "Move the Ruby-dependent YAML invariant into a focused test executable with the existing top-of-file first-line skip contract ... map selection to the workflow contract." Instead, this hunk still returns a mid-suite skip. Without Ruby, the suite first prints successful assertions, so bin/fm-test-run.sh:1534 does not recognize the later skip and records an unverified YAML contract as non-skipped. Extract the invariant into the requested focused executable, map workflow selection to it, and retain the unrelated tests.
  • ⚠️ tests/fm-public-followup.test.sh:3150 - The accepted decision requires: "Remove the redundant parent grace and assert immediately after child-confirmed cleanup, retaining emergency KILL only after failure." This added loop still waits five seconds before checking the supervisor. That overlaps the worker's five-second abandoned-root self-termination window, allowing a leaked supervisor to disappear after fixture deletion before the regression observes it. Remove this redundant waiting component and capture/assert survival immediately; use emergency KILL only after detecting failure.

🔧 Fix: Expose YAML capability skips and detect cleanup leaks immediately
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-test-run.test.sh encountered the known locale-ordering issue; LC_ALL=C bash tests/fm-test-run.test.sh passed.
  • bash tests/fm-ci-herdr-timeout.test.sh passed.
  • bin/fm-test-run.sh --json <evidence-path> tests/fm-ci-herdr-timeout.test.sh with Ruby present and with PATH=/usr/bin:/bin; verified persisted skip accounting.
  • bash tests/fm-public-followup.test.sh passed.
  • bash tests/fm-remote-job-orphan-reap.test.sh stopped at the existing PID-1 assumption; executed its new child-before-leader and TERM-resistant cases separately using a temporary harness.
  • Executed the focused shutdown regression against the base helper: failed before the fix and passed with the target helper.
  • FM_TEST_ONLY=test_remote_worker_cleanup_fixture bash tests/fm-public-followup.test.sh; immediately verified exit 0, absent worker group, and removed fixture root.
  • Temporary fault-injection harness under bash -e verified benign cleanup failure, survivor failure, and preservation of assertion exit 37. Removed all temporary harness files.
✅ **Document** - passed

✅ No issues found.

🔧 **Lint** - 1 issue found → auto-fixed ✅
  • ⚠️ linter found issues (exit code 1)

🔧 Fix: Quote empty PATH assignment to satisfy ShellCheck
✅ Re-checked - no issues remain.

✅ **Push** - passed

✅ No issues found.

Alex William added 8 commits September 8, 2026 17:04
…ng tests/lib.sh source directive and applied the existing production-module analysis boundary to the orphan-reaper test. Full-analysis pinned ShellCheck passes for both tests and the production library; Bash syntax, Herdr timeout test, and git diff --check also pass. Runtime behavior is unchanged
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.

1 participant