From ffcbed6c41c3807da14dc0976ce89d251cfa4d1c Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Tue, 15 Sep 2026 21:08:13 +0000 Subject: [PATCH] fix(tests): replace pipefail-racy `echo | grep -q` with here-strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why Two suites failed intermittently — tests/test-vale-wrap.sh case 21 and tests/test-check-release-needed.sh cases 4 and 15 — on correct output, and never when run alone. The cause is the `echo "$OUT" | grep -q P` idiom under `set -o pipefail`: grep -q exits as soon as it has an answer, bash's echo can hand a multi-line value to the pipe one line at a time, and a write after the reader is gone kills echo with SIGPIPE. pipefail then reports the writer's death, so output that DID match reads as "no match". Every observed failure had lines after its match; case 15's match is on line 1 of 6, the widest window in that file. Forced with a pause before the writer's last line, the pipe form failed 50 of 50 runs; a here-string, a match on the last line, and the same pipe without pipefail each passed 50 of 50. Unforced the rate is about 1 per 670 suite runs, which is why it read as a flaky gate rather than a bug. The failures at review time are consistent with this, but were not proven to be it: the suite was running while agents edited live config files in place, and a brief change to .vale.ini or .pre-commit-hooks.yaml would produce the same two failures. The race is real and fixed either way. Implementation Notes `grep -q P <<< "$VAR"` has no separate writer process, so there is nothing to race. It is not a retry or a sleep. 121 sites converted across 9 files, three of them scripts rather than tests: new-agent.sh, new-skill.sh and check-executables-allow-sync.sh. None ships via .pre-commit-hooks.yaml, so no external consumer pins them, and all three are single-pipeline checks whose verdict cannot change. Left alone deliberately: 14 sites whose writer is a command, not a shell builtin — they either absorb the writer's status with `|| true` or are python3 and awk, which write once at exit — and one file with no pipefail. `printf '%s'` sites differ from a here-string only by a trailing newline, which no -q verdict on a non-empty pattern depends on. tests/test-no-pipefail-early-exit-grep.sh is a static guard against new occurrences, discovered automatically by run-tests.sh. It only scans files that set pipefail, joins continuation lines, skips comments, and flags only echo/printf writers. Its first case proves the scanner can fail before its second trusts a clean verdict on the tree. A guard covers exactly the spellings its regex models, so the miss surface was measured rather than assumed. Four were found and closed: pipefail declared as `set -o errexit -o pipefail` (where the old pattern required pipefail to follow the FIRST -o, and a file-level miss skips every site in that file); a writer separated from grep by an intermediate stage; a pipeline wrapped on a trailing `|` rather than a backslash; and readers spelled egrep, fgrep, /bin/grep, `command grep` or with an env-var prefix. Segment characters exclude a bare `&` so `echo ok && other | grep -q x`, whose writer is `other`, does not false-fire. Widening surfaced 5 live sites invisible to the original scanner, all in tests/test-apm-current-hook.sh, all `echo "$out" | json_field ... | grep -q`; they are safe today only because json_field is python3, which reads to EOF and writes once. Fixtures go 4 to 12 vulnerable spellings plus near-miss negatives. Two `grep ... | head -1` sites (test-vale-wrap.sh) are the same race with a different early-exiting reader, and are fixed by absorbing the writer. The scanner deliberately does not model `head`, `sed -n 1p` or a bare `read`: most legitimate uses in this tree are already absorbed with `|| true` and the scanner cannot see absorption from pipeline text, so a high false-positive rate would be how this guard gets weakened. Heredoc bodies are scanned as code; none in the tree trips it today. Impact The bug predates the factory-audit merge: every converted site in check-release-needed and case 21 dates to 4d018af and aa8cc22 (2026-08-09). Test suites go 19 to 20. `run-tests.sh --strict` passes 20/20 with 0 skipped, four consecutive runs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD --- LESSONS.md | 4 + SIMPLIFICATION-AUDIT.md | 2 +- docs/spec/gates.md | 10 + .../skills/agent-author/scripts/new-agent.sh | 2 +- .../skills/skill-author/scripts/new-skill.sh | 2 +- scripts/check-executables-allow-sync.sh | 2 +- tests/test-apm-current-hook.sh | 10 +- tests/test-check-release-needed.sh | 33 +-- tests/test-no-pipefail-early-exit-grep.sh | 221 ++++++++++++++++++ tests/test-run-bats.sh | 32 +-- tests/test-run-tests.sh | 52 ++--- tests/test-statusline.sh | 20 +- tests/test-vale-wrap.sh | 96 ++++---- 13 files changed, 365 insertions(+), 121 deletions(-) create mode 100755 tests/test-no-pipefail-early-exit-grep.sh diff --git a/LESSONS.md b/LESSONS.md index 265097a..d36be99 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -133,3 +133,7 @@ Widening a description-opener rule to also catch mid-sentence text looked like a ## 2026-08-16 — A rule reversed inside a retrofit leaves no trace unless someone writes it down A retrofit replaced "keep reference chains one level deep" with "two hops, never three" — the opposite rule, needed because the new dispatch pattern requires `SKILL.md` → `improve.md` → `retrofit.md`. The ADR never mentioned chain depth, so the reversal was carried entirely by the diff with no sign a contradicting rule ever existed. Fix: when a change inverts a standing rule, record the inversion where the rule's rationale lives, or it reads as forgotten rather than overturned. + +## 2026-09-15 — A rare flake in a pipefail suite is a race until proven otherwise + +The pre-push `run-tests` failed 2 of 3 full runs, on a different suite each time, and neither failure reproduced alone, so it was treated as noise. The cause was `echo "$OUT" | grep -q P` under `set -o pipefail`, at 116 sites. `grep -q` exits on its first match, `echo` takes SIGPIPE on its next write, and pipefail reports correct output as "no match". Unforced it failed about once in 670 runs; with a pause forced before the last line, 50 of 50. Fix: use `grep -q P <<< "$OUT"` (a here-string has no writer process to race), and add a static guard (`tests/test-no-pipefail-early-exit-grep.sh`) instead of relying on convention. diff --git a/SIMPLIFICATION-AUDIT.md b/SIMPLIFICATION-AUDIT.md index 056ede5..8148df9 100644 --- a/SIMPLIFICATION-AUDIT.md +++ b/SIMPLIFICATION-AUDIT.md @@ -83,7 +83,7 @@ Where the 276 s goes (each suite run alone, sequential): Five suites account for 215 s of 276 s. Three of those five (sync-plugin-content, vale-style-sync, adr0020-differential) test tooling that findings 2, 7, and 14 propose to delete or shrink, so the fastest path to a quick pre-push is removing the duplication those tests guard rather than optimising the tests. -> **Also struck (2026-09-15):** `test-check-vale-style-sync.sh` (25 s) went with the `check-vale-style-sync` hook in finding 14's merge (ADR-0025). Measured at HEAD: `tests/` holds **19** `test-*.sh` suites and `tests/run-tests.sh` reports `19 passed, 0 skipped, 0 failed`. Same basis as the note below — arithmetic on the 2026-09-10 baseline minus the struck rows, not a fresh timing run. +> **Also struck (2026-09-15):** `test-check-vale-style-sync.sh` (25 s) went with the `check-vale-style-sync` hook in finding 14's merge (ADR-0025). Measured at HEAD: `tests/` holds **19** `test-*.sh` suites and `tests/run-tests.sh` reports `19 passed, 0 skipped, 0 failed`. (Later the same day, the pipefail-race fix added `tests/test-no-pipefail-early-exit-grep.sh`, making it **20**. That suite is a static scan and runs in well under a second, so the timing arithmetic here is unaffected.) Same basis as the note below — arithmetic on the 2026-09-10 baseline minus the struck rows, not a fresh timing run. > **Done (2026-09-14):** see commit `718c79a` on `docs/simplification-audit`. The three struck-through rows are gone: `test-sync-plugin-content.sh` (83 s, 1,289 lines, 92 cases), `check-plugin-content-sync` (4.5 s) and `validate-plugins` (4.9 s). Expected, not re-measured: roughly 92 s comes off every push (83 + 4.5 + 4.9 = 92.4 s) (~83 s of it out of `run-tests`, which loses its single slowest suite), on the arithmetic of the 2026-09-10 figures alone. The remaining rows have not been re-timed since, so treat every number in this section as the 2026-09-10 baseline minus those three, not as a fresh measurement. diff --git a/docs/spec/gates.md b/docs/spec/gates.md index 1e69e2a..3c4ea36 100644 --- a/docs/spec/gates.md +++ b/docs/spec/gates.md @@ -862,6 +862,16 @@ vale-less PATH, with the skip list swallowed. Without vale, two suites skip — what to install. (It was three until `test-check-vale-style-sync.sh` was deleted with its hook; see [One copy, one config](#one-copy-one-config).) +**Output assertions use a here-string, never a pipe.** Write `grep -q PATTERN <<< "$OUT"`, not +`echo "$OUT" | grep -q PATTERN`. Under `set -o pipefail` the pipe form fails depending on timing: +`grep -q` exits on its first match, `echo` takes SIGPIPE on its next write, and pipefail reports that +as the pipeline failing, so output that matched reads as "no match". It showed up as a push gate that +failed about once in 670 runs, on a different suite each time. `tests/test-no-pipefail-early-exit-grep.sh` +scans every tracked shell file that sets pipefail and fails on the pipe form. It does this for `echo` +or `printf` piped into `grep` with `-q`, `-m`, `-l`, `-L`, `--quiet` or `--silent`. It checks its own +scanner against fixtures before trusting a clean result. Pipes from other commands are out of scope. +In practice they either absorb the writer's exit status with `|| true` or write only once, at exit. + `tests/run-bats.sh` derives the set of `.bats` files it expects from `git ls-files`, so a `.bats` file deleted from the worktree but still tracked in the index fails the run rather than silently shrinking the suite. Remove one with `git rm` (or stage the deletion) when intentional; an untracked diff --git a/plugins/kyberforge/.apm/skills/agent-author/scripts/new-agent.sh b/plugins/kyberforge/.apm/skills/agent-author/scripts/new-agent.sh index 83b6a2e..f7df41a 100755 --- a/plugins/kyberforge/.apm/skills/agent-author/scripts/new-agent.sh +++ b/plugins/kyberforge/.apm/skills/agent-author/scripts/new-agent.sh @@ -61,7 +61,7 @@ AGENT_NAME="$1" ROOT="$2" # Validate agent name format -if ! echo "$AGENT_NAME" | grep -qE '^[a-z0-9]+(-[a-z0-9]+)*$'; then +if ! grep -qE '^[a-z0-9]+(-[a-z0-9]+)*$' <<< "$AGENT_NAME"; then echo "Error: agent-name must use lowercase letters, numbers, and hyphens only." >&2 echo " No leading, trailing, or consecutive hyphens." >&2 echo " Received: '$AGENT_NAME'" >&2 diff --git a/plugins/kyberforge/.apm/skills/skill-author/scripts/new-skill.sh b/plugins/kyberforge/.apm/skills/skill-author/scripts/new-skill.sh index 1a39fe6..0d4c9c2 100755 --- a/plugins/kyberforge/.apm/skills/skill-author/scripts/new-skill.sh +++ b/plugins/kyberforge/.apm/skills/skill-author/scripts/new-skill.sh @@ -61,7 +61,7 @@ SKILL_NAME="$1" TARGET_INPUT="$2" # Validate skill name format -if ! echo "$SKILL_NAME" | grep -qE '^[a-z0-9]+(-[a-z0-9]+)*$'; then +if ! grep -qE '^[a-z0-9]+(-[a-z0-9]+)*$' <<< "$SKILL_NAME"; then echo "Error: skill-name must use lowercase letters, numbers, and hyphens only." >&2 echo " No leading, trailing, or consecutive hyphens." >&2 echo " Received: '$SKILL_NAME'" >&2 diff --git a/scripts/check-executables-allow-sync.sh b/scripts/check-executables-allow-sync.sh index 4c9daff..51f46c0 100755 --- a/scripts/check-executables-allow-sync.sh +++ b/scripts/check-executables-allow-sync.sh @@ -202,7 +202,7 @@ if [[ "$ALLOW_STATE" != "present" ]]; then exit 1 fi -if printf '%s\n' "$ALLOW_KEYS" | grep -qxF "$EXPECTED_KEY"; then +if grep -qxF "$EXPECTED_KEY" <<< "$ALLOW_KEYS"; then exit 0 fi diff --git a/tests/test-apm-current-hook.sh b/tests/test-apm-current-hook.sh index 631270d..8e1acb2 100755 --- a/tests/test-apm-current-hook.sh +++ b/tests/test-apm-current-hook.sh @@ -114,11 +114,11 @@ if echo "$out" | python3 -m json.tool > /dev/null 2>&1; then && pass "declares hookEventName SessionStart" || fail "wrong hookEventName" [[ "$(echo "$out" | json_field reloadSkills)" == "True" ]] \ && pass "asks the host to reload skills after a successful refresh" || fail "reloadSkills should be true" - echo "$out" | json_field additionalContext | grep -q "6 package" \ + grep -q "6 package" <<< "$(json_field additionalContext <<< "$out")" \ && pass "reports the stale package count" || fail "should report the count" # The lockfile rewrite is the surprising part of auto-updating; the notice has # to say so or a dirty worktree looks like something else went wrong. - echo "$out" | json_field additionalContext | grep -q "apm.lock.yaml" \ + grep -q "apm.lock.yaml" <<< "$(json_field additionalContext <<< "$out")" \ && pass "warns that apm.lock.yaml was rewritten" || fail "should warn about the lockfile rewrite" else fail "emits valid JSON" @@ -136,7 +136,7 @@ if echo "$out" | python3 -m json.tool > /dev/null 2>&1; then pass "emits valid JSON on failure" [[ "$(echo "$out" | json_field reloadSkills)" == "False" ]] \ && pass "does not ask for a skill reload when nothing was deployed" || fail "reloadSkills should be false" - echo "$out" | json_field additionalContext | grep -q "apm update" \ + grep -q "apm update" <<< "$(json_field additionalContext <<< "$out")" \ && pass "tells the reader how to refresh by hand" || fail "should name the manual command" else fail "emits valid JSON on failure" @@ -175,7 +175,7 @@ out="$(run_hook_in "$ELSEWHERE" "$WORK")" [[ "$(cat "$WORK/apm-cwd" 2>/dev/null)" == "$WORK" ]] \ && pass "runs apm in the directory the guard checked, not the cwd" \ || fail "apm ran in '$(cat "$WORK/apm-cwd" 2>/dev/null)' — must run in the resolved project directory" -echo "$out" | json_field additionalContext | grep -q "6 package" \ +grep -q "6 package" <<< "$(json_field additionalContext <<< "$out")" \ && pass "reports the count found via CLAUDE_PROJECT_DIR" || fail "should report the count" # The fallback is not cosmetic: a host that installed this plugin natively sets @@ -348,7 +348,7 @@ EOF chmod +x "$FAKE_BIN/apm" rm -f "$WORK/update-was-called" out="$(run_hook)" - if [[ -n "$out" ]] && echo "$out" | json_field additionalContext 2>/dev/null | grep -q "$want package"; then + if [[ -n "$out" ]] && grep -q "$want package" <<< "$(json_field additionalContext 2>/dev/null <<< "$out")"; then pass "detects staleness in real \`apm outdated\` output and counts $want package(s)" else fail "real apm reported $want outdated dependency/dependencies but the hook did not act on it — apm reworded its summary line. Real output: $(tr '\n' ' ' < "$PROBE/genuine-$want.txt" | tail -c 120)" diff --git a/tests/test-check-release-needed.sh b/tests/test-check-release-needed.sh index 6e1afb2..2a321c6 100755 --- a/tests/test-check-release-needed.sh +++ b/tests/test-check-release-needed.sh @@ -9,6 +9,13 @@ FAIL=0 pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } +# Output assertions are `grep -q PATTERN <<< "$OUT"`, never `echo "$OUT" | grep -q`. +# Under pipefail the pipe form is scheduling-dependent: bash's echo writes a +# multi-line value one line at a time, `grep -q` exits on its first match, and a +# later line then hits a closed pipe. echo dies of SIGPIPE, pipefail reports the +# pipeline as failed, and a correct output reads as a missing match. The +# here-string has no writer process to race. + # Both entry shapes the real .pre-commit-hooks.yaml ships: a bare script with no # bundled data, and a bare script whose sibling assets/ tree it self-locates at # runtime. Neither carries arguments — pre-commit only rewrites entry[0] to the @@ -142,7 +149,7 @@ FIXTURE4="$(make_tagged_fixture)"; track "$FIXTURE4" echo "v2" > "$FIXTURE4/scripts/skill-size-check.sh" (cd "$FIXTURE4" && git add -A && git commit -q -m "update release-relevant script") OUT4=$(run_check "$FIXTURE4" "refs/heads/main" || true) -if echo "$OUT4" | grep -q "skill-size-check.sh"; then +if grep -q "skill-size-check.sh" <<< "$OUT4"; then pass "exits non-zero and names the changed file when a release-relevant path changed since the tag" else fail "did not flag the release-relevant file that changed since the tag" @@ -167,7 +174,7 @@ FIXTURE6="$(make_tagged_fixture)"; track "$FIXTURE6" rm -f "$FIXTURE6/$HOOK_DIR/assets/vale/.vale.ini" (cd "$FIXTURE6" && git add -A && git commit -q -m "delete the bundled vale config") OUT6=$(run_check "$FIXTURE6" "refs/heads/main" || true) -if echo "$OUT6" | grep -q "assets/vale/.vale.ini"; then +if grep -q "assets/vale/.vale.ini" <<< "$OUT6"; then pass "flags a deleted release-relevant path instead of silently dropping it from the diff" else fail "did not flag deletion of a release-relevant path since the tag" @@ -196,7 +203,7 @@ echo "checkpoint" > "$FIXTURE8/scripts/skill-size-check.sh" echo "v2" > "$FIXTURE8/scripts/skill-size-check.sh" (cd "$FIXTURE8" && git add -A && git commit -q -m "real release-relevant change") OUT8=$(run_check "$FIXTURE8" "refs/heads/main" || true) -if echo "$OUT8" | grep -q "skill-size-check.sh"; then +if grep -q "skill-size-check.sh" <<< "$OUT8"; then pass "still flags the release-relevant change since v1.0.0, ignoring the non-version checkpoint tag" else fail "an incidental non-version tag shifted the baseline and hid a real release-relevant change" @@ -225,7 +232,7 @@ FIXTURE10="$(make_tagged_fixture)"; track "$FIXTURE10" echo "rule: v2" > "$FIXTURE10/$HOOK_DIR/assets/vale/styles/Kyberforge/DemoRule.yml" (cd "$FIXTURE10" && git add -A && git commit -q -m "tighten a vale rule") OUT10=$(run_check "$FIXTURE10" "refs/heads/main" || true) -if echo "$OUT10" | grep -q "assets/vale/styles/Kyberforge/DemoRule.yml"; then +if grep -q "assets/vale/styles/Kyberforge/DemoRule.yml" <<< "$OUT10"; then pass "flags a change confined to a hook's bundled assets/vale/styles/ tree" else fail "a bundled Vale style rule changed since the tag without demanding a release" @@ -259,7 +266,7 @@ FIXTURE12="$(make_tagged_fixture)"; track "$FIXTURE12" rm -rf "${FIXTURE12:?}/$HOOK_DIR/assets" (cd "$FIXTURE12" && git add -A && git commit -q -m "delete the whole bundled assets tree") OUT12=$(run_check "$FIXTURE12" "refs/heads/main" || true) -if echo "$OUT12" | grep -q "assets/vale/.vale.ini"; then +if grep -q "assets/vale/.vale.ini" <<< "$OUT12"; then pass "flags a wholesale deletion of a hook's bundled assets/ tree" else fail "a hook's entire bundled assets/ tree vanished since the tag without demanding a release" @@ -276,7 +283,7 @@ FIXTURE13="$(make_tagged_fixture)"; track "$FIXTURE13" rm -f "$FIXTURE13/$HOOK_DIR/scripts/vale-wrap.sh" (cd "$FIXTURE13" && git add -A && git commit -q -m "delete a hook script, keep its manifest entry") OUT13=$(run_check "$FIXTURE13" "refs/heads/main" || true) -if echo "$OUT13" | grep -q "vale-wrap.sh"; then +if grep -q "vale-wrap.sh" <<< "$OUT13"; then pass "flags a hook script deleted out from under a surviving manifest entry" else fail "a manifest entry's script vanished since the tag without demanding a release" @@ -299,7 +306,7 @@ EOF rm -rf "${FIXTURE14:?}/$HOOK_DIR" (cd "$FIXTURE14" && git add -A && git commit -q -m "retire the vale hook entirely") OUT14=$(run_check "$FIXTURE14" "refs/heads/main" || true) -if echo "$OUT14" | grep -q "vale-wrap.sh" && echo "$OUT14" | grep -q "assets/vale/.vale.ini"; then +if grep -q "vale-wrap.sh" <<< "$OUT14" && grep -q "assets/vale/.vale.ini" <<< "$OUT14"; then pass "names the retired hook's script and bundled assets, not just the manifest edit" else fail "reported only the manifest change and hid which shipped paths the retirement removed" @@ -320,9 +327,9 @@ FIXTURE15="$(make_malformed_fixture "bash scripts/skill-size-check.sh")"; track OUT15=$(run_check "$FIXTURE15" "refs/heads/main" || true) if run_check "$FIXTURE15" "refs/heads/main" > /dev/null; then fail "silently exited 0 on a multi-token entry, dropping that hook's paths from the gate" -elif echo "$OUT15" | grep -q "fake-size-check" \ - && echo "$OUT15" | grep -q "bash scripts/skill-size-check.sh" \ - && echo "$OUT15" | grep -q "ADR-0014"; then +elif grep -q "fake-size-check" <<< "$OUT15" \ + && grep -q "bash scripts/skill-size-check.sh" <<< "$OUT15" \ + && grep -q "ADR-0014" <<< "$OUT15"; then pass "rejects a multi-token entry, quoting it back and naming the hook and ADR-0014" else fail "rejected the multi-token entry without naming the hook, the entry, and ADR-0014" @@ -339,7 +346,7 @@ FIXTURE16="$(make_malformed_fixture "vale")"; track "$FIXTURE16" OUT16=$(run_check "$FIXTURE16" "refs/heads/main" || true) if run_check "$FIXTURE16" "refs/heads/main" > /dev/null; then fail "silently exited 0 on an entry that names no shipped file" -elif echo "$OUT16" | grep -q "fake-size-check" && echo "$OUT16" | grep -q "ADR-0014"; then +elif grep -q "fake-size-check" <<< "$OUT16" && grep -q "ADR-0014" <<< "$OUT16"; then pass "rejects an entry that resolves to no file, naming the hook and the ADR-0014 constraint" else fail "rejected the unresolvable entry without naming the hook and the ADR-0014 constraint" @@ -358,7 +365,7 @@ echo "v2" > "$FIXTURE17/scripts/skill-size-check.sh" (cd "$FIXTURE17" && git add -A && git commit -q -m "release-relevant change" \ && git branch pushed-tip && git reset -q --hard v1.0.0) OUT17=$(run_check "$FIXTURE17" "refs/heads/main" "pushed-tip" || true) -if echo "$OUT17" | grep -q "skill-size-check.sh"; then +if grep -q "skill-size-check.sh" <<< "$OUT17"; then pass "gates the pushed ref's tip, not HEAD, when HEAD is behind it" else fail "diffed HEAD instead of PRE_COMMIT_TO_REF and missed a release-relevant change" @@ -431,7 +438,7 @@ FIXTURE21="$(make_tagged_fixture)"; track "$FIXTURE21" echo "v2" > "$FIXTURE21/scripts/skill-size-check.sh" (cd "$FIXTURE21" && git add -A && git commit -q -m "real release-relevant change" && git tag v1.0.1-checkpoint) OUT21=$(run_check "$FIXTURE21" "refs/heads/main" || true) -if echo "$OUT21" | grep -q "skill-size-check.sh"; then +if grep -q "skill-size-check.sh" <<< "$OUT21"; then pass "still flags the release-relevant change since v1.0.0, ignoring the vX.Y.Z-checkpoint tag" else fail "a vX.Y.Z-checkpoint tag satisfied the glob and hid a real release-relevant change" diff --git a/tests/test-no-pipefail-early-exit-grep.sh b/tests/test-no-pipefail-early-exit-grep.sh new file mode 100755 index 0000000..fe44666 --- /dev/null +++ b/tests/test-no-pipefail-early-exit-grep.sh @@ -0,0 +1,221 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Static guard: no `echo|printf ... | grep -q` pipeline in a script that runs +# under `set -o pipefail`. +# +# The defect. grep -q (and -m, -l, -L, --quiet, --silent) exits as soon as it +# has an answer. If the writer on the left of the pipe still has output to +# write, that next write hits a closed pipe and the writer dies of SIGPIPE. +# Under pipefail the pipeline then reports the WRITER's failure, so output that +# matched reads as "no match". Whether it happens depends on scheduling: bash's +# echo can hand a multi-line value to the pipe a line at a time, and a reader +# that matched on an early line exits before the later lines arrive. Forced with +# a pause before the last line, the pipe form failed 50 of 50 runs; each of a +# here-string, a match on the last line, and the same pipe without pipefail +# passed 50 of 50. Unforced it surfaced about once per 670 suite runs, which is +# why it read as a flaky gate rather than as a bug. +# +# The fix is `grep -q PATTERN <<< "$VAR"`: a here-string has no separate writer +# process, so there is nothing to race. `printf '%s\n' "$X" | grep` and +# `<<< "$X"` feed grep the same bytes; `printf '%s'` and `echo` differ only by a +# trailing newline, which no -q verdict on a non-empty pattern depends on. +# +# Scope. Only echo/printf writers are flagged: they are the shell builtins that +# can split a write, and they are always replaceable by a here-string. A +# command writer (`run_wrap ... | grep -q`) is not flagged; those sites either +# absorb the writer's status (`|| true`) or write once at exit. Files without +# pipefail are not flagged, because without it the pipeline's status is grep's. +# Comment-only lines are skipped so the pattern can be named in prose. +# +# Remit. This scanner models early-exiting GREP readers only — grep, egrep and +# fgrep, however they are spelled (a path prefix, a `command` prefix, env-var +# assignments), anywhere in the pipeline. They are not the only readers that +# exit early: `head`, `sed -n 1p` and a bare `read` do too, and an echo/printf +# writer feeding any of them is the same race. Those sites are guarded by +# convention instead — absorb the writer's status with `|| true` (or take the +# verdict from a here-string) — and deliberately not by this test, because most +# legitimate uses of them in this tree are already absorbed and the scanner +# cannot see the absorption from the pipeline text alone. Flagging them would be +# noise; the grep readers are flagged because a here-string is always available. +# +# Known limitation: heredoc bodies are scanned as code, so a `cat <<'EOF'` body +# containing a vulnerable-looking line reads as a real site. There are none in +# the tree today. +# +# Case 1 proves the scanner can fail before case 2 trusts its clean verdict on +# the live tree. + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SELF="$REPO_ROOT/tests/$(basename "${BASH_SOURCE[0]}")" +PASS=0 +FAIL=0 + +pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } +fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } + +RUN_TMP="$(mktemp -d)" +trap 'rm -rf "$RUN_TMP"' EXIT + +# A pipeline-safe run of characters: anything but `|`, `;` or `&`, except that a +# `&` immediately followed by a digit is kept so `2>&1` does not end a segment. +# Excluding a bare `&` is what stops `echo ok && other | grep -q x` — a pipeline +# whose writer is `other`, not the echo — from being read as one site. +SAFE='([^|;&]|&[[:digit:]])' + +# Writer, then any number of intermediate stages, then an early-exiting grep. +# The reader may be spelled `grep`, `egrep` or `fgrep`, behind a path prefix +# (`/bin/grep`), a `command` prefix, or env-var assignments (`LC_ALL=C grep`). +SITE_RE="(^|[^[:alnum:]_])(echo|printf)[[:space:]]${SAFE}*[|][[:space:]]*" +SITE_RE+="([^|;&[:space:]]${SAFE}*[|][[:space:]]*)*" +SITE_RE+="((command|[A-Za-z_][A-Za-z0-9_]*=[^[:space:]]*)[[:space:]]+)*" +SITE_RE+="([^[:space:];&|]*/)?(grep|egrep|fgrep)[[:space:]]" +SITE_RE+="(${SAFE}*[[:space:]])?(-[[:alpha:]]*[qmlL][[:alnum:]]*|--quiet|--silent|--max-count|--files-with)" + +# `pipefail` need not follow the first `-o`: `set -o errexit -o pipefail` and +# `set -o posix -o pipefail` are the same setting. A file-level miss here skips +# every site in the file, so this is deliberately loose — the cost of scanning a +# file that does not set pipefail is nil, the cost of skipping one is total. +PIPEFAIL_RE='(^|[^[:alnum:]_-])set[[:space:]]+-[^;#]*pipefail' + +# Prints `path:line: text` for every vulnerable pipeline in the given files. +# Continued lines are joined first — both a trailing backslash and a trailing +# `|`, which is equally legal as a line continuation in a pipeline — so a reader +# on the next physical line is still seen; the reported line is where the +# command starts. +scan() { + local f + for f in "$@"; do + grep -Eq "$PIPEFAIL_RE" "$f" || continue + awk -v file="$f" -v re="$SITE_RE" ' + function flush( b) { + if (buf == "") return + if (buf !~ /^[[:space:]]*#/ && buf ~ re) print file ":" start ": " buf + buf = "" + } + { + if (buf == "") start = NR + line = $0 + if (line ~ /\\$/) { buf = buf substr(line, 1, length(line) - 1) " "; next } + if (line ~ /[|][[:space:]]*$/) { buf = buf line " "; next } + buf = buf line + flush() + } + END { flush() } + ' "$f" + done +} + +# --------------------------------------------------------------------------- +echo "--- the scanner flags each vulnerable spelling and nothing else ---" +# --------------------------------------------------------------------------- +BAD="$RUN_TMP/bad.sh" +cat > "$BAD" <<'EOF_BAD' +#!/usr/bin/env bash +set -euo pipefail +if echo "$OUT" | grep -q "needle"; then :; fi +! printf '%s\n' "$OUT" | grep -qF "needle" && : +if printf '%s' "$OUT" | grep -E -q "a|b"; then :; fi +if printf '%s\n' "$OUT" \ + | grep -m1 "needle"; then :; fi +if echo "$OUT" | tr -d ' ' | grep -q "needle"; then :; fi +echo "$OUT" | + grep -q "needle" && : +if echo "$OUT" | egrep -q "needle"; then :; fi +if printf '%s\n' "$OUT" | fgrep -q "needle"; then :; fi +if echo "$OUT" | /bin/grep -q "needle"; then :; fi +if echo "$OUT" | command grep -q "needle"; then :; fi +if echo "$OUT" | LC_ALL=C grep -q "needle"; then :; fi +if echo "$OUT" | sed -n '1,$p' | command /usr/bin/grep --quiet "needle"; then :; fi +EOF_BAD +ALTSET="$RUN_TMP/alt-pipefail.sh" +cat > "$ALTSET" <<'EOF_ALT' +#!/usr/bin/env bash +set -o errexit -o pipefail +if echo "$OUT" | grep -q "needle"; then :; fi +EOF_ALT +GOOD="$RUN_TMP/good.sh" +cat > "$GOOD" <<'EOF_GOOD' +#!/usr/bin/env bash +set -euo pipefail +# A comment naming the bad form: echo "$OUT" | grep -q needle +if grep -q "needle" <<< "$OUT"; then :; fi +COUNT="$(echo "$OUT" | grep -c "needle" || true)" +echo "$OUT" || grep -q "needle" <<< "$OUT" +run_wrap "$DIR" file.md | grep -q "needle" || : +printf '%s\n' "$OUT" | grep -c "needle" +echo "$OUT" | tail -n 1 +echo "$OUT" | tr -d ' ' | tail -n 1 +echo "$OUT" | mygrep -q "needle" +echo "$OUT" | command tail -n 1 +echo "$OUT" | LC_ALL=C sort +echo "$OUT" | + tail -n 1 +echo ok && run_wrap "$DIR" | grep -q "needle" +EOF_GOOD +NOPIPEFAIL="$RUN_TMP/no-pipefail.sh" +cat > "$NOPIPEFAIL" <<'EOF_NOPF' +#!/usr/bin/env bash +set -eu +if echo "$OUT" | grep -q "needle"; then :; fi +EOF_NOPF + +BAD_HITS="$(scan "$BAD")" +BAD_COUNT="$(grep -c . <<< "$BAD_HITS" || true)" +if [[ "$BAD_COUNT" -eq 12 ]]; then + pass "all 12 vulnerable spellings are flagged (echo, negated printf, split options, both continuations, an intermediate stage, egrep/fgrep, and path/command/env-assignment reader prefixes)" +else + fail "expected 12 hits in the bad fixture, got $BAD_COUNT: $BAD_HITS" +fi +if grep -q "^$BAD:6: " <<< "$BAD_HITS"; then + pass "a backslash-continued pipe is reported at the line the command starts" +else + fail "the backslash-continued site was not reported at line 6: $BAD_HITS" +fi +if grep -q "^$BAD:9: " <<< "$BAD_HITS"; then + pass "a pipeline wrapped after a trailing '|' is reported at the line the command starts" +else + fail "the trailing-pipe continuation site was not reported at line 9: $BAD_HITS" +fi +ALT_HITS="$(scan "$ALTSET")" +if [[ -n "$ALT_HITS" ]]; then + pass "pipefail set as 'set -o errexit -o pipefail' is recognised, so its sites are scanned" +else + fail "a file setting pipefail after another -o option was skipped entirely" +fi +GOOD_HITS="$(scan "$GOOD")" +if [[ -z "$GOOD_HITS" ]]; then + pass "here-strings, comments, grep -c, '||', '&&', command writers and non-grep readers are left alone" +else + fail "the clean fixture was flagged: $GOOD_HITS" +fi +if [[ -z "$(scan "$NOPIPEFAIL")" ]]; then + pass "a file without pipefail is not flagged — grep's own status is the pipeline's" +else + fail "a file without pipefail was flagged" +fi + +# --------------------------------------------------------------------------- +echo "--- no tracked shell script carries the race ---" +# --------------------------------------------------------------------------- +FILES=() +while IFS= read -r rel; do + [[ "$REPO_ROOT/$rel" == "$SELF" ]] && continue + [[ -f "$REPO_ROOT/$rel" ]] && FILES+=("$REPO_ROOT/$rel") +done < <(git -C "$REPO_ROOT" ls-files -- '*.sh' '*.bats' '*.bash') + +if [[ "${#FILES[@]}" -lt 20 ]]; then + fail "only ${#FILES[@]} tracked shell files found — the scan is looking in the wrong place" +else + LIVE_HITS="$(scan ${FILES[@]+"${FILES[@]}"})" + if [[ -z "$LIVE_HITS" ]]; then + pass "none of ${#FILES[@]} tracked shell files pipes echo/printf into an early-exit grep under pipefail" + else + fail "use \`grep -q PATTERN <<< \"\$VAR\"\` instead at:" + echo "${LIVE_HITS//"$REPO_ROOT/"/ }" + fi +fi + +echo "" +echo "Results: $PASS passed, $FAIL failed" +[[ "$FAIL" -eq 0 ]] diff --git a/tests/test-run-bats.sh b/tests/test-run-bats.sh index 0763023..a2c87ef 100644 --- a/tests/test-run-bats.sh +++ b/tests/test-run-bats.sh @@ -112,7 +112,7 @@ EOF run_fake "$DIR1" if [[ $FAKE_RC -eq 0 ]]; then fail "a bats emitting nothing exited 0 — a total harness failure reported as a pass" -elif echo "$FAKE_OUT" | grep -q "no TAP output at all"; then +elif grep -q "no TAP output at all" <<< "$FAKE_OUT"; then pass "an empty TAP stream fails the run and names the broken harness" else fail "the run failed but not with the broken-harness message: $FAKE_OUT" @@ -134,9 +134,9 @@ EOF run_fake "$DIR2" if [[ $FAKE_RC -eq 0 ]]; then fail "every test declaring zero tests exited 0 — wholesale @test removal reported as a pass" -elif echo "$FAKE_OUT" | grep -q "declared 0 tests"; then +elif grep -q "declared 0 tests" <<< "$FAKE_OUT"; then pass "a plan-only TAP stream fails the run and names the removed tests" -elif echo "$FAKE_OUT" | grep -q "no TAP output at all"; then +elif grep -q "no TAP output at all" <<< "$FAKE_OUT"; then fail "a valid '1..0' plan was misreported as a broken harness — the two branches are not distinguished" else fail "the run failed but not with the no-tests-declared message: $FAKE_OUT" @@ -160,7 +160,7 @@ EOF run_fake "$DIR3" if [[ $FAKE_RC -ne 0 ]]; then fail "a healthy TAP stream was failed by the zero-count guard: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^4 tests, 0 failures$"; then +elif grep -q "^4 tests, 0 failures$" <<< "$FAKE_OUT"; then pass "two files reporting two passing tests each aggregate to 4 tests, 0 failures" else fail "a healthy TAP stream produced the wrong count: $FAKE_OUT" @@ -187,9 +187,9 @@ else run_fake "$DIR4" if [[ $FAKE_RC -ne 0 ]]; then fail "an empty .bats file beside a real one failed the run: $FAKE_OUT" - elif ! echo "$FAKE_OUT" | grep -q "^1\.\.0$"; then + elif ! grep -q "^1\.\.0$" <<< "$FAKE_OUT"; then fail "real bats did not emit '1..0' for an empty file, so the case-2 stub no longer matches it: $FAKE_OUT" - elif echo "$FAKE_OUT" | grep -q "^1 tests, 0 failures$"; then + elif grep -q "^1 tests, 0 failures$" <<< "$FAKE_OUT"; then pass "an empty .bats file contributes a '1..0' plan and the suite still passes" else fail "the real-bats run passed with an unexpected count: $FAKE_OUT" @@ -215,7 +215,7 @@ EOF run_fake "$DIR5" if [[ $FAKE_RC -eq 0 ]]; then fail "a 'not ok' TAP result exited 0 — a failing test reported as a pass because only the process status was consulted" -elif echo "$FAKE_OUT" | grep -q "^2 tests, 2 failures$"; then +elif grep -q "^2 tests, 2 failures$" <<< "$FAKE_OUT"; then pass "'not ok' lines fail the run even when every bats process exits 0" else fail "the run failed but with the wrong count: $FAKE_OUT" @@ -242,7 +242,7 @@ EOF run_fake "$DIR6" if [[ $FAKE_RC -eq 0 ]]; then fail "a bats process exiting 1 was reported as a pass because only the TAP text was consulted" -elif echo "$FAKE_OUT" | grep -q "^2 tests, 0 failures$"; then +elif grep -q "^2 tests, 0 failures$" <<< "$FAKE_OUT"; then pass "a non-zero bats exit fails the run even with no 'not ok' line to find" else fail "the run failed but with the wrong count: $FAKE_OUT" @@ -282,7 +282,7 @@ run_fake "$DIR7" # the stated reason: the TAP stream the stub emitted is healthy, so "1 tests, 0 # failures" proves the zero-count guard did not fire and the empty status is the # only thing left that can have failed the run. -if ! echo "$FAKE_OUT" | grep -q "^1 tests, 0 failures$"; then +if ! grep -q "^1 tests, 0 failures$" <<< "$FAKE_OUT"; then fail "the killed job did not leave the healthy TAP stream the case needs: $FAKE_OUT" elif [[ $FAKE_RC -eq 0 ]]; then fail "an empty status file was counted as a clean exit — a killed job reported as a pass" @@ -312,9 +312,9 @@ rm "$DIR8/tests/b.bats" run_fake "$DIR8" if [[ $FAKE_RC -eq 0 ]]; then fail "a tracked .bats file gone from the worktree passed — a deleted suite reads as green" -elif ! echo "$FAKE_OUT" | grep -q "tracked .bats file(s) were not discovered"; then +elif ! grep -q "tracked .bats file(s) were not discovered" <<< "$FAKE_OUT"; then fail "the run failed but not with the undiscovered-tracked-file message: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^ tests/b.bats$"; then +elif grep -q "^ tests/b.bats$" <<< "$FAKE_OUT"; then pass "a tracked .bats file missing from the walk fails the run and names the file" else fail "the run failed without naming the missing file: $FAKE_OUT" @@ -339,7 +339,7 @@ git -C "$DIR8B" add tests/a.bats run_fake "$DIR8B" if [[ $FAKE_RC -ne 0 ]]; then fail "an untracked .bats file was reported as a finding: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^2 tests, 0 failures$"; then +elif grep -q "^2 tests, 0 failures$" <<< "$FAKE_OUT"; then pass "an untracked .bats file is run without being demanded of the index" else fail "the untracked-file run passed with the wrong count: $FAKE_OUT" @@ -356,7 +356,7 @@ rm "$DIR8B/tests/b.bats" run_fake "$DIR8B" if [[ $FAKE_RC -eq 0 ]]; then fail "the .bats file added to the index a moment ago was not demanded back: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^ tests/b.bats$"; then +elif grep -q "^ tests/b.bats$" <<< "$FAKE_OUT"; then pass "a file added to the index joins the expected set with no floor to bump" else fail "the run failed but did not name the newly tracked file: $FAKE_OUT" @@ -380,9 +380,9 @@ EOF run_fake "$DIR8D" if [[ $FAKE_RC -ne 0 ]]; then fail "a non-git tree failed the run: $FAKE_OUT" -elif ! echo "$FAKE_OUT" | grep -q "not a git worktree root"; then +elif ! grep -q "not a git worktree root" <<< "$FAKE_OUT"; then fail "a non-git tree silently skipped the derived expectation with no note: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^2 tests, 0 failures$"; then +elif grep -q "^2 tests, 0 failures$" <<< "$FAKE_OUT"; then pass "a non-git tree runs the suite and says the expected set could not be derived" else fail "the non-git run passed with the wrong count: $FAKE_OUT" @@ -404,7 +404,7 @@ EOF run_fake "$DIR9" if [[ $FAKE_RC -eq 0 ]]; then fail "finding no .bats files at all exited 0 — the whole suite can vanish and the run stays green" -elif echo "$FAKE_OUT" | grep -q "found 0 .bats file"; then +elif grep -q "found 0 .bats file" <<< "$FAKE_OUT"; then pass "finding no .bats files fails the run and says so" else fail "the run failed but not with the zero-files message: $FAKE_OUT" diff --git a/tests/test-run-tests.sh b/tests/test-run-tests.sh index 1131f6b..7d84cbd 100644 --- a/tests/test-run-tests.sh +++ b/tests/test-run-tests.sh @@ -115,9 +115,9 @@ EOF run_fake "$DIR1" if [[ $FAKE_RC -ne 0 ]]; then fail "a healthy fixture failed: $FAKE_OUT" -elif ! echo "$FAKE_OUT" | grep -q "^=== bats ===$"; then +elif ! grep -q "^=== bats ===$" <<< "$FAKE_OUT"; then fail "a healthy run never announced the bats leg: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^=== Summary: 1 passed, 0 skipped, 0 failed ===$"; then +elif grep -q "^=== Summary: 1 passed, 0 skipped, 0 failed ===$" <<< "$FAKE_OUT"; then pass "a passing case script and a healthy bats runner report 1 passed, 0 failed" else fail "a healthy run produced the wrong summary: $FAKE_OUT" @@ -140,7 +140,7 @@ EOF run_fake "$DIR2" if [[ $FAKE_RC -eq 0 ]]; then fail "a non-executable run-bats.sh exited 0 — the whole bats suite can vanish silently" -elif echo "$FAKE_OUT" | grep -q "bats runner not found or not executable"; then +elif grep -q "bats runner not found or not executable" <<< "$FAKE_OUT"; then pass "a non-executable run-bats.sh fails the run and names what is missing" else fail "the run failed but not with the missing-runner message: $FAKE_OUT" @@ -160,7 +160,7 @@ EOF run_fake "$DIR3" if [[ $FAKE_RC -eq 0 ]]; then fail "a missing run-bats.sh exited 0 — a rename deletes the bats suite from the run with no diagnostic" -elif echo "$FAKE_OUT" | grep -q "bats runner not found or not executable"; then +elif grep -q "bats runner not found or not executable" <<< "$FAKE_OUT"; then pass "a missing run-bats.sh fails the run and names what is missing" else fail "the run failed but not with the missing-runner message: $FAKE_OUT" @@ -180,7 +180,7 @@ EOF run_fake "$DIR4" --bats-only if [[ $FAKE_RC -eq 0 ]]; then fail "--bats-only with no runner exited 0 having printed nothing — a total no-op reported as a pass" -elif echo "$FAKE_OUT" | grep -q "bats runner not found or not executable"; then +elif grep -q "bats runner not found or not executable" <<< "$FAKE_OUT"; then pass "--bats-only fails when the runner is missing rather than doing nothing quietly" else fail "--bats-only failed but not with the missing-runner message: $FAKE_OUT" @@ -232,11 +232,11 @@ add_case "$DIR5B" test-ok.sh <<'EOF' echo "fine" EOF run_fake "$DIR5B" -if echo "$FAKE_OUT" | grep -q "^=== Summary: 1 passed, 0 skipped, 0 failed ===$"; then +if grep -q "^=== Summary: 1 passed, 0 skipped, 0 failed ===$" <<< "$FAKE_OUT"; then fail "an empty run-bats.sh produced a green summary — the bats suite vanished with no diagnostic" elif [[ $FAKE_RC -eq 0 ]]; then fail "an empty run-bats.sh exited 0: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "without reporting an 'N tests, M failures' summary"; then +elif grep -q "without reporting an 'N tests, M failures' summary" <<< "$FAKE_OUT"; then pass "an empty run-bats.sh fails the run and says the bats suite was never verified" else fail "the run failed but not with the no-summary message: $FAKE_OUT" @@ -264,7 +264,7 @@ EOF run_fake "$DIR5C" if [[ $FAKE_RC -eq 0 ]]; then fail "a bats runner reporting 0 tests exited 0 — a suite that executed nothing read as green: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "reported 0 tests"; then +elif grep -q "reported 0 tests" <<< "$FAKE_OUT"; then pass "a bats runner reporting 0 tests fails the run and says the suite executed nothing" else fail "the run failed but not with the zero-tests message: $FAKE_OUT" @@ -298,11 +298,11 @@ kill -9 $PPID sleep 5 EOF run_fake "$DIR6" -if echo "$FAKE_OUT" | grep -q "^=== Summary: 1 passed, 0 skipped, 0 failed ===$"; then +if grep -q "^=== Summary: 1 passed, 0 skipped, 0 failed ===$" <<< "$FAKE_OUT"; then fail "an empty status file counted as a pass — a killed job reads as green" elif [[ $FAKE_RC -eq 0 ]]; then fail "an empty status file did not fail the run: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^=== Summary: 0 passed, 0 skipped, 1 failed ===$"; then +elif grep -q "^=== Summary: 0 passed, 0 skipped, 1 failed ===$" <<< "$FAKE_OUT"; then pass "an empty status file is counted as FAILED" else fail "an empty status file failed the run with the wrong summary: $FAKE_OUT" @@ -326,7 +326,7 @@ EOF run_fake "$DIR7" if [[ $FAKE_RC -eq 0 ]]; then fail "a SIGKILLed job did not fail the run: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^=== Summary: 0 passed, 0 skipped, 1 failed ===$"; then +elif grep -q "^=== Summary: 0 passed, 0 skipped, 1 failed ===$" <<< "$FAKE_OUT"; then pass "a job killed with no status file written is counted as FAILED" else fail "a SIGKILLed job failed the run with the wrong summary: $FAKE_OUT" @@ -348,7 +348,7 @@ EOF run_fake "$DIR8" if [[ $FAKE_RC -eq 0 ]]; then fail "a case script with a syntax error did not fail the run: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^=== Summary: 0 passed, 0 skipped, 1 failed ===$"; then +elif grep -q "^=== Summary: 0 passed, 0 skipped, 1 failed ===$" <<< "$FAKE_OUT"; then pass "a case script that fails to parse is counted as FAILED" else fail "a syntax error failed the run with the wrong summary: $FAKE_OUT" @@ -380,11 +380,11 @@ EOF run_fake "$DIR9" if [[ $FAKE_RC -eq 0 ]]; then fail "a case script exiting 1 did not fail the run: $FAKE_OUT" -elif ! echo "$FAKE_OUT" | grep -q "^=== Summary: 1 passed, 1 skipped, 1 failed ===$"; then +elif ! grep -q "^=== Summary: 1 passed, 1 skipped, 1 failed ===$" <<< "$FAKE_OUT"; then fail "the pass/skip/fail split was miscounted: $FAKE_OUT" -elif ! echo "$FAKE_OUT" | grep -q "^ test-b-skips.sh$"; then +elif ! grep -q "^ test-b-skips.sh$" <<< "$FAKE_OUT"; then fail "the skipped script was not named in the skip list: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^ test-a-fails.sh$"; then +elif grep -q "^ test-a-fails.sh$" <<< "$FAKE_OUT"; then pass "exit 1 is FAILED, exit 77 is SKIPPED, and both are named in their lists" else fail "the failed script was not named in the failure list: $FAKE_OUT" @@ -422,11 +422,11 @@ EOF run_fake "$DIR10" --strict if [[ $FAKE_RC -eq 0 ]]; then fail "--strict passed with a skipped suite — the gate reports green having verified less than it ran: $FAKE_OUT" -elif ! echo "$FAKE_OUT" | grep -q "a skip is a SETUP ERROR"; then +elif ! grep -q "a skip is a SETUP ERROR" <<< "$FAKE_OUT"; then fail "--strict failed but never said a skip is a setup error: $FAKE_OUT" -elif ! echo "$FAKE_OUT" | grep -q "test-needs-a-binary.sh"; then +elif ! grep -q "test-needs-a-binary.sh" <<< "$FAKE_OUT"; then fail "--strict failed without naming the skipped suite: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^ SKIP: frobnicator is not installed"; then +elif grep -q "^ SKIP: frobnicator is not installed" <<< "$FAKE_OUT"; then pass "--strict fails on a skip, names the suite, and carries through the reason it printed" else fail "--strict named the suite but swallowed its skip reason: $FAKE_OUT" @@ -445,7 +445,7 @@ STRICT_ENV_OUT="$(TMPDIR="$STRICT_ENV_PRIV" TEST_DIR="$DIR10/cases" RUN_TESTS_ST bash "$DIR10/tests/run-tests.sh" 2>&1)" || STRICT_ENV_RC=$? if [[ $STRICT_ENV_RC -eq 0 ]]; then fail "RUN_TESTS_STRICT=1 passed with a skipped suite: $STRICT_ENV_OUT" -elif echo "$STRICT_ENV_OUT" | grep -q "a skip is a SETUP ERROR"; then +elif grep -q "a skip is a SETUP ERROR" <<< "$STRICT_ENV_OUT"; then pass "RUN_TESTS_STRICT=1 is the same gate as --strict" else fail "RUN_TESTS_STRICT=1 failed for some other reason: $STRICT_ENV_OUT" @@ -460,9 +460,9 @@ echo "--- the same skipped suite passes, still SKIPPED, without strict ---" run_fake "$DIR10" if [[ $FAKE_RC -ne 0 ]]; then fail "a skipped suite failed a non-strict run — graceful skipping is gone: $FAKE_OUT" -elif ! echo "$FAKE_OUT" | grep -q "^=== Summary: 1 passed, 1 skipped, 0 failed ===$"; then +elif ! grep -q "^=== Summary: 1 passed, 1 skipped, 0 failed ===$" <<< "$FAKE_OUT"; then fail "a non-strict run miscounted the skip: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^ SKIP: frobnicator is not installed"; then +elif grep -q "^ SKIP: frobnicator is not installed" <<< "$FAKE_OUT"; then pass "without strict the suite is SKIPPED, the run passes, and the reason is still reported" else fail "a non-strict run passed but dropped the skip reason: $FAKE_OUT" @@ -484,7 +484,7 @@ EOF run_fake "$DIR10D" --strict if [[ $FAKE_RC -ne 0 ]]; then fail "--strict failed a run with nothing skipped — it fails unconditionally: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^=== Summary: 1 passed, 0 skipped, 0 failed ===$"; then +elif grep -q "^=== Summary: 1 passed, 0 skipped, 0 failed ===$" <<< "$FAKE_OUT"; then pass "--strict leaves a run with no skips green" else fail "--strict passed with the wrong summary: $FAKE_OUT" @@ -506,7 +506,7 @@ EOF run_fake "$DIR10E" --strickt if [[ $FAKE_RC -eq 0 ]]; then fail "a misspelled flag was ignored and the run passed — a typo silently disarms the gate: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "Usage: .*--bats-only.*--strict"; then +elif grep -q "Usage: .*--bats-only.*--strict" <<< "$FAKE_OUT"; then pass "an unrecognised flag fails the run with usage" else fail "an unrecognised flag failed but not with usage: $FAKE_OUT" @@ -532,7 +532,7 @@ EOF run_fake "$DIR10F" --strict if [[ $FAKE_RC -eq 0 ]]; then fail "--strict passed on a suite that skipped via stderr: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^ widgetizer not installed -- skipping"; then +elif grep -q "^ widgetizer not installed -- skipping" <<< "$FAKE_OUT"; then pass "a skip reason printed to stderr without a SKIP: prefix is still carried into the failure" else fail "--strict failed but lost the stderr skip reason: $FAKE_OUT" @@ -570,9 +570,9 @@ run_fake "$DIR10" unset RUN_TESTS_STRICT if [[ $FAKE_RC -ne 0 ]]; then fail "an inherited RUN_TESTS_STRICT=1 turned a non-strict fixture strict — this suite's own result depends on how it was launched: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "a skip is a SETUP ERROR"; then +elif grep -q "a skip is a SETUP ERROR" <<< "$FAKE_OUT"; then fail "the fixture ran strict despite not asking for it: $FAKE_OUT" -elif echo "$FAKE_OUT" | grep -q "^=== Summary: 1 passed, 1 skipped, 0 failed ===$"; then +elif grep -q "^=== Summary: 1 passed, 1 skipped, 0 failed ===$" <<< "$FAKE_OUT"; then pass "an ambient RUN_TESTS_STRICT=1 is scrubbed from fixtures that did not ask for strict" else fail "the scrubbed run produced the wrong summary: $FAKE_OUT" diff --git a/tests/test-statusline.sh b/tests/test-statusline.sh index c5cd332..91e9a09 100755 --- a/tests/test-statusline.sh +++ b/tests/test-statusline.sh @@ -22,32 +22,32 @@ echo "--- assembly ---" out=$(run "$FULL") -echo "$out" | grep -q "myproject" && pass "shows basename of working directory" || fail "dir missing — got: $out" -echo "$out" | grep -q " · " && pass "segments joined with ' · '" || fail "separator missing — got: $out" -echo "$out" | grep -q "Sonnet 4.6" && pass "shows model name" || fail "model name missing — got: $out" -echo "$out" | grep -q "context 20%" && pass "shows context % with 'context' label" || fail "context label wrong — got: $out" +grep -q "myproject" <<< "$out" && pass "shows basename of working directory" || fail "dir missing — got: $out" +grep -q " · " <<< "$out" && pass "segments joined with ' · '" || fail "separator missing — got: $out" +grep -q "Sonnet 4.6" <<< "$out" && pass "shows model name" || fail "model name missing — got: $out" +grep -q "context 20%" <<< "$out" && pass "shows context % with 'context' label" || fail "context label wrong — got: $out" echo "" echo "--- token formatting ---" # 15k + 5k = 20k → shown as "20k tokens" -echo "$out" | grep -q "20k tokens" && pass "formats total tokens as Xk when >= 1000" || fail "Xk format wrong — got: $out" +grep -q "20k tokens" <<< "$out" && pass "formats total tokens as Xk when >= 1000" || fail "Xk format wrong — got: $out" # Sub-1000 total: 500 + 300 = 800 → shown as "800 tokens" LOW='{"workspace":{"current_dir":"/x"},"context_window":{"total_input_tokens":500,"total_output_tokens":300}}' out_low=$(run "$LOW") -echo "$out_low" | grep -q "800 tokens" && pass "shows raw count when total tokens < 1000" || fail "sub-1000 format wrong — got: $out_low" +grep -q "800 tokens" <<< "$out_low" && pass "shows raw count when total tokens < 1000" || fail "sub-1000 format wrong — got: $out_low" echo "" echo "--- cost formatting ---" # 0.25 → 25¢ -echo "$out" | grep -q "25¢" && pass "formats sub-dollar cost as cents (¢)" || fail "cents format wrong — got: $out" +grep -q "25¢" <<< "$out" && pass "formats sub-dollar cost as cents (¢)" || fail "cents format wrong — got: $out" # 1.71 → $1.71 DOLLAR='{"workspace":{"current_dir":"/x"},"cost":{"total_cost_usd":1.71},"context_window":{"total_input_tokens":0,"total_output_tokens":0}}' out_dollar=$(run "$DOLLAR") -echo "$out_dollar" | grep -qF '$1.71' && pass "formats dollar-plus cost as \$X.XX" || fail "dollar format wrong — got: $out_dollar" +grep -qF '$1.71' <<< "$out_dollar" && pass "formats dollar-plus cost as \$X.XX" || fail "dollar format wrong — got: $out_dollar" echo "" echo "--- missing fields omitted ---" @@ -55,12 +55,12 @@ echo "--- missing fields omitted ---" # No cost field → no ¢ or $ in output NO_COST='{"workspace":{"current_dir":"/x"},"context_window":{"total_input_tokens":0,"total_output_tokens":0}}' out_nocost=$(run "$NO_COST") -! echo "$out_nocost" | grep -qE '[¢$]' && pass "omits cost segment when cost absent" || fail "cost shown unexpectedly — got: $out_nocost" +! grep -qE '[¢$]' <<< "$out_nocost" && pass "omits cost segment when cost absent" || fail "cost shown unexpectedly — got: $out_nocost" # Zero tokens → no token segment ZERO_TOK='{"workspace":{"current_dir":"/x"},"context_window":{"total_input_tokens":0,"total_output_tokens":0}}' out_zerotok=$(run "$ZERO_TOK") -! echo "$out_zerotok" | grep -q "tokens" && pass "omits token segment when total is zero" || fail "token segment shown unexpectedly — got: $out_zerotok" +! grep -q "tokens" <<< "$out_zerotok" && pass "omits token segment when total is zero" || fail "token segment shown unexpectedly — got: $out_zerotok" echo "" echo "--- executable bit ---" diff --git a/tests/test-vale-wrap.sh b/tests/test-vale-wrap.sh index 7fd36fc..6a19f3f 100755 --- a/tests/test-vale-wrap.sh +++ b/tests/test-vale-wrap.sh @@ -246,12 +246,12 @@ EOF trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5"' EXIT OUT5=$(run_wrap "$FIXTURE5" --config "$VALE_CONFIG" \ plugins/testplugin/skills/zzzskill/SKILL.md) -if echo "$OUT5" | grep -q "VagueWording"; then +if grep -q "VagueWording" <<< "$OUT5"; then pass "flags vague wording when the description contains an apostrophe" else fail "silently missed vague wording in a description containing an apostrophe" fi -if echo "$OUT5" | grep -qi "yaml:"; then +if grep -qi "yaml:" <<< "$OUT5"; then fail "flattened copy with an apostrophe produced a YAML parse error" else pass "flattened copy with an apostrophe is valid YAML (no parse error)" @@ -297,9 +297,9 @@ EOF trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7"' EXIT OUT7=$(run_wrap "$FIXTURE7" --config "$VALE_CONFIG" \ plugins/testplugin/skills/zzzskill/SKILL.md) -if echo "$OUT7" | grep -q "Traceback"; then +if grep -q "Traceback" <<< "$OUT7"; then fail "crashed while flattening a description with a blank line between paragraphs" -elif echo "$OUT7" | grep -q "VagueWording"; then +elif grep -q "VagueWording" <<< "$OUT7"; then pass "still flags vague wording in the second paragraph after a blank line" else fail "silently missed vague wording in the second paragraph after a blank line — the bug this test guards against" @@ -337,7 +337,7 @@ REL_CFG8="../../../../.vale.ini" REL_FILE8="SKILL.md" OUT_EQ=$(run_wrap "$SUBDIR8" "--config=$REL_CFG8" "$REL_FILE8") OUT_TWO=$(run_wrap "$SUBDIR8" --config "$REL_CFG8" "$REL_FILE8") -if echo "$OUT_EQ" | grep -q "VagueWording" && [[ "$OUT_EQ" == "$OUT_TWO" ]]; then +if grep -q "VagueWording" <<< "$OUT_EQ" && [[ "$OUT_EQ" == "$OUT_TWO" ]]; then pass "cwd-relative --config resolves from a subdirectory in both argv forms" else fail "cwd-relative --config did not resolve from a subdirectory (equals form vs two-argv form)" @@ -355,7 +355,7 @@ RC_REL_CFG=$? OUT_ABS_CFG=$(cd "$SUBDIR8" && bash "$SCRIPT" --config "$FIXTURE8/.vale.ini" "$REL_FILE8" 2>&1) RC_ABS_CFG=$? set -e -if echo "$OUT_REL_CFG" | grep -qi "does not exist"; then +if grep -qi "does not exist" <<< "$OUT_REL_CFG"; then fail "cwd-relative --config hard-errored ('does not exist') — the bug this test guards against" elif [[ "$OUT_REL_CFG" == "$OUT_ABS_CFG" && "$RC_REL_CFG" -eq "$RC_ABS_CFG" ]]; then pass "cwd-relative --config matches the absolute-path invocation (output and exit code)" @@ -373,9 +373,9 @@ echo "" echo "--- flattens a cwd-relative file argument passed from a subdirectory ---" WRAPPED_REL=$(run_wrap "$SUBDIR8" --config "$FIXTURE8/.vale.ini" "$REL_FILE8") BARE_REL=$(cd "$SUBDIR8" && vale --config "$FIXTURE8/.vale.ini" "$REL_FILE8" 2>&1 || true) -if ! echo "$WRAPPED_REL" | grep -q "VagueWording"; then +if ! grep -q "VagueWording" <<< "$WRAPPED_REL"; then fail "cwd-relative file argument produced no alert — flattening was silently skipped, the bug this test guards against" -elif echo "$BARE_REL" | grep -q "VagueWording"; then +elif grep -q "VagueWording" <<< "$BARE_REL"; then fail "bare vale already flags this fixture, so the test can't detect a silently-skipped flattening" else pass "cwd-relative file argument is flattened and flagged where bare vale reports nothing" @@ -443,7 +443,7 @@ trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTU REL11="plugins/testplugin/skills/zzzskill/SKILL.md" WRAPPED_OUT=$(run_wrap "$FIXTURE11" --config "$VALE_CONFIG" "$REL11") BARE_OUT=$(cd "$FIXTURE11" && vale --config "$VALE_CONFIG" "$REL11" 2>&1 || true) -if ! echo "$BARE_OUT" | grep -q "VagueWording"; then +if ! grep -q "VagueWording" <<< "$BARE_OUT"; then fail "bare vale reports nothing for a literal (|) block scalar — the 'literal blocks are not broken' premise is wrong" elif [[ "$WRAPPED_OUT" == "$BARE_OUT" ]]; then pass "literal (|) block scalar output matches bare vale exactly — untouched by flattening" @@ -461,7 +461,7 @@ echo "--- defaults --config to the wrapper's own sibling assets/vale/.vale.ini - FIXTURE12="$(make_fixture 2)" trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12"' EXIT OUT12=$(run_wrap "$FIXTURE12" plugins/testplugin/skills/zzzskill/SKILL.md) -if echo "$OUT12" | grep -q "VagueWording"; then +if grep -q "VagueWording" <<< "$OUT12"; then pass "a --config-less invocation uses the wrapper's bundled config" else fail "a --config-less invocation found no config — external pre-commit consumers get E100, the bug this test guards against" @@ -490,9 +490,9 @@ EOF chmod +x "$STUB13/realpath" OUT13=$(cd "$FIXTURE12" && PATH="$STUB13:$PATH" bash "$SCRIPT" --config "$VALE_CONFIG" \ plugins/testplugin/skills/zzzskill/SKILL.md 2>&1 || true) -if echo "$OUT13" | grep -q "illegal option"; then +if grep -q "illegal option" <<< "$OUT13"; then fail "invoked realpath -m — fails on macOS's BSD realpath, the bug this test guards against" -elif echo "$OUT13" | grep -q "VagueWording"; then +elif grep -q "VagueWording" <<< "$OUT13"; then pass "flattens and flags with no GNU realpath available" else fail "produced no alert under a BSD-style realpath: $OUT13" @@ -509,9 +509,9 @@ FIXTURE14="$(make_fixture 2)" trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" "$FIXTURE14"' EXIT WRAPPED_DIR=$(run_wrap "$FIXTURE14" --config "$VALE_CONFIG" plugins) BARE_DIR=$(cd "$FIXTURE14" && vale --config "$VALE_CONFIG" plugins 2>&1 || true) -if ! echo "$WRAPPED_DIR" | grep -q "VagueWording"; then +if ! grep -q "VagueWording" <<< "$WRAPPED_DIR"; then fail "a directory argument produced no alert — flattening was silently skipped, the bug this test guards against" -elif echo "$BARE_DIR" | grep -q "VagueWording"; then +elif grep -q "VagueWording" <<< "$BARE_DIR"; then fail "bare vale already flags this fixture, so the test can't detect a silently-skipped flattening" else pass "a directory argument is walked and its files flattened" @@ -525,7 +525,7 @@ mkdir -p "$SPACED15" cp "$FIXTURE14/plugins/testplugin/skills/zzzskill/SKILL.md" "$SPACED15/SKILL.md" rm -rf "$FIXTURE14/plugins/testplugin/skills/zzzskill" OUT15=$(run_wrap "$FIXTURE14" --config "$VALE_CONFIG" plugins/testplugin/skills) -if echo "$OUT15" | grep -q "zzz skill" && echo "$OUT15" | grep -q "VagueWording"; then +if grep -q "zzz skill" <<< "$OUT15" && grep -q "VagueWording" <<< "$OUT15"; then pass "a file under a directory whose name contains a space is walked and flattened" else fail "a path with a space was dropped from the directory walk" @@ -626,7 +626,7 @@ unguarded_expansions() { done < <(sourced_files "$file") while IFS= read -r hit; do name="$(printf '%s\n' "$hit" \ - | grep -oE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' | head -1 \ + | { grep -oE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' || true; } | head -1 \ | sed -E 's/^\$\{//; s/\[@\]\}$//')" # Shell-maintained arrays are never seeded by a `NAME=(...)` line, so the # seeding exemption below can never clear them: without this case they are @@ -653,8 +653,7 @@ unguarded_expansions() { PIPESTATUS|BASH_SOURCE|BASH_LINENO|BASH_VERSINFO|GROUPS|DIRSTACK) continue ;; esac # Same-line emptiness short-circuit: the expansion cannot be reached empty. - if printf '%s\n' "$hit" \ - | grep -qE "\\\$\{#$name\[@\]\}[[:space:]]*-(eq|lt)[[:space:]]*[01][^|]*\|\|"; then + if grep -qE "\\\$\{#$name\[@\]\}[[:space:]]*-(eq|lt)[[:space:]]*[01][^|]*\|\|" <<< "$hit"; then continue fi for seed_file in ${seed_files[@]+"${seed_files[@]}"}; do @@ -809,7 +808,7 @@ for ARGS17 in "" "--config $VALE_CONFIG" "--config $VALE_CONFIG emptydir"; do # shellcheck disable=SC2086 # deliberate word splitting of the argv fixture OUT17+="$( (cd "$FIXTURE17" && "$OLD_BASH" "$SCRIPT" $ARGS17 &1) || true)" done -if echo "$OUT17" | grep -q "unbound variable"; then +if grep -q "unbound variable" <<< "$OUT17"; then fail "aborted with 'unbound variable' on bash $OLD_BASH_VER — the bug this test guards against" else pass "degenerate invocations run clean under bash $OLD_BASH_VER ($OLD_BASH)" @@ -828,7 +827,7 @@ SPACED18="$FIXTURE18/plugins/testplugin/skills/zzz skill dir" mkdir -p "$SPACED18" mv "$FIXTURE18/plugins/testplugin/skills/zzzskill/SKILL.md" "$SPACED18/SKILL.md" OUT18=$(run_wrap "$FIXTURE18" --config "$VALE_CONFIG" "plugins/testplugin/skills/zzz skill dir/SKILL.md") -if echo "$OUT18" | grep -q "zzz skill dir/SKILL.md" && echo "$OUT18" | grep -q "VagueWording"; then +if grep -q "zzz skill dir/SKILL.md" <<< "$OUT18" && grep -q "VagueWording" <<< "$OUT18"; then pass "a path argument with a space is passed to vale as one word" else fail "a path argument with a space was split by the array expansion: $OUT18" @@ -934,7 +933,7 @@ else new_fixture "$DIR19" BARE19="$(cd "$DIR19" && vale --config "$VALE_CONFIG" "$REL_SKILL19" 2>&1 || true)" GOT19="$(alert_text "$(run_wrap "$DIR19" --config "$VALE_CONFIG" "$REL_SKILL19")")" - if echo "$BARE19" | grep -q "VagueWording"; then + if grep -q "VagueWording" <<< "$BARE19"; then fail "bare vale already flags the $FORM19 form, so this case can't detect a silently-skipped flattening" elif [[ "$GOT19" == "$BASELINE19" ]]; then pass "a $FORM19 multi-line description reports the same alerts as its single-line form" @@ -1044,7 +1043,7 @@ done echo "" echo "--- the |- literal-block fallback lints normally and preserves line numbers ---" OUT20B=$(run_wrap "$FIXTURE20_BLOCK" --config "$VALE_CONFIG" "$REL_SKILL19") -if echo "$OUT20B" | grep -q "VagueWording"; then +if grep -q "VagueWording" <<< "$OUT20B"; then pass "a description needing quotes with both an apostrophe and a double quote is still linted" else fail "a description needing quotes with both an apostrophe and a double quote produced no alerts" @@ -1053,7 +1052,7 @@ WANT20B_LINE="$(grep -n 'flattening marker phrase' "$FIXTURE20_BLOCK/$REL_SKILL1 # `--output line` prints `file:line:col:Rule:message`, so the line number reads # back without any wrapping or colour to strip. GOT20B_LINE="$(run_wrap "$FIXTURE20_BLOCK" --config "$APOS_STYLE/.vale.ini" --output line "$REL_SKILL19" \ - | grep 'Apostrophe.Body' | head -1 | cut -d: -f2)" + | { grep 'Apostrophe.Body' || true; } | head -1 | cut -d: -f2)" if [[ "$GOT20B_LINE" == "$WANT20B_LINE" ]]; then pass "a body line after a |- flattened description keeps its original line number ($WANT20B_LINE)" else @@ -1079,7 +1078,10 @@ if [[ "$BARE21_FILES" != "in 1 file" ]]; then fail "bare vale did not lint the symlinked file ($BARE21_FILES), so this case can't detect the walk dropping it" elif [[ "$WRAPPED21_FILES" != "$BARE21_FILES" ]]; then fail "the directory walk dropped a symlinked file: wrapper saw '$WRAPPED21_FILES', bare vale '$BARE21_FILES'" -elif echo "$WRAPPED21" | grep -q "VagueWording"; then +# A here-string, not `echo "$WRAPPED21" | grep -q`: the match sits on line 3 of +# 8, and under pipefail grep -q exiting early can SIGPIPE echo mid-write and +# fail this branch on correct output (see tests/test-check-release-needed.sh). +elif grep -q "VagueWording" <<< "$WRAPPED21"; then pass "a symlinked file under a directory argument is mirrored, flattened and flagged" else fail "a symlinked file was mirrored but not flattened — no alert came back" @@ -1117,9 +1119,9 @@ RC23=$? set -e if [[ $RC23 -eq 0 ]]; then fail "a typo'd path exited 0 — indistinguishable from a clean run, the bug this test guards against" -elif echo "$OUT23" | grep -q "in stdin"; then +elif grep -q "in stdin" <<< "$OUT23"; then fail "a typo'd path fell back to reading stdin and reported 'in stdin' instead of erroring" -elif echo "$OUT23" | grep -q "SKILLL.md"; then +elif grep -q "SKILLL.md" <<< "$OUT23"; then pass "a typo'd path exits nonzero with a message naming the path" else fail "a typo'd path exited $RC23 but the message does not name it: $OUT23" @@ -1141,9 +1143,9 @@ mkdir -p "$FIXTURE24/line" for FORM24 in "--output line" "--output=line" "--output JSON" "--output=JSON"; do # shellcheck disable=SC2086 # deliberate word splitting of the argv fixture OUT24="$(run_wrap "$FIXTURE24" --config "$VALE_CONFIG" $FORM24 "$REL_SKILL19")" - if echo "$OUT24" | grep -q "E100"; then + if grep -q "E100" <<< "$OUT24"; then fail "'$FORM24' was rewritten to a cwd path and vale flipped into template mode — the bug this test guards against" - elif echo "$OUT24" | grep -q "VagueWording"; then + elif grep -q "VagueWording" <<< "$OUT24"; then pass "'$FORM24' is passed through as a built-in style name" else fail "'$FORM24' produced no alert: $OUT24" @@ -1215,12 +1217,12 @@ FOUND26="$(unguarded_expansions "$FIXTURE26/probe.sh")" MISSING26="" LEAKED26="" for ARR26 in $EXEMPT26; do - if echo "$FOUND26" | grep -q "{$ARR26\[@\]}"; then + if grep -q "{$ARR26\[@\]}" <<< "$FOUND26"; then LEAKED26+="$ARR26 " fi done for ARR26 in $FLAGGED26; do - if ! echo "$FOUND26" | grep -q "{$ARR26\[@\]}"; then + if ! grep -q "{$ARR26\[@\]}" <<< "$FOUND26"; then MISSING26+="$ARR26 " fi done @@ -1465,7 +1467,7 @@ matches_any_regex28() { [[ -n "$regexes" ]] || return 1 while IFS= read -r re; do [[ -n "$re" ]] || continue - if printf '%s\n' "$rel" | grep -Eq "$re"; then + if grep -Eq "$re" <<< "$rel"; then return 0 fi done <}" elif [[ "$count" -eq 0 ]]; then echo "FAIL|$rel|$sec scanned 0 files for $rel — the section's glob covers no path of that shape, so vale exits 0 and every gate reads it as a pass" - elif ! printf '%s\n' "$report" | grep -qF "Kyberforge.VagueWording"; then + elif ! grep -qF "Kyberforge.VagueWording" <<< "$report"; then echo "FAIL|$rel|$sec scanned $rel but raised no Kyberforge alert — the glob matches but the style is not loaded, which lints the file and reports nothing" else echo "PASS|$rel|$sec scans $rel ($count file) and raises a Kyberforge alert" @@ -1622,7 +1624,7 @@ if [[ "$VALE_READY" == true && "$PART_A_CLEAN28" == true ]]; then [[ -n "$REL28" && "$ISO28" == "isolating" ]] || continue LINE28="$(printf '%s\n' "$RESULTS28" | { grep -F "|$REL28|" || true; } | head -1)" if [[ "$ROWSEC28" == "$MSEC28" ]]; then - printf '%s\n' "$LINE28" | grep -qF "but raised no Kyberforge alert" \ + grep -qF "but raised no Kyberforge alert" <<< "$LINE28" \ || STYLE_MUT_FAILS28+="[$MSEC28 lost Kyberforge but $REL28 did not fail as style-not-loaded: ${LINE28:-}] " else [[ "$LINE28" == PASS\|* ]] \ @@ -1719,7 +1721,7 @@ copilot_scope_failures30() { [[ -n "$rel" ]] || continue report="$(vale_report28 "$cfg" "$TREE28" "$rel")" has=false - if printf '%s\n' "$report" | grep -qF "KyberforgeCopilot.ProactivePhrase"; then has=true; fi + if grep -qF "KyberforgeCopilot.ProactivePhrase" <<< "$report"; then has=true; fi case "$rel" in *.agent.md) [[ "$has" == true ]] || bad+="[$rel is an agent file but raised no ProactivePhrase alert — the Copilot style is shipped but never loaded for it, so its rules lint nothing] " @@ -1728,7 +1730,7 @@ copilot_scope_failures30() { # Non-vacuity guard: `absent` only means `scoped out` if the file was # scanned at all. Without the Kyberforge half, a glob that stopped # matching this path entirely would read as correct scoping. - if ! printf '%s\n' "$report" | grep -qF "Kyberforge.VagueWording"; then + if ! grep -qF "Kyberforge.VagueWording" <<< "$report"; then bad+="[$rel raised no Kyberforge alert either, so its missing ProactivePhrase proves nothing about scoping] " elif [[ "$has" == true ]]; then bad+="[$rel is not an .agent.md file but raised a ProactivePhrase alert — the Copilot style has leaked past the scope ADR-0013 fixes it to] " @@ -1789,9 +1791,9 @@ UNLOAD_FAILS30="$(copilot_scope_failures30 "$UNLOAD30/.vale.ini")" LEAK_FAILS30="$(copilot_scope_failures30 "$LEAK30/.vale.ini")" if cmp -s "$VALE_ASSETS28/.vale.ini" "$UNLOAD30/.vale.ini" || cmp -s "$VALE_ASSETS28/.vale.ini" "$LEAK30/.vale.ini"; then fail "a Copilot-scope mutation left the copied config unchanged, so Part B mutated nothing and proves nothing about Part A" -elif ! printf '%s' "$UNLOAD_FAILS30" | grep -qF "[copilot/demo.agent.md is an agent file but raised no ProactivePhrase alert"; then +elif ! grep -qF "[copilot/demo.agent.md is an agent file but raised no ProactivePhrase alert" <<< "$UNLOAD_FAILS30"; then fail "dropping KyberforgeCopilot from [**/*.agent.md] did not fail Part A, so an unloaded Copilot style would pass silently again: ${UNLOAD_FAILS30:-}" -elif ! printf '%s' "$LEAK_FAILS30" | grep -qF "[plugins/demo/.apm/skills/demo/SKILL.md is not an .agent.md file but raised a ProactivePhrase alert"; then +elif ! grep -qF "[plugins/demo/.apm/skills/demo/SKILL.md is not an .agent.md file but raised a ProactivePhrase alert" <<< "$LEAK_FAILS30"; then fail "adding KyberforgeCopilot to [**/SKILL.md] did not fail Part A, so the style could leak past ADR-0013's scope unnoticed: ${LEAK_FAILS30:-}" else pass "unloading KyberforgeCopilot from .agent.md files and leaking it onto SKILL.md files are each caught by Part A" @@ -1972,7 +1974,7 @@ SILENCED31="$(vale_report28 "$OVERRIDE_DIR31/.vale.ini" "$TREE28" "copilot/demo. SILENCED_COUNT31="$(files_scanned28 "$SILENCED31")" if [[ -z "$SILENCED_COUNT31" || "$SILENCED_COUNT31" -eq 0 ]]; then fail "the override fixture scanned no file at all, so the disappearance of the VagueWording alert proves nothing about overrides" -elif printf '%s\n' "$SILENCED31" | grep -qF "Kyberforge.VagueWording"; then +elif grep -qF "Kyberforge.VagueWording" <<< "$SILENCED31"; then # Reported as a FAIL, not a pass. It is not a defect in the config, but it # means Parts A and B are guarding a failure mode this vale build no longer # has — and a guard that guards nothing while reporting PASS is the same @@ -2115,13 +2117,13 @@ awk '/^[ \t]*files:/ { sub(/\^/, "^zzz-no-such-path/") } { print }' \ "$PC_CONFIG32" > "$MUT32/.pre-commit-config.yaml" MUT_RECORDS32="$(hook_records28 "$MUT32/.pre-commit-config.yaml")" MUT_FAILS32="$(prefilter_scope_failures32 "$MUT32/.pre-commit-config.yaml" "$REPO_FILES32")" -if ! printf '%s\n' "$MUT_RECORDS32" | grep -q 'zzz-no-such-path'; then +if ! grep -q 'zzz-no-such-path' <<< "$MUT_RECORDS32"; then fail "the narrowed regexes never reached the copied config, so Part B narrowed nothing and proves nothing about Part A" elif [[ "$(printf '%s\n' "$MUT_RECORDS32" | grep -c .)" -ne 2 ]]; then fail "the mutated config did not parse back as two prefilter hooks, so any failure below would come from the parser, not from the narrowing" -elif ! printf '%s' "$MUT_FAILS32" | grep -qF "vale-audit-prefilter-skill: 'files: ^zzz-no-such-path/"; then +elif ! grep -qF "vale-audit-prefilter-skill: 'files: ^zzz-no-such-path/" <<< "$MUT_FAILS32"; then fail "narrowing the skill hook's regex to match zero files did not fail this check, so Part A cannot detect a prefilter that has been silently switched off for SKILL.md files" -elif ! printf '%s' "$MUT_FAILS32" | grep -qF "vale-audit-prefilter-agent: 'files: ^zzz-no-such-path/"; then +elif ! grep -qF "vale-audit-prefilter-agent: 'files: ^zzz-no-such-path/" <<< "$MUT_FAILS32"; then fail "narrowing the agent hook's regex to match zero files did not fail this check, so Part A cannot detect a prefilter that has been silently switched off for agent files" else pass "narrowing either hook's 'files:' regex to match zero files is caught by Part A, which is what makes its pass mean something" @@ -2314,15 +2316,15 @@ if ! grep -qF '^plugins/kyberforge/\.apm/skills/' "$MUT33/skill.yaml" \ || ! grep -qF '^plugins/kyberforge/\.apm/agents/' "$MUT33/agent.yaml" \ || ! grep -qF 'vale-audit-prefilter-skill-renamed' "$MUT33/id.yaml"; then fail "a mutation never reached its copied config, so Part B mutated nothing and proves nothing about Part A" -elif ! printf '%s' "$SKILL_FAILS33" | grep -qF "[plugins/demo/.apm/skills/demo/SKILL.md is in scope of kyberforge-vale-audit-skill"; then +elif ! grep -qF "[plugins/demo/.apm/skills/demo/SKILL.md is in scope of kyberforge-vale-audit-skill" <<< "$SKILL_FAILS33"; then fail "narrowing the local skill hook to ^plugins/kyberforge/ did not fail Part A, so the prefilter can drop every other plugin's skills with every gate green: ${SKILL_FAILS33:-}" -elif [[ -z "$AGREE_FAILS33" ]] && printf '%s' "$SKILL_FAILS33" | grep -qF "kyberforge-vale-audit-agent"; then +elif [[ -z "$AGREE_FAILS33" ]] && grep -qF "kyberforge-vale-audit-agent" <<< "$SKILL_FAILS33"; then # Only meaningful against a clean base: when Part A already failed, the copy # inherits that defect, and reporting it again here would be one defect twice. fail "narrowing only the skill hook also reported an agent-class defect, so the comparison is not confined to its class: $SKILL_FAILS33" -elif ! printf '%s' "$AGENT_FAILS33" | grep -qF "[plugins/demo/.apm/agents/demo.agent.md is in scope of kyberforge-vale-audit-agent"; then +elif ! grep -qF "[plugins/demo/.apm/agents/demo.agent.md is in scope of kyberforge-vale-audit-agent" <<< "$AGENT_FAILS33"; then fail "narrowing the local agent hook to ^plugins/kyberforge/ did not fail Part A: ${AGENT_FAILS33:-}" -elif ! printf '%s' "$ID_FAILS33" | grep -qF "[no hook with id 'vale-audit-prefilter-skill' and a files: regex in id.yaml"; then +elif ! grep -qF "[no hook with id 'vale-audit-prefilter-skill' and a files: regex in id.yaml" <<< "$ID_FAILS33"; then fail "renaming the local skill hook's id did not fail Part A by name, so the skill class could fall out of the comparison silently: ${ID_FAILS33:-}" else pass "narrowing either local hook to one plugin, or renaming one, is caught by Part A"