Skip to content

Commit 252ede4

Browse files
author
Test User
committed
fix(test): convert bare '!' assertions to count-based checks + hygiene guard (#303)
bats-core's ERR trap doesn't fire for negated commands, so mid-test '! cmd' lines assert nothing. All 22 occurrences (6 files, every one a negative grep) converted to the count-based form from PR #302: ! cmd | grep -q PAT -> [[ $(cmd | grep -c PAT) -eq 0 ]] New tests/unit/test_bats_hygiene.bats forbids line-leading bare '!' in any .bats file (RED against the prior tree with all 22 listed). Mutation-verified one conversion per file (6/6 killed): docker-start no-op guard, live-block set+e, gitignore template overwrite, default --model absence (import unit + conversion-call integration), e2e first-call --resume absence.
1 parent 7582b9d commit 252ede4

7 files changed

Lines changed: 54 additions & 22 deletions

File tree

tests/e2e/test_full_loop.bats

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ teardown() {
110110
assert_file_exists "src/work_1.txt"
111111
assert_file_exists "src/work_2.txt"
112112
assert_file_exists "src/work_3.txt"
113-
! grep -q '^- \[ \]' .ralph/fix_plan.md
113+
[[ $(grep -c '^- \[ \]' .ralph/fix_plan.md) -eq 0 ]]
114114
}
115115

116116
@test "E2E: three test-only loops exit with test_saturation" {
@@ -203,7 +203,7 @@ EOF
203203
[[ "$output" == *"Saved Claude session"* ]]
204204

205205
# Loop 1 started fresh (no --resume); loop 2 resumed the stored session
206-
! grep -qx -- "--resume" "$MOCK_DIR/calls/argv_1.log"
206+
[[ $(grep -cx -- "--resume" "$MOCK_DIR/calls/argv_1.log") -eq 0 ]]
207207
grep -qx -- "--resume" "$MOCK_DIR/calls/argv_2.log"
208208
grep -qx "e2e-sess-abc123" "$MOCK_DIR/calls/argv_2.log"
209209
}
@@ -249,7 +249,7 @@ EOF
249249
assert_success
250250
assert_equal "$(status_field max_calls_per_hour)" "7"
251251
# --no-continue: the stored session must NOT be resumed
252-
! grep -qx -- "--resume" "$MOCK_DIR/calls/argv_1.log"
252+
[[ $(grep -cx -- "--resume" "$MOCK_DIR/calls/argv_1.log") -eq 0 ]]
253253
}
254254

255255
# =============================================================================

tests/integration/test_prd_import.bats

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,7 +1248,7 @@ MOCK_GH_EOF
12481248
grep -q '^# Add User Login' "$prd"
12491249
grep -q 'Build login form' "$prd"
12501250
# Comments are excluded by default (untrusted input; opt in via --include-comments)
1251-
! grep -q '## Discussion' "$prd"
1251+
[[ $(grep -c '## Discussion' "$prd") -eq 0 ]]
12521252

12531253
# No temp conversion artifacts left behind in the project
12541254
[[ ! -f "add-user-login/.ralph_conversion_output.json" ]]
@@ -1399,7 +1399,7 @@ MOCK_CLAUDE_EOF
13991399
# Only the conversion call happened
14001400
[[ -f "$TEST_DIR/claude_stdin_1" ]]
14011401
[[ ! -f "$TEST_DIR/claude_stdin_2" ]]
1402-
! grep -q "Implementation Plan Generation Task" "$TEST_DIR/claude_stdin_1"
1402+
[[ $(grep -c "Implementation Plan Generation Task" "$TEST_DIR/claude_stdin_1") -eq 0 ]]
14031403

14041404
# No generated plan artifact
14051405
[[ ! -f "add-user-login/.ralph/specs/implementation-plan.md" ]]
@@ -1458,7 +1458,7 @@ MOCK_CLAUDE_EOF
14581458
# Call-specific assertions: --model goes to the plan-generation call
14591459
# (line 1) only — the conversion call (line 2) must not inherit it
14601460
sed -n '1p' "$TEST_DIR/claude_args.log" | grep -q -- "--model opus"
1461-
! sed -n '2p' "$TEST_DIR/claude_args.log" | grep -q -- "--model"
1461+
[[ $(sed -n '2p' "$TEST_DIR/claude_args.log" | grep -c -- "--model") -eq 0 ]]
14621462
}
14631463

14641464
# =============================================================================
@@ -1574,7 +1574,7 @@ MOCK_GH_EOF
15741574
[[ "$output" == *"No issues match"* ]]
15751575
# Nothing was imported
15761576
[[ ! -d "new-bug-report" ]]
1577-
! grep -q "issue view" "$TEST_DIR/gh_args.log"
1577+
[[ $(grep -c "issue view" "$TEST_DIR/gh_args.log") -eq 0 ]]
15781578
}
15791579

15801580
@test "ralph-import --dry-run previews matches without importing" {
@@ -1592,5 +1592,5 @@ MOCK_GH_EOF
15921592

15931593
# No project was created and no issue was fetched for conversion
15941594
[[ ! -d "old-bug" && ! -d "p0-critical-crash" && ! -d "new-bug-report" ]]
1595-
! grep -q "issue view" "$TEST_DIR/gh_args.log"
1595+
[[ $(grep -c "issue view" "$TEST_DIR/gh_args.log") -eq 0 ]]
15961596
}

tests/unit/test_bats_hygiene.bats

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#!/usr/bin/env bats
2+
# Guard: line-leading bare `! cmd` assertions are silent no-ops in bats
3+
# (Issue #303)
4+
#
5+
# bats-core's ERR trap does not fire for negated commands, so a failing
6+
# `! cmd` line in a @test body asserts nothing. Only a final-line `!`
7+
# affects the test via its return status — and refactors move lines — so
8+
# the repo rule is absolute: NO line-leading bare `!` in any .bats file.
9+
# Use instead:
10+
# [[ $(cmd | grep -c PATTERN) -eq 0 ]] # negative grep assertions
11+
# run cmd; [ "$status" -ne 0 ] # expected-failure commands
12+
# a helper that returns 1 on unexpected success, checked at the call site
13+
#
14+
# Discovered in #75 (PR #302): 9 such assertions let 5 product mutations
15+
# survive undetected. This guard prevents the pattern from returning.
16+
17+
load '../helpers/test_helper'
18+
19+
REPO_ROOT="$BATS_TEST_DIRNAME/../.."
20+
21+
@test "no .bats file contains a line-leading bare '!' assertion" {
22+
local violations
23+
# POSIX classes only — BSD grep (macOS) has no \s in ERE
24+
violations=$(grep -rnE '^[[:space:]]*![[:space:]]' "$REPO_ROOT/tests" --include='*.bats' || true)
25+
if [[ -n "$violations" ]]; then
26+
echo "Bare '!' assertions found (silent no-ops under bats — issue #303):" >&2
27+
echo "$violations" >&2
28+
echo "Convert to a count-based grep, 'run' + status check, or a checked helper." >&2
29+
return 1
30+
fi
31+
return 0
32+
}

tests/unit/test_cli_modern.bats

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -931,10 +931,10 @@ EOF
931931
live_block=$(sed -n '/Live output mode enabled/,/End of Output/p' "$script")
932932

933933
# set +e and set -e should NOT appear in the live block
934-
! echo "$live_block" | grep -q '^[[:space:]]*set +e$'
935-
! echo "$live_block" | grep -q '^[[:space:]]*set -e'
936-
! echo "$live_block" | grep -q 'set -o pipefail'
937-
! echo "$live_block" | grep -q 'set +o pipefail'
934+
[[ $(echo "$live_block" | grep -c '^[[:space:]]*set +e$') -eq 0 ]]
935+
[[ $(echo "$live_block" | grep -c '^[[:space:]]*set -e') -eq 0 ]]
936+
[[ $(echo "$live_block" | grep -c 'set -o pipefail') -eq 0 ]]
937+
[[ $(echo "$live_block" | grep -c 'set +o pipefail') -eq 0 ]]
938938
}
939939

940940
@test "live mode pipeline logs timeout events with exit code 124" {

tests/unit/test_enable_core.bats

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ EOF
480480
assert_success
481481
# Should have template content, not old content
482482
grep -q ".ralph/.call_count" .gitignore
483-
! grep -q "my-custom-ignore" .gitignore
483+
[[ $(grep -c "my-custom-ignore" .gitignore) -eq 0 ]]
484484
}
485485

486486
@test "enable_ralph_in_directory succeeds when templates dir exists but .gitignore is missing" {

tests/unit/test_github_import.bats

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ EOF
195195
grep -q 'alice' out.md
196196
grep -q 'Here is the plan' out.md
197197
# Empty comment bodies are skipped
198-
! grep -q 'bot' out.md
198+
[[ $(grep -c 'bot' out.md) -eq 0 ]]
199199
}
200200

201201
@test "format_issue_as_prd excludes comments by default (untrusted input)" {
@@ -205,9 +205,9 @@ EOF
205205

206206
run format_issue_as_prd issue.json out.md
207207
assert_success
208-
! grep -q 'Discussion' out.md
209-
! grep -q 'mallory' out.md
210-
! grep -q 'ignore previous instructions' out.md
208+
[[ $(grep -c 'Discussion' out.md) -eq 0 ]]
209+
[[ $(grep -c 'mallory' out.md) -eq 0 ]]
210+
[[ $(grep -c 'ignore previous instructions' out.md) -eq 0 ]]
211211
}
212212

213213
@test "format_issue_as_prd warns on empty body but still produces a PRD" {
@@ -502,7 +502,7 @@ JSON
502502
run generate_implementation_plan "issue_prd.md" "analysis.json" "plan.md"
503503
assert_success
504504

505-
! grep -q -- "--model" "$TEST_DIR/claude_args"
505+
[[ $(grep -c -- "--model" "$TEST_DIR/claude_args") -eq 0 ]]
506506
}
507507

508508
@test "generate_implementation_plan sends missing elements and issue content to claude" {
@@ -556,8 +556,8 @@ MOCKEOF
556556

557557
# Legacy path must not pass modern-only flags (codex P1): old CLIs
558558
# reject --strict-mcp-config / --output-format
559-
! grep -q -- "--strict-mcp-config" "$TEST_DIR/claude_args"
560-
! grep -q -- "--output-format" "$TEST_DIR/claude_args"
559+
[[ $(grep -c -- "--strict-mcp-config" "$TEST_DIR/claude_args") -eq 0 ]]
560+
[[ $(grep -c -- "--output-format" "$TEST_DIR/claude_args") -eq 0 ]]
561561
}
562562

563563
@test "generate_implementation_plan fails on an empty plan" {

tests/unit/test_sandbox_docker.bats

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ _docker_args() { cat "$TEST_DIR/docker_args" 2>/dev/null; }
423423
start_sandbox_container
424424
run ensure_sandbox_container
425425
assert_success
426-
! _docker_args | grep -q '^start '
426+
[[ $(_docker_args | grep -c '^start ') -eq 0 ]]
427427
}
428428

429429
@test "ensure_sandbox_container: restarts a stopped container (OOM kill recovery)" {
@@ -498,7 +498,7 @@ _docker_args() { cat "$TEST_DIR/docker_args" 2>/dev/null; }
498498
init_docker_sandbox
499499
run handle_sandbox_timeout
500500
assert_success
501-
! _docker_args | grep -q '^restart'
501+
[[ $(_docker_args | grep -c '^restart') -eq 0 ]]
502502
}
503503

504504
# -----------------------------------------------------------------------------

0 commit comments

Comments
 (0)