perf(runner): drop five process forks per hooked test

.claude/rules/perf-fork-budget.md

@@ -186,7 +186,10 @@ them:
 
 - **Not forks.** A PATH-shim census over 30 binaries counts **12 forks for 500
   tests** (3 perl, 2 rm, 2 awk, and one each of uname/tput/mkdir/bc/base64).
-  The per-test path is genuinely fork-free.
+  The per-test path is genuinely fork-free -- but read the fixture before
+  trusting a census: this one defines no `set_up` or `tear_down`, and a file
+  that does used to cost five more forks per test (#1345), which is why that
+  never showed up here. A census fixture only measures the path it exercises.
 - **Not the capture subshell.** A bare `$( )` costs ~0.46 ms here, about 6% of
   the 7.8 ms. The rest is bash work in the per-test machinery.
 - **Not quadratic.** Per-test cost is 7.17 ms at 100 tests and 8.06 ms at 1000
@@ -202,7 +205,9 @@ from them rather than re-deriving them.
 shell since #817, the header count reads a return slot so the cache survives
 into the runner — plus the duplicate check), `perl` ×2 clock reads (start/end;
 no `EPOCHREALTIME` before Bash 5), 1 `base64` capability probe, 1 `mkdir`,
-1 `tput`. Per-test cost is fork-free.
+1 `tput`. Per-test cost is fork-free, hooks included: `set_up`/`tear_down`
+capture their output in a run-dir file named from the folded file path and the
+per-suite ordinal, so no `mktemp` mints it and no `rm` removes it (#1345).
 
 **Cold start: 3 binary forks** — `uname` (OS detect), `tput` (snapshot width),
 `perl` (clock before Bash 5). It was 5 until #1124: `check_os::init` ran twice,

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+### Changed
+- Performance: a test in a file that defines `set_up` or `tear_down` is about 2.6x faster, which brings it level with a hookless test (14.1ms to 5.4ms per test on macOS arm64, bash 3.2). Each hook minted its output file with `mktemp` and removed it with `rm`, and the ownership marker that left behind made the runner `rm -rf` the test's temp files at exit — five forks per test, even for a test that created no temp file of its own (#1345)
+
 ### Fixed
 - Coverage counts every line of a multiline statement: array literals, quoted strings, heredocs, backslash continuations. An array written one element per line cost one uncovered line per element. Bash 3.x records the assignment on its closing `)`, so there the whole array read as uncovered. A parent statement's hits no longer mark commands inside command or process substitutions as covered, even within quotes or arrays (#1338)
 - `--exclude-filter` keeps a comma as part of the test function name. The value was split at the comma, so `--exclude-filter 'test_a,{b}'` also excluded `test_a`. Repeat the flag to exclude several names. `BASHUNIT_EXCLUDE_FILTER` stays comma-separated, so one of its filters cannot hold a literal comma (#1340)

src/config/env.sh

@@ -936,6 +936,39 @@ function bashunit::env::create_scratch_dirs() {
 
 bashunit::env::create_scratch_dirs "$_BASHUNIT_RUN_OUTPUT_DIR" "$BASHUNIT_TEMP_DIR" || exit 1
 
+##
+# Restores the run scratch directory when it vanishes mid-run.
+#
+# A missing directory makes a capture redirect fail, and bash reports that as
+# the *command* failing: exit 1 with nothing written to the capture file. That
+# read as a test file which would not source (#1137), and as a hook that failed
+# (#1345). Callers fall back to /dev/null when this returns 1 -- losing one
+# capture is worth far less than failing something for a reason that is not its
+# own.
+#
+# Says so when it happens, once per run: #1137 is open precisely because a
+# scratch directory goes missing on CI and nobody can say what removed it.
+# Surviving it silently would keep it that way.
+#
+# The common case is a `[ -d ]` builtin, so callers on the per-test path pay no
+# fork (.claude/rules/perf-fork-budget.md).
+# Returns: 0 when the directory is there, 1 when it could not be restored
+##
+function bashunit::env::ensure_run_output_dir() {
+  if [ -d "$_BASHUNIT_RUN_OUTPUT_DIR" ]; then
+    return 0
+  fi
+
+  if [ "${_BASHUNIT_RUN_DIR_VANISHED:-false}" = false ]; then
+    _BASHUNIT_RUN_DIR_VANISHED=true
+    printf 'bashunit: the run scratch directory disappeared mid-run: %s\n' \
+      "$_BASHUNIT_RUN_OUTPUT_DIR" >&2
+    printf 'bashunit: recreating it; please report this with the run log (#1137).\n' >&2
+  fi
+
+  mkdir -p "$_BASHUNIT_RUN_OUTPUT_DIR" 2>/dev/null
+}
+
 # Removes this run's scratch directory (guarded like parallel::cleanup so a
 # broken variable can never turn the rm loose elsewhere). Called at the end of
 # a run and on SIGINT; without it every invocation leaks one directory.

src/runner/discovery.sh

@@ -71,26 +71,11 @@ function bashunit::runner::load_test_files() {
     # run-dir cleanup removes it, saving a mktemp and an rm fork per file.
     local source_err_file source_err source_status
     source_err_file="$_BASHUNIT_RUN_OUTPUT_DIR/source_err"
-    # A missing scratch dir makes the redirect below fail, and bash reports that
-    # as the *command* failing: exit 1 with nothing written to the capture file,
-    # which read as "this test file failed to source" against a file that was
-    # complete and valid (#1137). Restore the directory, and fall back to
-    # /dev/null if even that is refused -- losing a file's stderr capture is
-    # worth far less than failing the file for a reason that is not its own.
-    #
-    # Say so when it happens, once per run: #1137 is open precisely because a
-    # scratch directory goes missing on CI and nobody can say what removed it.
-    # Surviving it silently would keep it that way.
-    if [ ! -d "$_BASHUNIT_RUN_OUTPUT_DIR" ]; then
-      if [ "${_BASHUNIT_RUN_DIR_VANISHED:-false}" = false ]; then
-        _BASHUNIT_RUN_DIR_VANISHED=true
-        printf 'bashunit: the run scratch directory disappeared mid-run: %s\n' \
-          "$_BASHUNIT_RUN_OUTPUT_DIR" >&2
-        printf 'bashunit: recreating it; please report this with the run log (#1137).\n' >&2
-      fi
-      if ! mkdir -p "$_BASHUNIT_RUN_OUTPUT_DIR" 2>/dev/null; then
-        source_err_file=/dev/null
-      fi
+    # A missing scratch dir makes the redirect below fail, which read as "this
+    # test file failed to source" against a file that was complete and valid
+    # (#1137). Restore it, and capture nothing if even that is refused.
+    if ! bashunit::env::ensure_run_output_dir; then
+      source_err_file=/dev/null
     fi
     # shellcheck source=/dev/null
     source "$test_file" 2>"$source_err_file"

src/runner/hooks.sh

@@ -80,6 +80,39 @@ function bashunit::runner::record_file_hook_failure() {
   return "$status"
 }
 
+_BASHUNIT_RUNNER_HOOK_FILE=""
+
+##
+# Names the file a hook's output is captured in, in _BASHUNIT_RUNNER_HOOK_FILE.
+#
+# A fixed path in the run dir, not a `temp_file`: minting one forked `mktemp`,
+# removing it forked `rm -f`, and the temp-owner marker `mktemp` leaves behind
+# made the EXIT trap `rm -rf` the test's temp files as well, even when the test
+# created none. Five forks per test in any file defining `set_up` or
+# `tear_down`, which cost 3.3x a hookless test (28.3ms vs 8.5ms) (#1345). The
+# `>` redirect truncates, so the name may be reused and nothing has to remove
+# it; the run-dir cleanup takes it with the rest at the end.
+#
+# The key has to survive concurrency, and a Bash 3 subshell inherits both `$$`
+# and the `RANDOM` state from its parent. So it is built the way result.sh
+# names `.result` files (#851): the folded file path, which keeps two files
+# sharing a basename apart (#959), plus the per-suite ordinal the dispatcher
+# assigns before forking, which keeps a file's parallel test workers apart.
+# The hook name closes the last gap, between a file hook and a test hook that
+# share an ordinal.
+#
+# Arguments: $1 - the hook name
+##
+function bashunit::runner::hook_output_path() {
+  if ! bashunit::env::ensure_run_output_dir; then
+    _BASHUNIT_RUNNER_HOOK_FILE=/dev/null
+    return 0
+  fi
+
+  local slot="${_BASHUNIT_RUNNER_FILE_SLOT}_${_BASHUNIT_RUNNER_RESULT_ORDINAL:-0}_$1"
+  _BASHUNIT_RUNNER_HOOK_FILE="${_BASHUNIT_RUN_OUTPUT_DIR}/hook_${slot}"
+}
+
 function bashunit::runner::execute_file_hook() {
   local hook_name="$1"
   local test_file="$2"
@@ -90,7 +123,9 @@ function bashunit::runner::execute_file_hook() {
   local hook_output=""
   local status=0
   local hook_output_file
-  hook_output_file=$(bashunit::temp_file "${hook_name}_output")
+  bashunit::runner::file_path_to_slot "$test_file"
+  bashunit::runner::hook_output_path "$hook_name"
+  hook_output_file=$_BASHUNIT_RUNNER_HOOK_FILE
 
   # Enable errtrace to catch any failing command in the hook.
   # Using -E (errtrace) without -e (errexit) prevents the main process from
@@ -138,7 +173,6 @@ function bashunit::runner::execute_file_hook() {
     while IFS= read -r line; do
       [ -z "$hook_output" ] && hook_output="$line" || hook_output="$hook_output"$'\n'"$line"
     done <"$hook_output_file"
-    rm -f "$hook_output_file"
   fi
 
   if [ $status -ne 0 ]; then
@@ -154,8 +188,8 @@ function bashunit::runner::execute_file_hook() {
 }
 
 function bashunit::runner::run_set_up() {
-  local _test_file="${1-}"
   bashunit::internal_log "run_set_up"
+  bashunit::runner::file_path_to_slot "${1-}"
   bashunit::runner::execute_test_hook 'set_up'
 }
 
@@ -199,8 +233,8 @@ function bashunit::runner::run_set_up_before_script() {
 }
 
 function bashunit::runner::run_tear_down() {
-  local _test_file="${1-}"
   bashunit::internal_log "run_tear_down"
+  bashunit::runner::file_path_to_slot "${1-}"
   bashunit::runner::execute_test_hook 'tear_down'
 }
 
@@ -212,7 +246,8 @@ function bashunit::runner::execute_test_hook() {
   local hook_output=""
   local status=0
   local hook_output_file
-  hook_output_file=$(bashunit::temp_file "${hook_name}_output")
+  bashunit::runner::hook_output_path "$hook_name"
+  hook_output_file=$_BASHUNIT_RUNNER_HOOK_FILE
 
   # Enable errtrace to catch any failing command in the hook.
   # Using -E (errtrace) without -e (errexit) prevents the subshell from
@@ -255,7 +290,6 @@ function bashunit::runner::execute_test_hook() {
     while IFS= read -r line; do
       [ -z "$hook_output" ] && hook_output="$line" || hook_output="$hook_output"$'\n'"$line"
     done <"$hook_output_file"
-    rm -f "$hook_output_file"
   fi
 
   if [ $status -ne 0 ]; then

src/runner/result.sh

@@ -34,10 +34,27 @@ _BASHUNIT_RUNNER_SUITE_DIR_OUT=""
 # Arguments: $1 - the test file path
 ##
 function bashunit::runner::parallel_suite_dir_to_slot() {
+  bashunit::runner::file_path_to_slot "$1"
+  _BASHUNIT_RUNNER_SUITE_DIR_OUT="${TEMP_DIR_PARALLEL_TEST_SUITE}/${_BASHUNIT_RUNNER_FILE_SLOT}"
+}
+
+_BASHUNIT_RUNNER_FILE_SLOT=""
+
+##
+# Folds a test file path into one filename-safe key in
+# _BASHUNIT_RUNNER_FILE_SLOT.
+#
+# Anything naming a per-file scratch path shares this, so that the reasoning
+# above about keying on the whole path holds everywhere at once.
+#
+# Pure parameter expansion: callers sit on the per-test path, which must stay
+# fork-free (.claude/rules/perf-fork-budget.md).
+# Arguments: $1 - the test file path
+##
+function bashunit::runner::file_path_to_slot() {
   local key="${1#./}"
   key="${key%.sh}"
-  key="${key//\//_}"
-  _BASHUNIT_RUNNER_SUITE_DIR_OUT="${TEMP_DIR_PARALLEL_TEST_SUITE}/${key}"
+  _BASHUNIT_RUNNER_FILE_SLOT="${key//\//_}"
 }
 
 function bashunit::runner::parse_result_parallel() {

tests/acceptance/bashunit_run_forks_test.sh

@@ -282,3 +282,56 @@ function test_reports_do_not_fork_base64_per_field() {
   # that, with room for the run's own bookkeeping; it was 56.
   assert_less_or_equal_than 16 "$calls"
 }
+
+# Regression guard for the per-test hook path. A test in a file that defines
+# `set_up` or `tear_down` used to cost five process forks: each hook minted its
+# output file with `mktemp` and removed it with `rm -f`, and the temp-owner
+# marker `mktemp` left behind made the EXIT trap `rm -rf` the test's temp files
+# even when the test itself created none. That is 3.3x the cost of a hookless
+# test (28.3ms vs 8.5ms). The hook output file is named arithmetically inside
+# the run directory now, and the `>` redirect truncates it, so a hooked test
+# forks neither binary — only the run's own single `rm` of its scratch dir
+# remains.
+function test_test_hooks_do_not_fork_mktemp_or_rm_per_test() {
+  if bashunit::check_os::is_windows; then
+    bashunit::skip "PATH shims are unreliable under Git Bash" && return
+  fi
+
+  local dir
+  dir="$(bashunit::temp_dir)"
+  local count_file="$dir/count"
+  local bin
+  for bin in mktemp rm; do
+    local real_bin
+    real_bin="$(command -v "$bin")"
+    {
+      echo '#!/usr/bin/env bash'
+      echo "echo $bin >> \"$count_file\""
+      echo "exec \"$real_bin\" \"\$@\""
+    } >"$dir/$bin"
+    chmod +x "$dir/$bin"
+  done
+
+  local fixture="$dir/hook_forks_test.sh"
+  {
+    echo 'function set_up() { :; }'
+    echo 'function tear_down() { :; }'
+    echo 'function test_a() { assert_true true; }'
+    echo 'function test_b() { assert_true true; }'
+    echo 'function test_c() { assert_true true; }'
+    echo 'function test_d() { assert_true true; }'
+  } >"$fixture"
+
+  PATH="$dir:$PATH" ./bashunit --no-parallel "$fixture" >/dev/null 2>&1
+
+  local mktemp_forks=0
+  local rm_forks=0
+  if [ -f "$count_file" ]; then
+    mktemp_forks="$(grep -c '^mktemp$' "$count_file" || true)"
+    rm_forks="$(grep -c '^rm$' "$count_file" || true)"
+  fi
+
+  assert_equals 0 "$mktemp_forks"
+  # The run's own scratch-dir cleanup, and nothing per test.
+  assert_less_or_equal_than 1 "$rm_forks"
+}