Context
While adding stub-based tests for check-stale-expected (pgxntool #61 / pgxntool-test #31), test/lib/helpers.bash's make_stub_script was changed to write directly to "$BATS_TEST_TMPDIR/<filename>" instead of calling mktemp -- BATS_TEST_TMPDIR is already a fresh, unique directory bats-core creates automatically for every single @test, so mktemp was only buying a randomized name inside a directory that's already unique. This issue is to investigate whether the same simplification applies elsewhere in the suite. This is an investigation, not a mandate -- it may turn out some/all of the existing usages are fine as-is.
Known ad-hoc mktemp usages (found via grep, excluding the vendored test/bats/ submodule)
test/standard/update-setup-files.bats:101 -- local tmp_file=$(mktemp), used to stage a modified _.gitignore before mv-ing it into place. Looks like a straightforward candidate: could become $BATS_TEST_TMPDIR/gitignore-header (or similar) instead.
test/standard/concurrent-make-test.bats:95-96 -- log1=$(mktemp) / log2=$(mktemp), capturing output from two make test invocations run in parallel background subshells. Also looks like a candidate ($BATS_TEST_TMPDIR/log1, $BATS_TEST_TMPDIR/log2), but should be double-checked against the parallel-subshell usage in that test -- confirm nothing about backgrounding/wait semantics depends on the files living outside BATS_TEST_TMPDIR (it shouldn't, but verify).
There may be other ad-hoc temp-file/dir creation patterns beyond literal mktemp calls (e.g. hand-rolled unique-name generation) that a broader search should also cover.
Relationship to the existing test/.envs/<name>/ + ensure_foundation/load_test_env infrastructure
This is NOT the same granularity as BATS_TEST_TMPDIR and the two are not interchangeable:
test/.envs/<name>/ (created via create_env/load_test_env, populated via ensure_foundation) is a heavier, longer-lived, explicitly concurrency-safe mechanism -- it has its own lock-file/PID tracking (see the concurrent-test-execution warnings at the top of helpers.bash), persists across many @test blocks within a setup_file/setup lifecycle in the same file (and is reused across separate bats invocations within a run), and holds a full foundation TEST_REPO (git subtree, template files, etc.).
BATS_TEST_TMPDIR is fresh per single test, torn down (or at least not guaranteed to persist) after that one @test completes, and has no concurrency-safety mechanism of its own beyond bats-core's own per-test isolation.
Using the heavier mechanism where the lighter one would do adds unnecessary setup cost and coupling to the environment-lock system; using the lighter one where cross-test persistence or concurrency-safety is actually needed would be a correctness bug, not just a style issue.
Open questions for further research
- Which of the two known
mktemp usages above (and any others a fuller search turns up) genuinely need nothing beyond single-test-scoped temp storage, and are safe to switch to BATS_TEST_TMPDIR?
- Are there any places currently using the heavier
test/.envs/<name>/ fake-repo infrastructure where a single test's BATS_TEST_TMPDIR would actually have sufficed (over-provisioned use of the heavier mechanism)?
- Does switching
concurrent-make-test.bats's log files to BATS_TEST_TMPDIR have any interaction with the parallel-subshell/wait pattern in that test that needs verifying?
- Is there value in documenting this distinction (ad-hoc
mktemp vs. BATS_TEST_TMPDIR vs. test/.envs/<name>/) in CLAUDE.md once the investigation lands, so future test additions default to the right granularity from the start?
References
test/lib/helpers.bash (make_stub_script, the concrete example that prompted this)
test/standard/update-setup-files.bats:101
test/standard/concurrent-make-test.bats:95-96
test/lib/helpers.bash (ensure_foundation, load_test_env, create_env, clean_env -- the heavier fake-repo infrastructure)
Context
While adding stub-based tests for
check-stale-expected(pgxntool #61 / pgxntool-test #31),test/lib/helpers.bash'smake_stub_scriptwas changed to write directly to"$BATS_TEST_TMPDIR/<filename>"instead of callingmktemp--BATS_TEST_TMPDIRis already a fresh, unique directory bats-core creates automatically for every single@test, somktempwas only buying a randomized name inside a directory that's already unique. This issue is to investigate whether the same simplification applies elsewhere in the suite. This is an investigation, not a mandate -- it may turn out some/all of the existing usages are fine as-is.Known ad-hoc
mktempusages (found via grep, excluding the vendoredtest/bats/submodule)test/standard/update-setup-files.bats:101--local tmp_file=$(mktemp), used to stage a modified_.gitignorebeforemv-ing it into place. Looks like a straightforward candidate: could become$BATS_TEST_TMPDIR/gitignore-header(or similar) instead.test/standard/concurrent-make-test.bats:95-96--log1=$(mktemp)/log2=$(mktemp), capturing output from twomake testinvocations run in parallel background subshells. Also looks like a candidate ($BATS_TEST_TMPDIR/log1,$BATS_TEST_TMPDIR/log2), but should be double-checked against the parallel-subshell usage in that test -- confirm nothing about backgrounding/waitsemantics depends on the files living outsideBATS_TEST_TMPDIR(it shouldn't, but verify).There may be other ad-hoc temp-file/dir creation patterns beyond literal
mktempcalls (e.g. hand-rolled unique-name generation) that a broader search should also cover.Relationship to the existing
test/.envs/<name>/+ensure_foundation/load_test_envinfrastructureThis is NOT the same granularity as
BATS_TEST_TMPDIRand the two are not interchangeable:test/.envs/<name>/(created viacreate_env/load_test_env, populated viaensure_foundation) is a heavier, longer-lived, explicitly concurrency-safe mechanism -- it has its own lock-file/PID tracking (see the concurrent-test-execution warnings at the top ofhelpers.bash), persists across many@testblocks within asetup_file/setuplifecycle in the same file (and is reused across separatebatsinvocations within a run), and holds a full foundation TEST_REPO (git subtree, template files, etc.).BATS_TEST_TMPDIRis fresh per single test, torn down (or at least not guaranteed to persist) after that one@testcompletes, and has no concurrency-safety mechanism of its own beyond bats-core's own per-test isolation.Using the heavier mechanism where the lighter one would do adds unnecessary setup cost and coupling to the environment-lock system; using the lighter one where cross-test persistence or concurrency-safety is actually needed would be a correctness bug, not just a style issue.
Open questions for further research
mktempusages above (and any others a fuller search turns up) genuinely need nothing beyond single-test-scoped temp storage, and are safe to switch toBATS_TEST_TMPDIR?test/.envs/<name>/fake-repo infrastructure where a single test'sBATS_TEST_TMPDIRwould actually have sufficed (over-provisioned use of the heavier mechanism)?concurrent-make-test.bats's log files toBATS_TEST_TMPDIRhave any interaction with the parallel-subshell/waitpattern in that test that needs verifying?mktempvs.BATS_TEST_TMPDIRvs.test/.envs/<name>/) in CLAUDE.md once the investigation lands, so future test additions default to the right granularity from the start?References
test/lib/helpers.bash(make_stub_script, the concrete example that prompted this)test/standard/update-setup-files.bats:101test/standard/concurrent-make-test.bats:95-96test/lib/helpers.bash(ensure_foundation,load_test_env,create_env,clean_env-- the heavier fake-repo infrastructure)