test(hooks): bats suite (33 tests) + CI workflow + 2 hook bugfixes#4
test(hooks): bats suite (33 tests) + CI workflow + 2 hook bugfixes#4WomB0ComB0 merged 1 commit intomainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 4 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive BATS-based test suite for the project's git hooks and includes several improvements to the hook scripts themselves. Key changes include reordering guards in the commit-msg hook for better error messaging and adding safety checks to the pre-push hook to handle empty ref inputs. A high-severity issue was identified in the test helper script where a reference to a non-existent post-merge hook would cause test setup to fail.
| git -C "$dir" init -q | ||
| git -C "$dir" -c user.email=t@t.io -c user.name=t commit --allow-empty -m "init" -q | ||
| mkdir -p "$dir/.git-hooks" | ||
| cp "$HOOK_SRC"/{pre-commit,commit-msg,prepare-commit-msg,pre-push,post-checkout,post-merge} "$dir/.git-hooks/" |
There was a problem hiding this comment.
The cp command includes post-merge in the brace expansion, but this file is not present in the scripts/git-hooks/ directory according to the repository structure. This will cause the init_repo_with_hooks helper to fail, subsequently breaking all tests that rely on it.
cp "$HOOK_SRC"/{pre-commit,commit-msg,prepare-commit-msg,pre-push,post-checkout} "$dir/.git-hooks/"
Phase 3.1 of the canonical-hooks hardening roadmap.
tests/hooks/ — 33 bats tests across 5 files:
- commit-msg.bats: conventional commits accept/reject, ! marker, ticket
prefix tolerance, WIP-on-main guard, GIT_HOOKS_SKIP, local dispatch,
here-string safety against `-`-prefixed subjects
- prepare-commit-msg.bats: ticket-prefix injection from branch name,
no-op on merge/squash/commit sources
- pre-push.bats: branch-naming convention, changeset-release allowlist,
force-push guard with ref-replay through stdin, fast-forward path,
GIT_HOOKS_SKIP, local-pre-push stdin propagation
- post-checkout.bats: lock-file change notice, local dispatch
- pre-commit.bats: soft-skip when resq is missing, local dispatch
- helpers.bash: shared init_repo_with_hooks / checkout_branch /
commit_no_hooks helpers (the last bypasses installed hooks during
test setup so commit-msg doesn't reject "x" fixture commits)
.github/workflows/hooks-tests.yml — installs bats and runs the suite.
Decoupled from hooks-sync.yml so no merge conflict with #2.
Two real bugs surfaced and fixed by writing the tests:
1. pre-push: my earlier here-doc refactor (commit 21c7a77) fed an empty
line through `<<EOF\n$PUSH_REFS\nEOF` when stdin was empty, causing
the force-push guard to fire spuriously on `git push` invocations
without any refs (e.g. setup pushes from a clean main). Now guarded
with `[ -n "$PUSH_REFS" ]` and a per-iteration `[ -z "$local_sha" ]`
skip.
2. commit-msg: WIP/fixup/squash guard ran *after* the conventional-commits
format check. Since "WIP: foo" / "fixup! x" never pass the format
regex, the WIP guard was dead code and users always saw the generic
"Invalid format" error on main. Moved the WIP guard before the format
check so users on main get the branch-specific "WIP commits not
allowed on main" message.
Verified: bats tests/hooks/ → 33/33 pass locally.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WomB0ComB0
force-pushed
the
test/bats-hook-suite
branch
from
4876f58 to
d5e7553
Compare
|
@gemini-code-assist re your high-severity |
|
Thank you for the clarification. Since you have verified that |
1 participant
Summary
Phase 3.1 of the canonical-hooks hardening roadmap.
tests/hooks/— 33 bats tests across 5 filescommit-msg.bats— Conventional Commits accept/reject,!marker, ticket prefix, WIP-on-main guard,GIT_HOOKS_SKIP, local dispatch, here-string safetyprepare-commit-msg.bats— ticket-prefix injection, no-op on merge/squash/commit sourcespre-push.bats— branch naming,changeset-release/*, force-push guard via ref-replay, fast-forward path, stdin propagation tolocal-pre-pushpost-checkout.bats— lock-file change notice, local dispatchpre-commit.bats— soft-skip when resq is missing, local dispatchhelpers.bash—init_repo_with_hooks/checkout_branch/commit_no_hooks(last one bypasses installed hooks during fixture setup).github/workflows/hooks-tests.ymlInstalls bats and runs
bats tests/hooks/. Decoupled fromhooks-sync.ymlto ship without conflicting with dev #2.Two real bugs surfaced and fixed by the tests
pre-push — my earlier here-doc refactor (21c7a77) fed an empty line through
<<EOF\n$PUSH_REFS\nEOFwhen stdin was empty, causing the force-push guard to fire spuriously on pushes without ref input. Now guarded with[ -n "$PUSH_REFS" ]+ a per-iteration[ -z "$local_sha" ]skip.commit-msg — WIP/fixup/squash guard ran after the format check. Since
WIP:never matches the conventional-commits regex, the guard was dead code; users on main always saw the generic "Invalid format" error. Moved the guard before the format check so they now get the specific "WIP commits not allowed on main" message.Test plan
bats tests/hooks/→ 33/33 pass locallyshellcheck -S warningclean for all hook files🤖 Generated with Claude Code