diff --git a/CHANGELOG.md b/CHANGELOG.md index 7154ed16..3174615f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ - Internal: Split `src/runner.sh` and `src/coverage.sh` into focused modules with no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925) ### Fixed +- Snapshot placeholders no longer report a false match without `perl`. The fallback matched line by line, so a multi-line snapshot whose placeholder sat on its own line contributed a bare `.*` that matched any input; it now uses `awk` over the whole value, and fails loudly if neither tool is available +- Snapshots resolve correctly when the test file is given as an absolute path. The path was always prefixed with `./`, so the real snapshot was never read and a stray one was recorded under the current directory, silently passing every snapshot assertion in the run - `assert_false` no longer passes when the command does not exist; exit codes 127 and 126 fail both boolean assertions, because the command never ran (#982) - `assert_have_been_called_times` and `assert_have_been_called_nth_with` report a usage error for a non-numeric count instead of leaking a raw `integer expression expected` (#984) - A failing test whose output quotes a shell-error phrase is no longer also reported as a runtime error (#992) diff --git a/docs/snapshots.md b/docs/snapshots.md index 73e1a253..542a371d 100644 --- a/docs/snapshots.md +++ b/docs/snapshots.md @@ -101,9 +101,11 @@ with ANSI escape sequences removed. ## Placeholders -Snapshot files can contain placeholder tokens to ignore variable parts of the output. -By default the token `::ignore::` will match any text. You can override it with the -`BASHUNIT_SNAPSHOT_PLACEHOLDER` environment variable. +Output that changes between runs — a timestamp, a PID, a temp path, a duration — would +make a snapshot fail every time. A placeholder marks the part you have decided not to +pin, so the rest still is. + +The default token is `::ignore::`; override it with `BASHUNIT_SNAPSHOT_PLACEHOLDER`. ```bash [Example] # snapshot file content @@ -113,6 +115,47 @@ echo 'Run at ::ignore::' > snapshots/example.snapshot assert_match_snapshot "Run at $(date)" ``` +### What a placeholder does and does not relax + +Everything outside the placeholder still has to match **exactly**. A placeholder is not +a wildcard for the whole snapshot: `a::ignore::b` matches `a123b`, and does *not* match +`XXX` or `aXXXc`. + +| Snapshot | Matches | Does not match | +|---|---|---| +| `a::ignore::b` | `a123b`, `ab` (an empty region matches) | `XXX`, `aXXXc` | +| `::ignore::tail` | `anythingtail` | `anythingelse` | +| `a=::ignore:: b=::ignore::` | `a=1 b=2` | `a=1 c=2` | +| `::ignore::` alone | any value at all | — | + +Text around a placeholder is compared literally, so regex metacharacters need no +escaping: a snapshot reading `cost $5.00 (x) ::ignore::` matches that exact prefix and +nothing else. + +A placeholder may span several lines. Given this snapshot: + +``` +Report +::ignore:: +Done +``` + +`Report\nany\nnumber\nof lines\nDone` matches, and unrelated output does not. + +### Placeholders and `--snapshot-update` + +`--snapshot-update` **skips** any snapshot containing a placeholder and says so. The +placeholder is a deliberate decision about what not to pin, and re-recording would +silently replace it with whatever this run happened to produce. Remove the placeholder +first if you really want the literal value. + +### Requirements + +Matching a placeholder needs `perl` or `awk`; one of them is present on essentially any +system that runs bash. If neither is, the assertion **fails** with a message saying so +rather than reporting a pass it could not verify. Snapshots without a placeholder are a +plain string comparison and need neither. + ## Finding unused snapshots A snapshot file is named after its test file and test function, so renaming or deleting a diff --git a/src/assert/snapshot.sh b/src/assert/snapshot.sh index 811cad2f..dadb680c 100644 --- a/src/assert/snapshot.sh +++ b/src/assert/snapshot.sh @@ -220,23 +220,32 @@ function bashunit::snapshot::match_with_placeholder() { local regex="^${escaped//$token/(.|\\n)*}$" if command -v perl >/dev/null 2>&1; then - echo "$actual" | REGEX="$regex" perl -0 -e ' + printf '%s' "$actual" | REGEX="$regex" perl -0 -e ' my $r = $ENV{REGEX}; my $input = join("", ); exit($input =~ /$r/s ? 0 : 1); ' && return 0 || return 1 - else - # No perl: build the pattern exactly like the perl branch — swap the - # placeholder for a token that survives escaping, escape the regex - # metacharacters, then turn the token into `.*`. (The previous order, - # escaping after inserting `.*`, escaped the `.*` itself and broke every - # fallback match.) grep matches line-by-line, so unlike the perl branch a - # placeholder cannot span multiple lines here. - local fallback="${snapshot//$placeholder/$token}" - fallback=$(printf '%s' "$fallback" | sed -e 's/[.[\\^$*+?{}()|]/\\&/g') - fallback="^${fallback//$token/.*}$" - echo "$actual" | grep -Eq "$fallback" && return 0 || return 1 fi + + # awk, not grep. grep applies the pattern per line, and a pattern that itself + # contains newlines is read as several alternative patterns -- so a multi-line + # snapshot whose placeholder sits on its own line contributed a bare `.*`, + # which matches any line of any input. That made an unrelated value pass while + # comparing nothing. Setting RS to a byte the input cannot contain gives awk + # the whole value as one record, so the anchors and the placeholder behave as + # they do under perl. + if bashunit::dependencies::has_awk; then + printf '%s' "$actual" | REGEX="$regex" awk ' + BEGIN { RS = "\001"; re = ENVIRON["REGEX"] } + { exit !($0 ~ re) } + ' && return 0 || return 1 + fi + + # Neither available: refuse rather than guess. A placeholder snapshot that + # cannot be evaluated must not report success. + printf '%sCannot match a snapshot placeholder: neither perl nor awk is available.%s\n' \ + "${_BASHUNIT_COLOR_FAILED:-}" "${_BASHUNIT_COLOR_DEFAULT:-}" >&2 + return 1 } # Writes the resolved snapshot path into _BASHUNIT_SNAPSHOT_FILE_OUT (no fork). @@ -255,7 +264,11 @@ function bashunit::snapshot::resolve_file() { # dirname via parameter expansion. `dirname "foo.sh"` (no slash) is ".", which # `${src%/*}` cannot yield, so special-case the slashless path. - local src="${BASH_SOURCE[2]}" + # + # $4 exists for the tests: BASH_SOURCE[2] is the test file only when this is + # reached through assert_match_snapshot, so a test calling it directly would + # otherwise resolve against the runner's own source. + local src="${4:-${BASH_SOURCE[2]}}" local dir_part case "$src" in */*) dir_part="${src%/*}" ;; @@ -272,7 +285,14 @@ function bashunit::snapshot::resolve_file() { name="$name.$_BASHUNIT_HELPER_VARNAME_OUT" fi - _BASHUNIT_SNAPSHOT_FILE_OUT="./${dir_part}/snapshots/${test_file}.${name}.snapshot" + # An absolute directory must stay absolute. Prefixing "./" turned + # "/abs/dir" into a path relative to the caller's cwd, so the real snapshot + # was never read and a stray one was recorded elsewhere -- every snapshot + # assertion in that run passed while comparing nothing. + case "$dir_part" in + /*) _BASHUNIT_SNAPSHOT_FILE_OUT="${dir_part}/snapshots/${test_file}.${name}.snapshot" ;; + *) _BASHUNIT_SNAPSHOT_FILE_OUT="./${dir_part}/snapshots/${test_file}.${name}.snapshot" ;; + esac } function bashunit::snapshot::initialize() { diff --git a/tests/unit/assert/snapshot_test.sh b/tests/unit/assert/snapshot_test.sh index 4187e52e..724a5093 100644 --- a/tests/unit/assert/snapshot_test.sh +++ b/tests/unit/assert/snapshot_test.sh @@ -262,3 +262,104 @@ function test_snapshot_resolve_file_uses_explicit_hint_verbatim() { assert_same "/tmp/custom.snapshot" "$_BASHUNIT_SNAPSHOT_FILE_OUT" } + +# resolve_file used to prefix "./" unconditionally, so an absolute test path +# became "./" + "/abs/dir" -- a path relative to the *current* directory. The +# real snapshot was never read and a stray one was recorded under the caller's +# cwd, so every snapshot assertion in that run passed while comparing nothing. +function test_resolve_file_keeps_an_absolute_directory_absolute() { + bashunit::snapshot::resolve_file "" "test_example" "" "/abs/dir/my_test.sh" + + assert_same "/abs/dir/snapshots/my_test_sh.test_example.snapshot" \ + "$_BASHUNIT_SNAPSHOT_FILE_OUT" +} + +function test_resolve_file_keeps_a_relative_directory_relative() { + bashunit::snapshot::resolve_file "" "test_example" "" "tests/unit/my_test.sh" + + assert_same "./tests/unit/snapshots/my_test_sh.test_example.snapshot" \ + "$_BASHUNIT_SNAPSHOT_FILE_OUT" +} + +# A path with no slash yields dir_part "." and therefore a doubled "./". That is +# the shape this function has always produced and the comment above it says so +# deliberately; it resolves identically, so it is pinned rather than tidied. +function test_resolve_file_handles_a_slashless_path() { + bashunit::snapshot::resolve_file "" "test_example" "" "my_test.sh" + + assert_same "././snapshots/my_test_sh.test_example.snapshot" \ + "$_BASHUNIT_SNAPSHOT_FILE_OUT" +} + +function test_resolve_file_includes_the_snapshot_name_when_given() { + bashunit::snapshot::resolve_file "" "test_example" "stderr" "tests/my_test.sh" + + assert_same "./tests/snapshots/my_test_sh.test_example.stderr.snapshot" \ + "$_BASHUNIT_SNAPSHOT_FILE_OUT" +} + +# The placeholder marks a region of the snapshot the author chose not to pin. +# These cover the whole surface, and deliberately include the negative cases: +# a placeholder must not turn the assertion into one that always passes. +function snapshot_with() { # $1 snapshot content -> prints the path + local path="$(bashunit::temp_dir)/placeholder.snapshot" + printf '%s' "$1" >"$path" + printf '%s' "$path" +} + +function test_placeholder_matches_a_varying_region_mid_line() { + assert_empty "$(assert_match_snapshot "a123b" "$(snapshot_with 'a::ignore::b')")" +} + +function test_placeholder_matches_at_the_start_and_end() { + assert_empty "$(assert_match_snapshot "anythingtail" "$(snapshot_with '::ignore::tail')")" + assert_empty "$(assert_match_snapshot "headanything" "$(snapshot_with 'head::ignore::')")" +} + +function test_placeholder_matches_an_empty_region() { + assert_empty "$(assert_match_snapshot "ab" "$(snapshot_with 'a::ignore::b')")" +} + +function test_several_placeholders_in_one_snapshot() { + assert_empty "$(assert_match_snapshot "a=1 b=2" "$(snapshot_with 'a=::ignore:: b=::ignore::')")" +} + +function test_a_lone_placeholder_matches_any_value() { + assert_empty "$(assert_match_snapshot "literally anything" "$(snapshot_with '::ignore::')")" +} + +function test_placeholder_spans_multiple_lines() { + local snapshot actual + snapshot=$(snapshot_with "$(printf 'A\n::ignore::\nZ')") + actual=$(printf 'A\nq\nw\nZ') + + assert_empty "$(assert_match_snapshot "$actual" "$snapshot")" +} + +# The important half. Without these a placeholder could silently become +# "match anything" and the assertion would report success while comparing +# nothing -- which is exactly what the pre-awk fallback did for the multi-line +# case. +function test_placeholder_still_requires_the_surrounding_text() { + assert_not_empty "$(assert_match_snapshot "XXX" "$(snapshot_with 'a::ignore::b')")" +} + +function test_placeholder_rejects_a_differing_suffix() { + assert_not_empty "$(assert_match_snapshot "aXXXc" "$(snapshot_with 'a::ignore::b')")" +} + +function test_multi_line_placeholder_rejects_unrelated_output() { + local snapshot + snapshot=$(snapshot_with "$(printf 'A\n::ignore::\nZ')") + + assert_not_empty "$(assert_match_snapshot "NOPE" "$snapshot")" +} + +# Regex metacharacters in the snapshot are literal text, not pattern syntax. +function test_regex_metacharacters_around_a_placeholder_are_literal() { + local snapshot + snapshot=$(snapshot_with 'cost $5.00 (x) ::ignore::') + + assert_empty "$(assert_match_snapshot 'cost $5.00 (x) Z' "$snapshot")" + assert_not_empty "$(assert_match_snapshot 'cost 999 (x) Z' "$snapshot")" +}