test(tests): stop the test runner from leaking fixture dirs into /tmp - #2
Open
brchue-ux wants to merge 3 commits into
Open
test(tests): stop the test runner from leaking fixture dirs into /tmp#2brchue-ux wants to merge 3 commits into
brchue-ux wants to merge 3 commits into
Conversation
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.
…eanup, and test isolation
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
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.
What Changed
bin/fm-test-run.shnow owns fixture cleanup end-to-end: it creates a private run root (with EXIT/INT/TERM/HUP/QUIT traps) plus a privateTMPDIR/TMP/FM_TASK_TMP_ROOTfor each executed script, removing each script's temp root as soon as it finishes so an interrupted run cleans up instead of leaking/tmpfixtures (or RAM, on tmpfs hosts);bin/fm-spawn.shhonors the newFM_TASK_TMP_ROOToverride so spawned tasks land inside a test's private root.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 livetasktmp=path across discoverable Firstmate homes/secondmates, or still held open by a process (via/procorlsof), and is configurable/testable viaFM_TEST_REAP_ROOT,FM_TEST_REAP_MIN_AGE_SECONDS,FM_TEST_REAP_MAX, andFM_TEST_REAP_HOMES.stop_run_workers/on_run_signalto terminate a signalled--jobsrun's worker subshells and their full process trees before the run root is removed, fixed an inert worker-stop guard and a slow batched/procreap scan, and updatedtests/lib.sh,CONTRIBUTING.md, and the touchedtests/*.test.shfiles (including a newtests/fm-test-run.test.shcontract 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
Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
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 bugIsolated re-run of the same test against the post-fix bin/fm-test-run.sh (commit 90f739b, branch HEAD) — passesbash 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.