Skip to content

test: own the temp namespace in the test runners - #2068

Merged
Astro-Han merged 2 commits into
mainfrom
test/runner-owns-tmpdir
Aug 4, 2026
Merged

test: own the temp namespace in the test runners#2068
Astro-Han merged 2 commits into
mainfrom
test/runner-owns-tmpdir

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Test suites reach the temp directory through 933 mkdtemp(join(tmpdir(), …)) calls spread over 250 files, and nothing removes what they create. On a developer machine that runs the suite repeatedly this accumulates without bound — the tmpdir this was found on held over 47,000 stale maka-* directories. CI containers are discarded after a run, so the cost lands entirely on local development: slower tmpdir traversal, noise for anything that scans it, and enough churn that "did this run leak?" stops being measurable.

The obvious fix is to give each call site a cleanup array and an afterEach, which is the convention builtin-tools-file-worker.test.ts already follows. Applied across the repo that is the same eight lines of book-keeping in 250 files — roughly 2,000 lines whose correctness depends on nobody ever forgetting them again, which the current state shows is not a safe assumption.

So this puts the temp namespace where the process lifetime already is. runWorkspace is the single spawn point for every workspace suite: it now hands each test process its own TMPDIR/TMP/TEMP and removes the tree when the workspace finishes, however it finishes. The headless runner gets the same treatment — it already isolates HOME, the XDG directories and APPDATA into a tree it deletes on exit, and simply left the temp namespace pointing at the host. No call site changes, and nothing new for a future test author to remember.

Ownership is the whole point, so the exit paths matter: the tree is created before the try that removes it covers everything after it, including a synchronous throw from spawn, and a removal that fails warns rather than replacing whatever result the run was already reporting.

Verification

packages/runtime against the same build, once through the runner and once directly (npm run test:dist, which is what the runner invokes and therefore the no-isolation control), counting top-level maka-* directories in the host tmpdir before and after:

tests pass fail skipped leaked dirs
direct, no isolation 3167 3158 0 9 51
through the runner 3167 3158 0 9 0
  • node --test scripts/run-workspace-tests-parallel.test.mjs scripts/run-headless-tests.test.mjs — 7/7 pass.
  • Both new behaviours were checked against their own removal rather than assumed: reverting the implementation file turns the spawn-throw test red, and dropping the rm turns the three namespace tests red.
  • npm run test:scripts (the one npm test path that does not go through the runner) measured at zero leaked directories already, so both paths are now clean.
  • npm run lint, npm run format:check — pass.

Known boundaries

  • Running a suite outside the runner (node --test by hand in a package) still leaks, by design: the isolation belongs to whatever starts the test process, and a bare node --test has no such owner.
  • packages/runtime-host builds its Unix socket endpoint under a hardcoded /tmp rather than os.tmpdir() (src/control/endpoint.ts), so it is unaffected either way; it already prunes its own stale directories.
  • The Windows TMP/TEMP variables are set alongside TMPDIR but were not exercised — no Windows host was available.
  • Not run: the full npm test across every workspace.

Suites reach the temp directory through 933 mkdtemp(tmpdir(), …) calls
across 250 files, and nothing removes what they create: the user's
tmpdir had accumulated over 47,000 stale maka-* directories. Making each
call site book-keep its own removal needs the same eight lines of
cleanup in every one of those files, and stays correct only for as long
as nobody forgets.

Give each workspace test process its own TMPDIR instead, removed with
the process tree when the workspace finishes however it ends, and do the
same in the headless runner, which already isolates HOME and XDG but
left the temp namespace pointing at the host. No call site changes.

Verified on packages/runtime: 2961 tests / 2930 pass / 22 fail both
before and after, with 51 leaked directories before and 0 after.
Review of the first commit found two holes in the ownership it claimed.
mkdtemp ran before the try, so a synchronous throw from spawn left the
tree behind — the exact shape of the leak this PR exists to stop. And an
rm that throws inside finally replaced whatever the function was already
reporting, so a cleanup failure would mask the real test failure.

Move everything after the tree exists under the try, and let removal
failure warn instead of propagate. The headless runner had the same
masking in its pre-existing finally, so it gets the same treatment.

The comment claiming the process tree is always down by then was wrong:
when terminateWorkspace itself fails the tree is removed under a process
that may still be writing. That is the right trade — the run is already
failed and killed, and leaking the tree is worse — so the comment now
says so instead of asserting something the code does not guarantee.

Tests lock the spawn-throw and stop paths; verified by reverting the
implementation, which turns the spawn-throw case red.
@Astro-Han
Astro-Han merged commit e90d59b into main Aug 4, 2026
11 checks passed
@Astro-Han
Astro-Han deleted the test/runner-owns-tmpdir branch August 4, 2026 05:33
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