fix(test): bare '!' assertions are silent no-ops — convert all 22 + hygiene guard (#303)#307
Conversation
…e 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.
|
Warning Review limit reached
More reviews will be available in 7 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
This is a clean, well-motivated fix for a real silent-failure problem in bats. The dual approach — converting all 22 occurrences and adding a hygiene guard to prevent regression — is exactly right. Conversion correctness: Every substitution is faithful. The Guard design: The ERE Mutation testing: 6/6 kills across all 6 changed files confirms that each conversion now makes the test meaningful. That is the right quality bar for a fix of this nature. Count verification: 3 (e2e) + 6 (prd_import) + 4 (cli_modern) + 1 (enable_core) + 6 (github_import) + 2 (sandbox_docker) = 22 conversions, matching the stated number. No issues found. The logic is correct, the guard closes the loop on regression, and the mutation evidence confirms the assertions now have teeth. Mergeable as-is. |
Summary
Implements #303. bats-core's ERR trap doesn't fire for negated commands, so a mid-test
! cmdline asserts nothing — discovered in #75 when 9 such assertions let 5 product mutations survive.grepassertion[[ $(cmd | grep -c PAT) -eq 0 ]](flags like-x/--preserved; missing-file semantics unchanged — empty substitution compares equal to 0, same as the old silent pass)tests/unit/test_bats_hygiene.bats: guard forbidding line-leading bare!in any.batsfile (was RED with all 22 listed before conversion), modeled on the sha-pinning guardTest Plan
docker start→ test failsset +einjected into the live block → test fails--modelalways passed → test fails--plan-model→ test fails--resumemade unconditional → test failsKnown Limitations / Intentionally Deferred
.batsfiles only; sourced.bashhelpers may use!inside functions whose return value is checked at the call site (that pattern works correctly).Closes #303