Skip to content

test(tests): stop the test runner from leaking fixture dirs into /tmp - #2

Open
brchue-ux wants to merge 3 commits into
mainfrom
fm/tmp-fixture-leak
Open

test(tests): stop the test runner from leaking fixture dirs into /tmp#2
brchue-ux wants to merge 3 commits into
mainfrom
fm/tmp-fixture-leak

Conversation

@brchue-ux

Copy link
Copy Markdown
Owner

What Changed

  • bin/fm-test-run.sh now owns fixture cleanup end-to-end: it creates a private run root (with EXIT/INT/TERM/HUP/QUIT traps) plus a private TMPDIR/TMP/FM_TASK_TMP_ROOT for each executed script, removing each script's temp root as soon as it finishes so an interrupted run cleans up instead of leaking /tmp fixtures (or RAM, on tmpfs hosts); bin/fm-spawn.sh honors the new FM_TASK_TMP_ROOT override so spawned tasks land inside a test's private root.
  • Added an orphan-fixture reap that runs before each execution: it narrowly targets fm-<prefix>.XXXXXX-shaped directories directly under the reap root, skipping anything not owned by the current user, younger than the minimum age, recorded as a live tasktmp= path across discoverable Firstmate homes/secondmates, or still held open by a process (via /proc or lsof), and is configurable/testable via FM_TEST_REAP_ROOT, FM_TEST_REAP_MIN_AGE_SECONDS, FM_TEST_REAP_MAX, and FM_TEST_REAP_HOMES.
  • Added stop_run_workers/on_run_signal to terminate a signalled --jobs run's worker subshells and their full process trees before the run root is removed, fixed an inert worker-stop guard and a slow batched /proc reap scan, and updated tests/lib.sh, CONTRIBUTING.md, and the touched tests/*.test.sh files (including a new tests/fm-test-run.test.sh contract suite) to match the runner's single-owner cleanup model.

Risk Assessment

✅ Low: The branch is test-infrastructure-only (bin/fm-test-run.sh, bin/fm-spawn.sh's FM_TASK_TMP_ROOT is a no-op by default in production), already went through two self-review hardening passes within the branch itself (fixing an inert worker-stop guard and an ARG_MAX-unsafe /proc scan), and the final state's containment/reap logic is exercised by real, non-mocked tests covering normal completion, mid-run interruption, and SIGKILL-then-heal scenarios; I traced the signal-trap, fd-9 pinning, and reap in-use/protected checks and found no newly reachable leak or destructive-removal path beyond the residual, self-healing race the design already accounts for (an orphaned run root from a kill mid-worker-spawn is picked up by the next run's reap).

Testing

Ran the full contract suite for bin/fm-test-run.sh (tests/fm-test-run.test.sh, 26/26 passing) plus every other test file touched by the diff; additionally proved the new test_signalled_jobs_run_stops_its_workers test is a real regression guard by running it against both the pre-fix and post-fix runner script and observing fail→pass. The only failure seen (tests/fm-session-start.test.sh's MISSING-diagnostic assertion) was confirmed via an isolated worktree checkout to already fail identically at the base commit, so it predates and is unrelated to this change. Worktree was left clean; no stray files or temp swaps remain.

Evidence: Proof that test_signalled_jobs_run_stops_its_workers fails pre-fix and passes post-fix
Evidence: test_signalled_jobs_run_stops_its_workers (tests/fm-test-run.test.sh)
catches the "inert worker-stop guard" bug fixed in commit 90f739b.

1) Against pre-fix bin/fm-test-run.sh (commit 0fc0c98, the guard used
   `[ -n "${WORKER_PIDS+x}" ]` instead of `[ -n "${WORKER_PIDS[*]+x}" ]`):

   not ok - a signalled --jobs run left its run root behind: fm-test-run.pGW1nv
   EXIT=1

2) Against post-fix bin/fm-test-run.sh (commit 90f739b, this branch's HEAD):

   ok - a signalled --jobs run stops its workers before removing the run root
   EXIT=0

This confirms the fix is necessary (the test fails without it) and effective
(the test passes with it), i.e. a signalled --jobs run now actually kills its
worker subshells and the scripts they launched before removing the shared
run-temp root, instead of leaving an orphaned run root with live children
still writing into it.
- Outcome: ⚠️ 1 info across 1 run (4m52s)

Pipeline

Updates from git push no-mistakes

⏭️ **intent** - skipped

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

⚠️ **Test** - 1 info
  • ℹ️ tests/fm-session-start.test.sh:789 - tests/fm-session-start.test.sh fails deterministically on the assertion 'MISSING diagnostic did not appear at all' (tests/fm-session-start.test.sh:789). Verified this is a pre-existing failure unrelated to this branch: checked out the base commit 57a9478 in an isolated worktree and reproduced the identical failure there, before any of this branch's changes existed. Not a regression introduced by this diff.
  • bash bin/fm-test-run.sh tests/fm-test-run.test.sh (all 26 contract tests for the runner itself pass, including the new test_signalled_jobs_run_stops_its_workers)
  • Isolated re-run of test_signalled_jobs_run_stops_its_workers against the pre-fix bin/fm-test-run.sh (commit 0fc0c98) — fails with 'a signalled --jobs run left its run root behind', confirming the test catches the fixed worker-stop-guard bug
  • Isolated re-run of the same test against the post-fix bin/fm-test-run.sh (commit 90f739b, branch HEAD) — passes
  • bash bin/fm-test-run.sh tests/fm-backend-orca.test.sh tests/fm-backend.test.sh tests/fm-gotmp.test.sh tests/fm-kimi-harness.test.sh tests/fm-session-start.test.sh (all touched-by-diff test files)
  • bash bin/fm-test-run.sh tests/fm-session-start.test.sh on base commit 57a9478 in an isolated git worktree (/tmp/fm-base-check, removed after use) to confirm the one observed failure pre-dates this branch
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Bchue added 3 commits August 5, 2026 21:49
…to /tmp

/tmp is a tmpfs on some hosts, so a fixture the suite leaves behind is leaked
RAM. Fixtures accumulated without bound: most test files installed no EXIT trap,
and the ones that did still lost their fixtures whenever a run was killed. On one
host that reached 3.0 GB of RAM held by dead fixtures.

Fix it in the runner, which is the one place that already sits at both the start
and the end of every run, rather than in each test file.

- Every executed script now runs with TMPDIR, TMP, and FM_TASK_TMP_ROOT pointed
  at a private directory inside the run's own root, so fixtures built the way the
  suite builds them land inside the run. Each script's root is removed as soon as
  that script finishes, so peak usage stays at one script's fixtures.
- The run root is removed on a normal exit and on INT, TERM, HUP, and QUIT, and
  the runner then dies from the same signal so callers still see a signal death.
- An executing run first reaps fixture directories orphaned by runs that were
  killed outright, so an already-leaked host heals on the next run. The reap
  keeps anything it cannot establish: only direct children of the reap root, only
  mktemp-shaped fm- names, only directories this user owns, only entries past a
  minimum age, never a recorded tasktmp= path or a per-task scratch root, never a
  directory a process still holds open, and nothing at all when the in-use
  inventory or a home's task records cannot be read.
- bin/fm-spawn.sh roots per-task scratch at FM_TASK_TMP_ROOT, which is unset in
  normal operation so the real path is unchanged. A test that drives a real spawn
  previously stranded a /tmp/fm-<fixture id>/ that no teardown would ever claim.

Behavioural coverage in tests/fm-test-run.test.sh proves a completed run, an
interrupted run, and a killed run each leave no fixture behind, that a later run
reaps pre-existing orphans, and that a live task's recorded scratch directory and
a directory another process holds open are never removed.
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