Proposal
Require every test/standard/*.bats (and similar) file to start with a header comment laying out:
- The broad test plan / scope of that suite (what code/feature it's testing)
- How state is mutated/accumulated across the suite's
@test blocks (since BATS runs them in file order within one shared environment, later tests can depend on state left by earlier ones)
Motivation
This came up concretely while consolidating test files in PRs #61/#31 (build-system fixes):
- A new file (
versioned-sql-race.bats) was created without realizing concurrent-make-test.bats already built a compatible single-extension environment for a similar concurrency-testing purpose -- a suite-scope header would have surfaced that overlap immediately instead of requiring a full read-through to discover.
- Folding
check-stale-expected.bats's tests into make-test.bats required manually verifying that make-test.bats's existing tests (which progressively mutate shared state -- creating/removing test/output, adding test/sql/dbname.sql/test/expected/dbname.out) wouldn't interfere with the new orphan-detection tests being added. A state-mutation header would make this "can this fold in safely" judgment call near-instant instead of requiring careful manual tracing.
Known risk: staleness
We hit this directly in the same PR: a header comment describing check-stale-expected's execution order ("runs before install/installcheck") went stale after the underlying code changed (it now runs after, via an explicit dependency edge) and wasn't updated to match -- directly contradicting the file's own test titled "...runs after pg_regress, not before" a few lines below it. A bare comment convention with no enforcement is exactly the kind of documentation that silently rots.
Ask
Decide on:
- Exact required content/format for the header (test plan + state-mutation summary, at minimum)
- Some mechanism to catch drift -- e.g. a review-time checklist item whenever a suite's
setup/setup_file/state-mutating tests change, since there's no automated way to verify a comment matches code behavior
- Whether to add it retroactively to existing suites or only require it going forward for new/modified ones
Proposal
Require every
test/standard/*.bats(and similar) file to start with a header comment laying out:@testblocks (since BATS runs them in file order within one shared environment, later tests can depend on state left by earlier ones)Motivation
This came up concretely while consolidating test files in PRs #61/#31 (build-system fixes):
versioned-sql-race.bats) was created without realizingconcurrent-make-test.batsalready built a compatible single-extension environment for a similar concurrency-testing purpose -- a suite-scope header would have surfaced that overlap immediately instead of requiring a full read-through to discover.check-stale-expected.bats's tests intomake-test.batsrequired manually verifying thatmake-test.bats's existing tests (which progressively mutate shared state -- creating/removingtest/output, addingtest/sql/dbname.sql/test/expected/dbname.out) wouldn't interfere with the new orphan-detection tests being added. A state-mutation header would make this "can this fold in safely" judgment call near-instant instead of requiring careful manual tracing.Known risk: staleness
We hit this directly in the same PR: a header comment describing
check-stale-expected's execution order ("runs before install/installcheck") went stale after the underlying code changed (it now runs after, via an explicit dependency edge) and wasn't updated to match -- directly contradicting the file's own test titled "...runs after pg_regress, not before" a few lines below it. A bare comment convention with no enforcement is exactly the kind of documentation that silently rots.Ask
Decide on:
setup/setup_file/state-mutating tests change, since there's no automated way to verify a comment matches code behavior