Skip to content

A snapshot whose test file was deleted is invisible to --snapshot-report-unused and survives --snapshot-prune #1194

Closed

Description

@Chemaclass

Problem

The two flags exist to find snapshots no test uses. They cannot see the most
common kind: one whose owning test file was deleted or renamed.

Reproduced in a scratch project — an orphan whose owner file does not exist:

snapshots/renamed_away_test_sh.test_gone.snapshot

$ bashunit --snapshot-report-unused .
No unused snapshots.

$ bashunit --snapshot-prune .
  still present

An unused snapshot whose owner file does exist is reported correctly, with
both file and directory arguments. So the gap is specifically "owner file
gone".

This repo has two

91e523bb split bashunit_direct_fn_call_test.sh into _basic_ and
_advanced_; the pre-split snapshots were left behind and are byte-identical
leftovers:

tests/acceptance/snapshots/bashunit_direct_fn_call_test_sh.test_bashunit_direct_fn_call_failure.snapshot
tests/acceptance/snapshots/bashunit_direct_fn_call_test_sh.test_bashunit_direct_fn_call_non_existing_fn.snapshot

Neither is referenced. The tooling has never been able to flag them.

Cause

collect_unused keeps a snapshot only if its owner prefix matches a test file
this run discovered:

local owner="${file##*/}"; owner="${owner%%.*}"
case "$owners" in *"$owner"$'\n'*) ;; *) continue ;; esac

That filter is right for a subset run — running tests/unit/ must not report
tests/acceptance/'s snapshots. But an owner that no longer exists can never
be in owners, so a true orphan is skipped forever.

Proposed rule

Within a snapshots/ directory the scan already covers, if the owner test file
is absent from disk next to it, the snapshot is an orphan regardless of
what ran. Owner present but not run keeps today's behaviour.

Note this widens what --snapshot-prune deletes. That is the flag's stated
purpose, but it is a destructive command, so the change deserves a deliberate
yes rather than being folded in silently — collect_unused is shared by both
flags on purpose ("two implementations of unused is how they drift").

Activity

  1. self-assigned this
    on Aug 14, 2026
  2. Chemaclass commented on Aug 14, 2026

    @Chemaclass
    MemberAuthor

    Correction: "byte-identical" was wrong

    I wrote that the two leftovers are byte-identical to the live snapshots. They
    are not — I checked after writing it. The orphan is the pre-split recording
    and lacks the source-location line the current one carries:

    orphan:  ✗ Failed: assert same / Expected 'foo' / but got 'bar'
    live:    ✗ Failed: assert same / Expected 'foo' / but got 'bar'
             at tests/acceptance/bashunit_direct_fn_call_advanced_test.sh:16
    

    Which makes them more clearly stale, not less: they record a file path that
    no longer exists. The substance of the report is unchanged — they are
    unreferenced orphans the tooling cannot see — but the claim as written was
    inaccurate.

  3. Chemaclass commented on Aug 14, 2026

    @Chemaclass
    MemberAuthor

    I implemented the proposed rule and reverted it — it contradicts a tested guarantee

    The rule ("owner file absent from disk ⇒ orphan") works: it found both of this
    repo's leftovers and left in-use snapshots alone. Then it failed an existing
    test, test_a_snapshot_of_a_test_file_outside_the_run_is_not_reported:

    echo "not mine" >"$WORKDIR/snapshots/other_test_sh.test_something.snapshot"
    # ... run against snap.sh only
    assert_not_contains "other_test_sh" "$output"

    The fixture never creates other_test.sh. So "a snapshot naming a file that is
    not on disk" is exactly the shape that test requires to stay unreported —
    and it is the shape my rule reports. The two cannot both hold.

    Which means an orphan and "a snapshot belonging to a file outside this run" are
    not distinguishable by the signal I proposed, and the current behaviour is a
    deliberate, tested choice rather than an oversight.

    Options, all needing a decision I should not make alone:

    1. Leave it. Orphans stay invisible; --snapshot-report-unused keeps its
      "never report what this run did not own" guarantee.
    2. Report owner-missing snapshots, and change that test's fixture to create
      other_test.sh so it tests "exists but was not run" — the intent its name
      states. Rewriting a maintainer's test to fit my change is not something to
      slip into a bug fix.
    3. A separate flag (--snapshot-report-orphans), leaving both existing
      guarantees untouched.

    I have reverted the implementation. The two dead files in this repo are still
    worth deleting on their own, independently of which option is chosen.

  4. Chemaclass commented on Aug 16, 2026

    @Chemaclass
    MemberAuthor

    Done in #1275 (merged).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

  • Status
    Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions