Skip to content

test: consolidate hook-utils suites into one shared lib suite#25

Merged
kyle-sexton merged 1 commit into
mainfrom
test/consolidate-hook-utils-suite
Jul 4, 2026
Merged

test: consolidate hook-utils suites into one shared lib suite#25
kyle-sexton merged 1 commit into
mainfrom
test/consolidate-hook-utils-suite

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the follow-up recorded in the shared-code decision record (#24): the four per-plugin hooks/hook-utils.test.sh suites had diverged — three were logic-identical (differing only in hook-id/extension fixtures), ruff-format's added six hook::read_file_path membership-guard cases — despite testing the byte-identical lib.

  • The union (43 cases, ruff-format's superset with a neutralized sample-hook id and a lib-scoped header) now lives at lib/hook-utils.test.sh, beside the canonical source.
  • The hook-utils-sync CI lane runs the suite — since the lane already enforces that every plugin copy is byte-identical to the source, testing the source covers the copies. Lib unit tests now run in CI for the first time (previously manual-only).
  • The four per-plugin copies are deleted; plugins keep only their own black-box hook contract tests (<plugin>.test.sh).
  • All four plugin versions bump patch so installed copies drop the removed file; playbook follow-up note replaced with the settled state.

Verification

  • lib/hook-utils.test.sh: 43/43 pass.
  • Remaining contract suites pass locally: bash-lint 31/31, markdown-formatter 41/41, ruff-format 46/46 (biome-format skips without a local Biome binary, as before).
  • claude plugin validate ×4 + --strict catalog pass; shellcheck, actionlint, markdownlint clean.
  • No references to the removed files anywhere in the repo.

🤖 Generated with Claude Code

https://claude.ai/code/session_012NNwc8U9MbgWsd5sbrfpPh


Note

Low Risk
Test and packaging-only changes; no shared runtime library behavior is modified in this diff.

Overview
Consolidates duplicated hook-utils unit tests into a single suite at lib/hook-utils.test.sh and removes the four per-plugin hooks/hook-utils.test.sh copies (bash-lint, biome-format, markdown-formatter, ruff-format).

The shared suite generalizes telemetry fixtures to a neutral sample-hook id and documents that byte-identical plugin copies are covered by testing the canonical lib. The hook-utils-sync CI job now runs bash lib/hook-utils.test.sh after the sync check, so lib unit tests are enforced in CI for the first time.

Docs in MIGRATION-PLAYBOOK.md replace the deferred “merge the four suites” note with the settled single-suite model. Patch version bumps on all four carrying plugins so installs drop the removed test files from cached plugin trees.

Reviewed by Cursor Bugbot for commit fed7c3d. Bugbot is set up for automated code reviews on this repo. Configure here.

The four per-plugin hook-utils.test.sh copies had diverged (three
identical in logic, ruff-format's adding six read_file_path membership
cases) while testing the byte-identical lib. The union now lives beside
the source as lib/hook-utils.test.sh with a neutral hook id, and the
hook-utils-sync CI lane runs it — copy byte-identity means testing the
source covers every plugin copy. Plugins keep only their black-box hook
contract tests.

All four plugin versions bump patch so installed copies drop the
now-removed per-plugin lib test file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012NNwc8U9MbgWsd5sbrfpPh
@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 52s —— View job


PR Review

  • Gather context and read changed files
  • Review CI changes
  • Review lib/hook-utils.test.sh
  • Review plugin.json version bumps
  • Review migration playbook update
  • Post findings

Verdict: Looks good — one pre-existing test quality observation below, nothing blocking.

This is a clean consolidation. The design rationale is sound: the byte-identity CI gate already guarantees the plugin copies are identical to the source, so testing the source is sufficient. Removing ~1 000 lines of duplicate tests without losing coverage is a net improvement.

CI wiring

.github/workflows/ci.yml line 198–199

Step ordering in the hook-utils-sync job is correct:

  1. Verify byte-identity → confirms the source is what the copies actually contain
  2. Run lib tests (new) → exercises the canonical source
  3. Verify carrying-plugin version bumps (PR-only)

ubuntu-latest has jq pre-installed, so no extra install step is needed. No issues here.

lib/hook-utils.test.sh

lib/hook-utils.test.sh line 10

HOOK_DIR is correctly resolved via ${BASH_SOURCE[0]} so source "$HOOK_DIR/hook-utils.sh" finds lib/hook-utils.sh from the new location regardless of $PWD.

set -uo pipefail (not -e) is intentional — the test runner must not exit on the first failing assertion. Correct.

The 43-case suite is the correct superset: it subsumes the three identical per-plugin versions and adds the six hook::read_file_path membership-guard cases from the ruff-format suite, with the ruff-format fixture neutralized to sample-hook. All good.

One pre-existing observation (inherited, not introduced here):

lib/hook-utils.test.sh line 273–283

Test 9 ("relative sink, no anchor: fail-open skip") removes OUT9 at line 274 and then checks ! -s "$OUT9" at line 283, but HOOK_TELEMETRY_SINK is set to "relative/no/anchor/sink.sh" — a completely unrelated path. Nothing could write to OUT9, so ! -s "$OUT9" always passes regardless of what emit_telemetry does. The meaningful assertion is only rc9 -eq 0. This doesn't affect correctness of the current suite (the fail-open rc check is the real guard), but the OUT9 variable could be removed for clarity. This is entirely out of scope for this PR to fix, just noting it for a future cleanup.

Plugin version bumps

All four plugins bumped patch:

Plugin Before After
bash-lint 0.1.2 0.1.3
biome-format 0.1.1 0.1.2
markdown-formatter 0.1.3 0.1.4
ruff-format 0.1.1 0.1.2

Patch is the right semver level for removing a file with no behavioral change to the hooks themselves. Consumers updating will have the deleted hook-utils.test.sh removed from their plugin cache.

Migration playbook

docs/MIGRATION-PLAYBOOK.md

The "Known follow-up" is correctly replaced with the settled state. The new wording accurately describes the architecture (source ↔ copies, one suite, CI-enforced).

Summary

No correctness, security, or CLAUDE.md alignment issues. The Test 9 OUT9 observation is a pre-existing limitation carried over verbatim — not a regression introduced here and not blocking. Ready to merge.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk: low. Cursor Bugbot passed with no findings. Approved — test-suite consolidation with CI coverage and no security or behavior risk.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit 3d3d6f6 into main Jul 4, 2026
22 checks passed
@kyle-sexton
kyle-sexton deleted the test/consolidate-hook-utils-suite branch July 4, 2026 10:17
kyle-sexton added a commit that referenced this pull request Jul 22, 2026
…e IV, flip pre-build (#939)

Dated Phase-4 note: operator ruling (2026-07-22) to move at maximum
speed within the predicate's intent. Calendar gates stay binding;
sanctioned levers are front-loaded seeding (genuine items only),
parallel Phase IV ignition (gate already met), and pre-built Phase III
flip machinery. Records same-day watch activity: drain PRs #23/#24/#26 +
Dependabot #25 merged in kyle-sexton/autonomy-demo-scratch, queue
reseeded (scratch#27–#31), predicate completions=6.

## Related

- kyle-sexton/autonomy-demo-scratch#27, #28, #29, #30, #31. No linked
issue — status-note documentation; nothing in this repo closes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant