fix(tests): repair four guards that pass without checking what they claim - #949
Conversation
…laim Four independent bug fixes under one theme: a test or hook that reports green without exercising the thing it names. - stub-real-pairing.bats detected a stubbing suite structurally, but only by grepping the .bats file itself; a suite stubbing a binary from a sourced library (tests/vault-health-golden.bats via tests/golden/vault-health/lib.sh) evaded it entirely. Widened to follow one level of `.`/`source` indirection under tests/, added the new exemption, and single-sourced the exemption list -- which was itself drifted (bitacora-rollout was exempted but missing from the stale-entries check). (#892) - The busy-binary install test never actually held a binary busy: a coreutils multi-call `sleep`, copied to a file named `dotf`, exits immediately instead of sleeping. Confirmed on this machine (uutils coreutils dispatches by resolved executable path, not argv[0], so the commonly-proposed `exec -a` fix does not help here either). Replaced the fixture with a copy of `bash` busy-spinning on builtins only -- no multi-call dispatch to satisfy, so the ETXTBSY path is genuinely reached regardless of host coreutils flavor. (#807) - check-spec-gate.sh's archive-on-merge LOC credit only fires when SDD_PR_BODY names a closing keyword, which is empty on every local pre-push run by design -- so a PR that correctly archived its spec in the same change could not be pushed locally. Added scripts/spec-gate-prepush.sh, a local wrapper mirroring the existing CI adapter (spec-gate-pr.sh) that resolves the branch's live PR via `gh` and falls through to today's behaviour on any resolution failure. Wired into .pre-commit-config.yaml's opt-in pre-push hook. (An earlier attempt credited any local active-to-archived spec transition unconditionally; the repo's own #397 regression tests caught that it reopens the "gratuitous archive-move dodging the gate" bypass, since local and CI share the same script gated only by an env var.) (#854) - git-hooks/** already carries an explicit eol=lf rule (BUG-068); the same CRLF hazard was uncovered for cli/internal/initrepo/templates/* (embedded into the dotf binary, so a CRLF checkout ships CRLF Makefile/gitignore/claude-md into every scaffolded repo), ssh/config, LICENSE, and the tests/golden/** fixture corpora. Added the missing .gitattributes rules and a class-level regression test asserting every tracked extensionless text file resolves an explicit eol, so the next one added outside a covered group fails loudly. (#761) Each fix is mutation-tested: the corresponding guard is shown failing against the pre-fix code/config and passing again after. Closes #892 Closes #807 Closes #854 Closes #761
|
Warning Review limit reached
Next review available in: 100 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
check-spec-gate.sh's ADJACENCY_ISSUES comment and spec-gate-pr.sh's header both described the pre-push tier as staying entirely offline, citing #854 as the reason. That framing predates this PR's fix: the pre-push tier now gets live PR context through scripts/spec-gate-prepush.sh, a separate wrapper that needs no token. Reworded both comments so they describe what still holds (this script fetches nothing itself; the CI adapter needs GH_TOKEN because it reads a PR it does not own) instead of a claim the same PR just made no longer true.
|
Cross-session note from the branch working on BUG-074 (PR #950).
|
|
@copilot resolve the merge conflicts in this pull request |
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: mlorentedev <13562150+mlorentedev@users.noreply.github.com>
Summary
Four independent fixes under one theme: a test or hook that reports green without exercising the thing it names.
stub-real-pairing.batsdetected a stubbing suite structurally, but only by grepping the.batsfile itself; a suite stubbing a binary from a sourced library (tests/vault-health-golden.batsviatests/golden/vault-health/lib.sh) evaded it entirely. Widened the detector to follow one level of./sourceindirection undertests/, added the new exemption, and single-sourced the exemption list — which was itself already drifted (bitacora-rolloutwas exempted but missing from the stale-entries check, so a staleness regression there would have gone undetected).sleep, copied to a file nameddotf, exits immediately instead of sleeping. Confirmed empirically on this machine —uutils coreutilsdispatches by resolved executable path, notargv[0], so the commonly-proposedexec -afix does not help here either (see the lessons.md entry for the reproduction). Replaced the fixture with a copy ofbashbusy-spinning on shell builtins only — no multi-call dispatch to satisfy, so the ETXTBSY path is genuinely reached regardless of host coreutils flavor.check-spec-gate.sh's archive-on-merge LOC credit only fires whenSDD_PR_BODYnames a closing keyword, which is empty on every local pre-push run by design — so a PR that correctly archived its spec in the same change could not be pushed locally. Addedscripts/spec-gate-prepush.sh, a local wrapper mirroring the existing CI adapter (spec-gate-pr.sh) that resolves the branch's live PR viaghand falls through to today's behaviour on any resolution failure. Wired into.pre-commit-config.yaml's opt-in pre-push hook. An earlier attempt credited any local active-to-archived spec transition unconditionally; the repo's own#397regression tests caught that this reopens the "gratuitous archive-move dodging the gate" bypass, since local and CI share the same script gated only by an env var — reverted in favor of the wrapper.git-hooks/**already carries an expliciteol=lfrule (BUG-068); the same CRLF hazard was uncovered forcli/internal/initrepo/templates/*(embedded into thedotfbinary viago:embed, so a CRLF checkout ships a CRLF Makefile/gitignore/claude-md into every scaffolded repo),ssh/config,LICENSE, and thetests/golden/**fixture corpora. Added the missing.gitattributesrules and a class-level regression test asserting every tracked extensionless text file resolves an expliciteol, so the next one added outside a covered group fails loudly instead of depending on which OS happens to check it out.Each fix is mutation-tested: the corresponding guard is shown failing against the pre-fix code/config and passing again after (details in the commit message and
docs/lessons.md).SDD skip rationale
Four independent, self-contained bug fixes, each individually a small obvious-cause fix backed by its own mutation-tested regression guard. They do not share one feature-id to scope a single spec around — SDD specs model a single feature/change, not a themed bundle of unrelated small fixes — so this ships under the skip-sdd escape hatch rather than forcing an artificial spec folder.
Test plan
~/.local/bin/shellcheckclean on every touched/added.shbash -nclean on every touched/added.sh~/.local/bin/bats tests/*.bats— 1235 passed, 0 failed, 73 skipped (Windows/pwsh-only)cd cli && go build ./...— unaffected, sanity-checkedCloses stub-real-pairing misses suites that stub a binary from a sourced library #892
Closes BUG-054: the busy-binary install test never holds a binary busy, so the ETXTBSY swap path is untested #807
Closes BUG-061: pre-push spec-gate false-negative on archive-on-merge (no PR body locally) #854
Closes BUG-039: extensionless git-hooks miss .gitattributes eol=lf — CRLF shebangs disable the GUARD-001 dispatcher #761