Skip to content

Investigate: replace ad-hoc mktemp usage with BATS_TEST_TMPDIR where appropriate #45

Description

@jnasbyupgrade

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

  1. 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?
  2. 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)?
  3. 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?
  4. 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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    investigationOpen research/investigation, not a mandated change

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions