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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions