Skip to content

Mandate a test-plan/state-mutation header comment at the top of each BATS suite #41

Description

@jnasbyupgrade

Proposal

Require every test/standard/*.bats (and similar) file to start with a header comment laying out:

  1. The broad test plan / scope of that suite (what code/feature it's testing)
  2. 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:

  1. Exact required content/format for the header (test plan + state-mutation summary, at minimum)
  2. 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
  3. Whether to add it retroactively to existing suites or only require it going forward for new/modified ones

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