From d5e7553a8bf048c88487909f608b126b77b8ecfb Mon Sep 17 00:00:00 2001 From: Mike Odnis Date: Tue, 14 Apr 2026 06:20:47 -0400 Subject: [PATCH] test(hooks): add bats suite (33 tests) + CI workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `< --- .github/workflows/hooks-tests.yml | 45 +++++++++++++ tests/hooks/commit-msg.bats | 99 +++++++++++++++++++++++++++++ tests/hooks/helpers.bash | 44 +++++++++++++ tests/hooks/post-checkout.bats | 61 ++++++++++++++++++ tests/hooks/pre-commit.bats | 46 ++++++++++++++ tests/hooks/pre-push.bats | 98 ++++++++++++++++++++++++++++ tests/hooks/prepare-commit-msg.bats | 56 ++++++++++++++++ 7 files changed, 449 insertions(+) create mode 100644 .github/workflows/hooks-tests.yml create mode 100644 tests/hooks/commit-msg.bats create mode 100644 tests/hooks/helpers.bash create mode 100644 tests/hooks/post-checkout.bats create mode 100644 tests/hooks/pre-commit.bats create mode 100644 tests/hooks/pre-push.bats create mode 100644 tests/hooks/prepare-commit-msg.bats diff --git a/.github/workflows/hooks-tests.yml b/.github/workflows/hooks-tests.yml new file mode 100644 index 0000000..2746c6b --- /dev/null +++ b/.github/workflows/hooks-tests.yml @@ -0,0 +1,45 @@ +# Copyright 2026 ResQ Software +# SPDX-License-Identifier: Apache-2.0 +# +# Run the bats suite over the canonical git-hooks shipped from this repo. +# +# Decoupled from hooks-sync.yml on purpose so this file can ship in a +# different PR without merge conflicts. + +name: hooks-tests + +on: + push: + branches: [main] + paths: + - 'scripts/git-hooks/**' + - 'tests/hooks/**' + - '.github/workflows/hooks-tests.yml' + pull_request: + paths: + - 'scripts/git-hooks/**' + - 'tests/hooks/**' + - '.github/workflows/hooks-tests.yml' + workflow_dispatch: + +permissions: + contents: read + +jobs: + bats: + name: bats + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install bats + run: | + sudo apt-get update -y + sudo apt-get install -y bats + + - name: Run hook test suite + run: bats tests/hooks/ + + - name: Print bats version (diagnostic) + if: always() + run: bats --version diff --git a/tests/hooks/commit-msg.bats b/tests/hooks/commit-msg.bats new file mode 100644 index 0000000..94866b8 --- /dev/null +++ b/tests/hooks/commit-msg.bats @@ -0,0 +1,99 @@ +#!/usr/bin/env bats +# Conventional Commits validation in the canonical commit-msg hook. + +load helpers + +setup() { + REPO="$(mktemp -d)" + init_repo_with_hooks "$REPO" + MSG="$REPO/.msg" +} + +teardown() { + rm -rf "$REPO" +} + +write_msg() { printf '%s\n' "$1" > "$MSG"; } + +@test "accepts feat: subject" { + write_msg "feat: add the thing" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -eq 0 ] +} + +@test "accepts feat(scope): subject" { + write_msg "feat(api): add /v2/foo" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -eq 0 ] +} + +@test "accepts feat!: breaking change marker" { + write_msg "feat!: drop legacy endpoint" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -eq 0 ] +} + +@test "accepts feat(scope)!: breaking with scope" { + write_msg "feat(api)!: drop legacy endpoint" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -eq 0 ] +} + +@test "accepts ticket-prefixed message" { + write_msg "[ABC-123] feat: add ticket prefix" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -eq 0 ] +} + +@test "rejects unknown type" { + write_msg "wat: this is not a type" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -ne 0 ] + [[ "$output" == *"Invalid commit message"* ]] +} + +@test "rejects missing subject after type" { + write_msg "feat:" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -ne 0 ] +} + +@test "rejects WIP on main" { + checkout_branch "$REPO" main + write_msg "WIP: still working" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -ne 0 ] + [[ "$output" == *"not allowed on main"* ]] +} + +@test "rejects fixup! on master" { + checkout_branch "$REPO" master + write_msg "fixup! feat: thing" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -ne 0 ] +} + +@test "GIT_HOOKS_SKIP=1 short-circuits even on bad input" { + write_msg "garbage that would normally fail" + GIT_HOOKS_SKIP=1 run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -eq 0 ] +} + +@test "subject starting with - does not break here-string handling" { + # Branch where head: was an `echo $X | grep` ate `-` as flag + write_msg "feat: -fix typo in flag handling" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -eq 0 ] +} + +@test "dispatches to local-commit-msg when present and executable" { + cat > "$REPO/.git-hooks/local-commit-msg" <<'EOF' +#!/usr/bin/env bash +echo "LOCAL_FIRED" +EOF + chmod +x "$REPO/.git-hooks/local-commit-msg" + write_msg "feat: trigger local" + run run_hook "$REPO" commit-msg "$MSG" + [ "$status" -eq 0 ] + [[ "$output" == *"LOCAL_FIRED"* ]] +} diff --git a/tests/hooks/helpers.bash b/tests/hooks/helpers.bash new file mode 100644 index 0000000..8008d65 --- /dev/null +++ b/tests/hooks/helpers.bash @@ -0,0 +1,44 @@ +# shellcheck shell=bash +# Common helpers for bats tests over the canonical ResQ git hooks. +# +# Each test gets a fresh tempdir initialized as a git repo with the canonical +# hooks copied in. core.hooksPath is set so `git` invokes the hooks naturally. + +# Absolute path to the canonical hook templates shipped by this repo. +HOOK_SRC="${BATS_TEST_DIRNAME}/../../scripts/git-hooks" + +# Initialize a fresh git repo in $1 with canonical hooks installed. +init_repo_with_hooks() { + local dir="$1" + 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/" + chmod +x "$dir/.git-hooks"/* + git -C "$dir" config core.hooksPath .git-hooks + git -C "$dir" config user.email t@t.io + git -C "$dir" config user.name t +} + +# Run a hook directly against a repo: run_hook [args...] +run_hook() { + local dir="$1" hook="$2" + shift 2 + (cd "$dir" && bash ".git-hooks/$hook" "$@") +} + +# Force-switch to branch (creates if missing). Avoids the fragile +# `branch -m` || `checkout -b` dance in tests. +checkout_branch() { + local dir="$1" name="$2" + git -C "$dir" checkout -q -B "$name" + git -C "$dir" symbolic-ref HEAD "refs/heads/$name" +} + +# Make a setup commit without firing the installed hooks. +# Args: [extra git-commit args...] +commit_no_hooks() { + local dir="$1" msg="$2" + shift 2 + git -C "$dir" -c "core.hooksPath=" commit --allow-empty -q -m "$msg" "$@" +} diff --git a/tests/hooks/post-checkout.bats b/tests/hooks/post-checkout.bats new file mode 100644 index 0000000..6adc31e --- /dev/null +++ b/tests/hooks/post-checkout.bats @@ -0,0 +1,61 @@ +#!/usr/bin/env bats +# Lock-file change notices and local dispatch for post-checkout/post-merge. + +load helpers + +setup() { + REPO="$(mktemp -d)" + init_repo_with_hooks "$REPO" +} + +teardown() { + rm -rf "$REPO" +} + +@test "post-checkout notifies on Cargo.lock change" { + PREV=$(git -C "$REPO" rev-parse HEAD) + printf 'lock\n' > "$REPO/Cargo.lock" + git -C "$REPO" add Cargo.lock + git -C "$REPO" -c "core.hooksPath=" commit -q -m "feat: lock" + NEW=$(git -C "$REPO" rev-parse HEAD) + run run_hook "$REPO" post-checkout "$PREV" "$NEW" 1 + [ "$status" -eq 0 ] + [[ "$output" == *"Cargo.lock changed"* ]] +} + +@test "post-checkout silent when no lockfile changed" { + PREV=$(git -C "$REPO" rev-parse HEAD) + git -C "$REPO" commit --allow-empty -q -m "feat: nothing" + NEW=$(git -C "$REPO" rev-parse HEAD) + run run_hook "$REPO" post-checkout "$PREV" "$NEW" 1 + [ "$status" -eq 0 ] + [[ "$output" != *"changed"* ]] +} + +@test "post-checkout dispatches to local-post-checkout" { + cat > "$REPO/.git-hooks/local-post-checkout" <<'EOF' +#!/usr/bin/env bash +echo "LOCAL_POST_CHECKOUT_RAN" +EOF + chmod +x "$REPO/.git-hooks/local-post-checkout" + PREV=$(git -C "$REPO" rev-parse HEAD) + commit_no_hooks "$REPO" "x" + NEW=$(git -C "$REPO" rev-parse HEAD) + run run_hook "$REPO" post-checkout "$PREV" "$NEW" 1 + [ "$status" -eq 0 ] + [[ "$output" == *"LOCAL_POST_CHECKOUT_RAN"* ]] +} + +@test "GIT_HOOKS_SKIP=1 short-circuits post-checkout" { + cat > "$REPO/.git-hooks/local-post-checkout" <<'EOF' +#!/usr/bin/env bash +echo "SHOULD_NOT_RUN" +EOF + chmod +x "$REPO/.git-hooks/local-post-checkout" + PREV=$(git -C "$REPO" rev-parse HEAD) + commit_no_hooks "$REPO" "x" + NEW=$(git -C "$REPO" rev-parse HEAD) + GIT_HOOKS_SKIP=1 run run_hook "$REPO" post-checkout "$PREV" "$NEW" 1 + [ "$status" -eq 0 ] + [[ "$output" != *"SHOULD_NOT_RUN"* ]] +} diff --git a/tests/hooks/pre-commit.bats b/tests/hooks/pre-commit.bats new file mode 100644 index 0000000..c1f5a22 --- /dev/null +++ b/tests/hooks/pre-commit.bats @@ -0,0 +1,46 @@ +#!/usr/bin/env bats +# Soft-skip + local dispatch behavior in the canonical pre-commit hook. +# (The full `resq pre-commit` checks are exercised by resq-cli's own tests; +# here we verify wiring only.) + +load helpers + +setup() { + REPO="$(mktemp -d)" + init_repo_with_hooks "$REPO" +} + +teardown() { + rm -rf "$REPO" +} + +@test "soft-skip with hint when resq is missing" { + # Run with PATH that excludes any resq + override $HOME to a clean dir + # so ~/.cargo/bin/resq isn't found either. + EMPTY="$(mktemp -d)" + PATH="/usr/bin:/bin" HOME="$EMPTY" run bash -c "cd '$REPO' && bash .git-hooks/pre-commit" + rm -rf "$EMPTY" + [ "$status" -eq 0 ] + [[ "$output" == *"resq not found"* ]] +} + +@test "GIT_HOOKS_SKIP=1 short-circuits before the resq lookup" { + EMPTY="$(mktemp -d)" + GIT_HOOKS_SKIP=1 PATH="/usr/bin:/bin" HOME="$EMPTY" run bash -c "cd '$REPO' && bash .git-hooks/pre-commit" + rm -rf "$EMPTY" + [ "$status" -eq 0 ] + [[ "$output" != *"resq not found"* ]] +} + +@test "dispatches to local-pre-commit even when resq is missing" { + cat > "$REPO/.git-hooks/local-pre-commit" <<'EOF' +#!/usr/bin/env bash +echo "LOCAL_PRE_COMMIT_RAN" +EOF + chmod +x "$REPO/.git-hooks/local-pre-commit" + EMPTY="$(mktemp -d)" + PATH="/usr/bin:/bin" HOME="$EMPTY" run bash -c "cd '$REPO' && bash .git-hooks/pre-commit" + rm -rf "$EMPTY" + [ "$status" -eq 0 ] + [[ "$output" == *"LOCAL_PRE_COMMIT_RAN"* ]] +} diff --git a/tests/hooks/pre-push.bats b/tests/hooks/pre-push.bats new file mode 100644 index 0000000..2db5630 --- /dev/null +++ b/tests/hooks/pre-push.bats @@ -0,0 +1,98 @@ +#!/usr/bin/env bats +# Branch naming, force-push guard, stdin propagation in the canonical pre-push hook. + +load helpers + +setup() { + REPO="$(mktemp -d)" + init_repo_with_hooks "$REPO" +} + +teardown() { + rm -rf "$REPO" +} + +# Generates a fake "git push" stdin line for one ref. +# Args: local_ref local_sha remote_ref remote_sha +push_line() { printf '%s %s %s %s\n' "$1" "$2" "$3" "$4"; } + +@test "accepts feat/ branch name" { + git -C "$REPO" checkout -q -b feat/add-thing + run bash -c "cd '$REPO' && bash .git-hooks/pre-push origin git@example "$REPO/.git-hooks/local-pre-push" <<'EOF' +#!/usr/bin/env bash +# Echo what's on stdin so the test can verify propagation. +echo "LOCAL_STDIN_BEGIN" +cat +echo "LOCAL_STDIN_END" +EOF + chmod +x "$REPO/.git-hooks/local-pre-push" + + LINE="$(push_line refs/heads/feat/x abc1234 refs/heads/feat/x def5678)" + run bash -c "cd '$REPO' && printf '%s\n' '$LINE' | bash .git-hooks/pre-push origin git@example" + [ "$status" -eq 0 ] + [[ "$output" == *"LOCAL_STDIN_BEGIN"* ]] + [[ "$output" == *"$LINE"* ]] + [[ "$output" == *"LOCAL_STDIN_END"* ]] +} + +@test "force push to main is rejected" { + checkout_branch "$REPO" main + # local sha (HEAD) and a fake remote sha that isn't an ancestor → force push. + LOCAL=$(git -C "$REPO" rev-parse HEAD) + REMOTE="0000000000000000000000000000000000000001" # arbitrary non-zero sha + LINE="$(push_line refs/heads/main "$LOCAL" refs/heads/main "$REMOTE")" + run bash -c "cd '$REPO' && printf '%s\n' '$LINE' | bash .git-hooks/pre-push origin git@example" + [ "$status" -ne 0 ] + [[ "$output" == *"Force push"* ]] +} + +@test "fast-forward push to main is allowed" { + checkout_branch "$REPO" main + OLD=$(git -C "$REPO" rev-parse HEAD) + commit_no_hooks "$REPO" "feat: more" + NEW=$(git -C "$REPO" rev-parse HEAD) + LINE="$(push_line refs/heads/main "$NEW" refs/heads/main "$OLD")" + run bash -c "cd '$REPO' && printf '%s\n' '$LINE' | bash .git-hooks/pre-push origin git@example" + [ "$status" -eq 0 ] +} + +@test "branch starting with - does not break grep here-string handling" { + # `git checkout -b -foo` is rejected by git itself, so we target the + # naming-convention check only — the regex must handle a `-` safely. + # Use a name that begins with a normal prefix but contains tricky chars. + git -C "$REPO" checkout -q -b "feat/-leading-dash" + run bash -c "cd '$REPO' && bash .git-hooks/pre-push origin git@example "$MSG" + run run_hook "$REPO" prepare-commit-msg "$MSG" + [ "$status" -eq 0 ] + grep -q "^\[ABC-123\] feat: do the thing$" "$MSG" +} + +@test "leaves message untouched on branch with no ticket" { + git -C "$REPO" checkout -q -b feat/no-ticket + printf 'feat: nothing to prefix\n' > "$MSG" + run run_hook "$REPO" prepare-commit-msg "$MSG" + [ "$status" -eq 0 ] + grep -q "^feat: nothing to prefix$" "$MSG" +} + +@test "does not double-prepend when ticket already in message" { + git -C "$REPO" checkout -q -b feat/ABC-123-thing + printf '[ABC-123] feat: already there\n' > "$MSG" + run run_hook "$REPO" prepare-commit-msg "$MSG" + [ "$status" -eq 0 ] + # Should still be exactly one occurrence + [ "$(grep -c 'ABC-123' "$MSG")" -eq 1 ] +} + +@test "skips on merge source" { + git -C "$REPO" checkout -q -b feat/ABC-123 + printf 'Merge pull request #1\n' > "$MSG" + run run_hook "$REPO" prepare-commit-msg "$MSG" merge + [ "$status" -eq 0 ] + grep -q "^Merge pull request" "$MSG" + ! grep -q "ABC-123" "$MSG" +} + +@test "skips on commit source" { + git -C "$REPO" checkout -q -b feat/ABC-123 + printf 'feat: something\n' > "$MSG" + run run_hook "$REPO" prepare-commit-msg "$MSG" commit + [ "$status" -eq 0 ] + ! grep -q "ABC-123" "$MSG" +}