Skip to content

fix(snapshot): two false passes in the placeholder path, and cover its behaviour - #1002

Merged
Chemaclass merged 1 commit into
mainfrom
fix/snapshot-absolute-path-and-placeholder-tests
Aug 9, 2026
Merged

fix(snapshot): two false passes in the placeholder path, and cover its behaviour#1002
Chemaclass merged 1 commit into
mainfrom
fix/snapshot-absolute-path-and-placeholder-tests

Conversation

@Chemaclass

Copy link
Copy Markdown
Member

🤔 Background

Two bugs, both making a snapshot assertion report success while comparing nothing — the exact failure a snapshot test exists to prevent.

🐛 1. The placeholder fallback matched anything

Without perl, matching fell back to grep. grep applies a pattern per line, and a pattern containing newlines is read as several alternative patterns — so a multi-line snapshot whose placeholder sat on its own line contributed a bare .*, matching any line of any input.

This snapshot matched the string NOPE:

Report
::ignore::
Done

The existing comment noted grep couldn't span lines. It didn't notice grep matched everything instead.

Now awk, with RS set to a byte the input cannot contain so the whole value is one record — verified at parity with perl across all twelve behaviours. If neither perl nor awk exists, the assertion fails and says so rather than guessing.

🐛 2. Absolute test paths silently auto-recorded

resolve_file prefixed ./ unconditionally, so an absolute test path became a path relative to the caller's cwd. The real snapshot was never read, a stray one was recorded elsewhere, and every snapshot assertion in the run passed.

This is what made me disbelieve the first bug for an hour — my own probes were running against auto-recorded snapshots instead of the files I'd written.

🧪 Fourteen tests, half of them negative

A placeholder must narrow what is compared, never turn the assertion into one that always passes.

Snapshot Matches Does not match
a::ignore::b a123b, ab XXX, aXXXc
::ignore::tail anythingtail anythingelse
cost $5.00 (x) ::ignore:: that exact prefix cost 999 (x) Z
multi-line span A\n…\nZ NOPE

All pass under both perl and awk.

resolve_file gained an optional source override so a test can call it at any stack depth — BASH_SOURCE[2] is the test file only when reached through assert_match_snapshot.

📖 Docs

docs/snapshots.md now states what a placeholder does not relax, with a matching/non-matching table, the multi-line rule, why --snapshot-update skips placeholder snapshots (verified), and the perl/awk requirement.

✅ Verification

make sa · make lint · bash build.sh bin -v✅ Build verified ✅ · 1733 sequential / 1692 parallel-simple-strict.

…s behaviour

Both bugs made a snapshot assertion report success while comparing nothing,
which is the failure a snapshot test exists to prevent.

Without perl, the placeholder matcher fell back to grep. grep applies a pattern
per line, and a pattern that itself contains newlines is read as several
alternative patterns -- so a multi-line snapshot whose placeholder sat on its own
line contributed a bare `.*`, which matches any line of any input. A snapshot of

    Report
    ::ignore::
    Done

matched the string "NOPE". The existing comment noted grep could not span lines;
it did not notice that it matched everything instead. Now awk, with RS set to a
byte the input cannot contain so the whole value is one record. Verified at
parity with perl across all twelve behaviours. If neither perl nor awk exists the
assertion fails and says so, rather than guessing.

resolve_file prefixed "./" unconditionally, so an absolute test path became a
path relative to the caller's cwd. The real snapshot was never read, a stray one
was recorded somewhere else, and every snapshot assertion in that run passed. It
is what made me disbelieve the first bug for an hour: my own probes were running
against auto-recorded snapshots rather than the files I had written.

Fourteen tests added. Half of them are negative, deliberately: a placeholder must
narrow what is compared, never turn the assertion into one that always passes.
They cover mid-line, start, end, empty region, several placeholders, a lone
placeholder, multi-line spans, literal regex metacharacters, and the three
rejection cases. All pass under both perl and awk.

resolve_file takes an optional source override so a test can call it at any
stack depth; BASH_SOURCE[2] is the test file only when reached through
assert_match_snapshot.

docs/snapshots.md now states what a placeholder does *not* relax, with a table of
matching and non-matching values, the multi-line rule, why --snapshot-update
skips a placeholder snapshot, and the perl/awk requirement.

1733 sequential / 1692 parallel; baseline + 14.
@Chemaclass Chemaclass added the bug Something isn't working label Aug 9, 2026
@Chemaclass Chemaclass self-assigned this Aug 9, 2026
@Chemaclass
Chemaclass merged commit b7aa830 into main Aug 9, 2026
37 checks passed
@Chemaclass
Chemaclass deleted the fix/snapshot-absolute-path-and-placeholder-tests branch August 9, 2026 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant