test(gates): pin hook wiring and enforce the bats TAP plan
The repo's gates were not pinned to their wiring. Deleting the check-skill-version-bump block from .pre-commit-config.yaml left the whole suite green; deleting eight blocks at once, run-tests among them, also left it green. Only 4 of 20 hook ids had their wiring pinned anywhere, so a merge conflict resolved badly could stop the suite running at pre-push forever while every test still reported green. test-adr0020-contract now derives the repo-authored hooks from the repo: local entries and pins each one's id, entry and stages against an explicit expected set, both directions, with the same non-vacuity guards the file already applies to its own fixtures. Upstream hooks and their rev: values are untouched, so a rev bump does not churn the test. Mutation-checked: a removed block, a repointed entry and a hook moved off pre-push each go red; a rev bump, a comment edit and reordering stay green. 29 -> 44 assertions. run-bats computed each file's TAP plan and then discarded it, so a process printing "1..10", three ok lines and exit 0 was counted as "3 tests, 0 failures" with seven tests silently gone. That is exactly the wrapper-swallows-the-status case the runner's own comment puts in its threat model, and the plan was the only surviving signal. The plan is now enforced in both directions when a file emits exactly one. Also: test-no-pipefail-early-exit-grep's live-tree floor goes from 20 to 50 against an actual 57, matching test-vale-wrap's per-glob discipline, and test-vale-wrap's header names the real path to vale-wrap.sh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
This commit is contained in:
@@ -180,6 +180,15 @@ for f in ${TEST_FILES[@]+"${TEST_FILES[@]}"}; do
|
||||
# file bats really did run, so it has to be distinguishable from a file that
|
||||
# produced nothing whatsoever.
|
||||
file_plan="$(grep -c '^1\.\.[0-9]' "$SCRATCH_ROOT/$i.log" || true)"
|
||||
# The planned count itself, extracted only when the file emitted exactly one
|
||||
# plan line. Zero plans is the broken-harness case the aggregate guard below
|
||||
# names, and two or more means the log is not one file's TAP stream at all --
|
||||
# in neither case does "the planned count" mean anything, so the per-file
|
||||
# comparison is skipped and the previous behaviour stands.
|
||||
file_planned=""
|
||||
if [[ "$file_plan" -eq 1 ]]; then
|
||||
file_planned="$(sed -n 's/^1\.\.\([0-9][0-9]*\).*$/\1/p' "$SCRATCH_ROOT/$i.log")"
|
||||
fi
|
||||
# String-compared below, not `-ne`. `-ne` is arithmetic and bash evaluates an
|
||||
# empty string as 0 there -- `[[ "" -ne 0 ]]` is false -- so an *empty* status
|
||||
# file read as a clean exit. The `|| echo 1` fallback only covers a *missing*
|
||||
@@ -189,14 +198,28 @@ for f in ${TEST_FILES[@]+"${TEST_FILES[@]}"}; do
|
||||
TOTAL_OK=$((TOTAL_OK + file_ok))
|
||||
TOTAL_NOT_OK=$((TOTAL_NOT_OK + file_not_ok))
|
||||
TOTAL_PLANS=$((TOTAL_PLANS + file_plan))
|
||||
# Two independent failure signals, deliberately OR-ed: a file can report `not
|
||||
# ok` lines while its process still exits 0 (a bats formatter or wrapper that
|
||||
# swallows the status), and a file can exit non-zero having emitted no `not
|
||||
# ok` at all (a crash, a timeout, an unbound variable in setup_file). Real
|
||||
# bats normally emits both at once, so each signal masks the other and
|
||||
# dropping either half is invisible without tests that produce one without
|
||||
# the other -- tests/test-run-bats.sh has those.
|
||||
if [[ "$file_not_ok" -gt 0 || "$status" != "0" ]]; then
|
||||
# Three independent failure signals, deliberately OR-ed: a file can report
|
||||
# `not ok` lines while its process still exits 0 (a bats formatter or wrapper
|
||||
# that swallows the status), a file can exit non-zero having emitted no `not
|
||||
# ok` at all (a crash, a timeout, an unbound variable in setup_file), and a
|
||||
# file can emit FEWER results than its own plan line promised. Real bats
|
||||
# normally emits all three consistently, so each signal masks the others and
|
||||
# dropping any one of them is invisible without tests that produce one without
|
||||
# the rest -- tests/test-run-bats.sh has those.
|
||||
#
|
||||
# The plan is the third signal and it is now enforced, not merely counted. It
|
||||
# is the one that survives precisely the wrapper-swallows-the-status case
|
||||
# named above: a process printing `1..10`, three `ok` lines and exit 0 used to
|
||||
# be counted as "3 tests, 0 failures" and go green with seven tests silently
|
||||
# gone, because the plan was computed for the aggregate zero-count guard below
|
||||
# and then discarded. Mismatch either way is a failure -- more results than
|
||||
# planned is as broken a TAP stream as fewer.
|
||||
file_short=false
|
||||
if [[ -n "$file_planned" && $((file_ok + file_not_ok)) -ne "$file_planned" ]]; then
|
||||
file_short=true
|
||||
echo "Error: $rel planned $file_planned test(s) but emitted $((file_ok + file_not_ok)) result line(s) — the run was truncated, or its exit status was swallowed" >&2
|
||||
fi
|
||||
if [[ "$file_not_ok" -gt 0 || "$status" != "0" || "$file_short" == true ]]; then
|
||||
FAIL=1
|
||||
fi
|
||||
done
|
||||
|
||||
@@ -41,6 +41,12 @@
|
||||
# passing hook, and a SUGGESTION deliberately does not fail, so dropping
|
||||
# one word from the config silences the tier ADR-0020 depends on while
|
||||
# every test and every hook still reports green.
|
||||
# 4. The WIRING of every repo-authored hook — its id, its `entry:` and its
|
||||
# `stages:`. Same failure shape as 3, one level up: the tests drive these
|
||||
# scripts by path, so nothing noticed whether .pre-commit-config.yaml
|
||||
# still invoked them. On a scratch copy, deleting eight local hook blocks
|
||||
# at once — `run-tests` among them — left every suite green. Upstream
|
||||
# hooks and every `rev:` are deliberately out of scope.
|
||||
set -euo pipefail
|
||||
|
||||
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
@@ -680,6 +686,199 @@ while IFS=$'\t' read -r status msg; do
|
||||
fi
|
||||
done <<< "$VERBOSE_REPORT"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 4. Every repo-authored hook is still WIRED, at the stage it claims
|
||||
# ---------------------------------------------------------------------------
|
||||
# Same failure mode as assertion 3, one level up. Every gate this repo owns is
|
||||
# driven in tests by PATH -- tests/test-skill-version-bump.sh runs
|
||||
# scripts/check-skill-version-bump.sh directly -- so the script staying correct
|
||||
# and the hook still existing are independent facts, and only the first one was
|
||||
# pinned. Deleting the `run-tests` block from .pre-commit-config.yaml (a routine
|
||||
# merge-conflict casualty) stops the entire suite from running at pre-push
|
||||
# forever, and `bash tests/run-tests.sh --strict` still prints all green: the
|
||||
# suite cannot notice that nothing invokes it. Verified on a scratch copy --
|
||||
# eight repo-authored hook blocks were deleted at once and every suite stayed
|
||||
# green.
|
||||
#
|
||||
# So: id + entry + stages, for the repo-authored hooks only. "Repo-authored"
|
||||
# is derived from the config (every hook under a `repo: local` entry), not
|
||||
# hardcoded, and the derived set is then compared against the expected list
|
||||
# below -- a new local hook that nobody pinned is itself a failure.
|
||||
#
|
||||
# Deliberately NOT pinned: the stock upstream hooks (gitleaks, check-yaml,
|
||||
# pretty-format-json, shellcheck, conventional-pre-commit, the `repo: meta`
|
||||
# pair) and every `rev:`. Those are somebody else's contract; a rev bump must
|
||||
# not churn this test.
|
||||
echo ""
|
||||
echo "--- every repo-authored hook is wired, with the entry and stages it claims ---"
|
||||
WIRING_REPORT="$(python3 - "$REPO_ROOT" <<'PY'
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
import yaml
|
||||
|
||||
root = sys.argv[1]
|
||||
|
||||
|
||||
def emit(status, msg):
|
||||
print("%s\t%s" % (status, msg))
|
||||
|
||||
|
||||
# The expected wiring of every hook this repo authors: id -> (entry, stages).
|
||||
# ADDING OR REMOVING A REPO-AUTHORED HOOK DELIBERATELY REQUIRES EDITING THIS
|
||||
# LIST. That is the point -- the list is the second party to the agreement, so a
|
||||
# hook cannot leave .pre-commit-config.yaml without someone saying so here.
|
||||
# Upstream hooks are absent on purpose and must stay absent.
|
||||
EXPECTED = {
|
||||
# pre-push
|
||||
'run-tests': (
|
||||
'bash tests/run-tests.sh --strict', ['pre-push']),
|
||||
'check-executables-allow-sync': (
|
||||
'bash scripts/check-executables-allow-sync.sh', ['pre-push']),
|
||||
'apm-audit-ci': (
|
||||
'bash -c \'for d in . plugins/*/; do (cd "$d" && apm audit --ci) || '
|
||||
'{ echo "apm audit --ci failed in $d" >&2; exit 1; }; done\'',
|
||||
['pre-push']),
|
||||
'check-apm-agents-valid': (
|
||||
'bash scripts/check-apm-agents-valid.sh', ['pre-push']),
|
||||
'apm-pack-check-clean': (
|
||||
'apm pack --check-versions --check-clean --dry-run', ['pre-push']),
|
||||
'check-scope-walkup-sync': (
|
||||
'bash scripts/check-scope-walkup-sync.sh', ['pre-push']),
|
||||
'check-skill-version-bump': (
|
||||
'bash scripts/check-skill-version-bump.sh', ['pre-push']),
|
||||
'validate-marketplace': (
|
||||
'claude plugin validate --strict .claude-plugin/marketplace.json',
|
||||
['pre-push']),
|
||||
# pre-commit
|
||||
'skill-size-check': (
|
||||
'scripts/skill-size-check.sh', ['pre-commit']),
|
||||
'check-rtk-prefix': (
|
||||
'scripts/check-rtk-prefix.sh', ['pre-commit']),
|
||||
'vale-audit-prefilter-skill': (
|
||||
'plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh',
|
||||
['pre-commit']),
|
||||
'vale-audit-prefilter-agent': (
|
||||
'plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh',
|
||||
['pre-commit']),
|
||||
}
|
||||
|
||||
path = os.path.join(root, '.pre-commit-config.yaml')
|
||||
try:
|
||||
with open(path, encoding='utf-8') as fh:
|
||||
cfg = yaml.safe_load(fh) or {}
|
||||
except Exception as exc:
|
||||
emit('FAIL', '.pre-commit-config.yaml did not parse: %s' % exc)
|
||||
sys.exit(0)
|
||||
|
||||
local = {}
|
||||
duplicates = []
|
||||
for repo in cfg.get('repos') or []:
|
||||
if repo.get('repo') != 'local':
|
||||
continue
|
||||
for hook in (repo.get('hooks') or []):
|
||||
hid = hook.get('id')
|
||||
if hid in local:
|
||||
duplicates.append(hid)
|
||||
local[hid] = hook
|
||||
|
||||
# Non-vacuity, first. Every assertion below iterates the derived set, so an
|
||||
# empty one would report nothing and the section would pass having checked
|
||||
# nothing at all -- the same shape of silent hole this whole file exists to
|
||||
# close.
|
||||
if not EXPECTED:
|
||||
emit('FAIL', 'the expected repo-authored hook list is empty — '
|
||||
'every wiring assertion below is vacuous')
|
||||
sys.exit(0)
|
||||
if not local:
|
||||
emit('FAIL', '.pre-commit-config.yaml declares no `repo: local` hooks at '
|
||||
'all — every gate this repo authors has been unwired, or the '
|
||||
'config moved and this assertion is now measuring nothing')
|
||||
sys.exit(0)
|
||||
emit('PASS', '.pre-commit-config.yaml declares %d repo-authored (`repo: local`) '
|
||||
'hooks — the wiring assertions below are not vacuous' % len(local))
|
||||
|
||||
if duplicates:
|
||||
emit('FAIL', '.pre-commit-config.yaml declares duplicate local hook ids '
|
||||
'(%s) — pre-commit runs one of them and the other is dead '
|
||||
'config' % ', '.join(sorted(set(duplicates))))
|
||||
|
||||
# The set, before the per-hook detail: a deleted block shows up here as a
|
||||
# missing id even if nothing else in the file changed.
|
||||
missing = sorted(set(EXPECTED) - set(local))
|
||||
extra = sorted(set(local) - set(EXPECTED))
|
||||
if missing or extra:
|
||||
parts = []
|
||||
if missing:
|
||||
parts.append('NOT WIRED in .pre-commit-config.yaml: %s' % ', '.join(missing))
|
||||
if extra:
|
||||
parts.append('wired but not pinned in this test: %s' % ', '.join(extra))
|
||||
emit('FAIL', 'the repo-authored hook set has drifted — %s. A hook that '
|
||||
'leaves the config stops running at push time while every '
|
||||
'test stays green; a hook that arrives unpinned can leave '
|
||||
'again unnoticed. Fix the config, or update EXPECTED in '
|
||||
'tests/test-adr0020-contract.sh deliberately.' % '; '.join(parts))
|
||||
else:
|
||||
emit('PASS', '.pre-commit-config.yaml wires exactly the %d expected '
|
||||
'repo-authored hooks, no more and no fewer' % len(EXPECTED))
|
||||
|
||||
# Per hook: the entry that runs and the stage it runs at. Both are single
|
||||
# tokens whose loss is invisible -- an entry repointed at a path that no longer
|
||||
# exists makes pre-commit fail loudly, but an entry repointed at a DIFFERENT
|
||||
# real script does not, and a hook moved off pre-push simply never fires.
|
||||
entry_paths = []
|
||||
for hid in sorted(EXPECTED):
|
||||
hook = local.get(hid)
|
||||
if hook is None:
|
||||
continue # already reported as missing above
|
||||
exp_entry, exp_stages = EXPECTED[hid]
|
||||
got_entry = hook.get('entry')
|
||||
got_stages = hook.get('stages')
|
||||
problems = []
|
||||
if got_entry != exp_entry:
|
||||
problems.append('entry is %r, expected %r' % (got_entry, exp_entry))
|
||||
if list(got_stages or []) != exp_stages:
|
||||
problems.append('stages is %r, expected %r — a hook at the wrong stage '
|
||||
'(or at none) never fires' % (got_stages, exp_stages))
|
||||
if problems:
|
||||
emit('FAIL', 'hook %s: %s' % (hid, '; '.join(problems)))
|
||||
else:
|
||||
emit('PASS', 'hook %s runs `%s` at %s'
|
||||
% (hid, exp_entry, ','.join(exp_stages)))
|
||||
# Collect the in-repo script/manifest paths the pinned entry names, so the
|
||||
# pin cannot agree with a config that points at nothing.
|
||||
for token in re.split(r'\s+', exp_entry):
|
||||
token = token.strip('\'"')
|
||||
if re.fullmatch(r'[A-Za-z0-9_.\-/]+\.(sh|json)', token):
|
||||
entry_paths.append((hid, token))
|
||||
|
||||
# Every pinned entry path exists on disk. Without this, EXPECTED and the config
|
||||
# could agree perfectly on a script that was deleted.
|
||||
if not entry_paths:
|
||||
emit('FAIL', 'no pinned entry named an in-repo script or manifest — the '
|
||||
'existence check below iterated zero times and proved nothing')
|
||||
else:
|
||||
broken = ['%s -> %s' % (hid, token)
|
||||
for hid, token in entry_paths
|
||||
if not os.path.exists(os.path.join(root, token))]
|
||||
if broken:
|
||||
emit('FAIL', 'pinned hook entries point at files that do not exist: %s'
|
||||
% ', '.join(broken))
|
||||
else:
|
||||
emit('PASS', 'all %d in-repo files named by a pinned hook entry exist '
|
||||
'on disk' % len(entry_paths))
|
||||
PY
|
||||
)"
|
||||
while IFS=$'\t' read -r status msg; do
|
||||
[[ -n "$status" ]] || continue
|
||||
if [[ "$status" == PASS ]]; then
|
||||
pass "$msg"
|
||||
else
|
||||
fail "$msg"
|
||||
fi
|
||||
done <<< "$WIRING_REPORT"
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
@@ -204,7 +204,14 @@ while IFS= read -r rel; do
|
||||
[[ -f "$REPO_ROOT/$rel" ]] && FILES+=("$REPO_ROOT/$rel")
|
||||
done < <(git -C "$REPO_ROOT" ls-files -- '*.sh' '*.bats' '*.bash')
|
||||
|
||||
if [[ "${#FILES[@]}" -lt 20 ]]; then
|
||||
# Floored at 50 against a real 57 (58 tracked `*.sh`/`*.bats`/`*.bash` files, less
|
||||
# this one, which excludes itself above). Same discipline as the per-glob floors in
|
||||
# tests/test-vale-wrap.sh: a guard against a broken `git ls-files` invocation or a
|
||||
# moved search root resolving to a fraction of the tree, not a headcount to keep in
|
||||
# step. Seven files of slack is one deliberate multi-file deletion -- a plugin's
|
||||
# whole tests/ directory is three or four .bats files -- and nowhere near enough to
|
||||
# absorb a discovery that degraded to a single directory.
|
||||
if [[ "${#FILES[@]}" -lt 50 ]]; then
|
||||
fail "only ${#FILES[@]} tracked shell files found — the scan is looking in the wrong place"
|
||||
else
|
||||
LIVE_HITS="$(scan ${FILES[@]+"${FILES[@]}"})"
|
||||
|
||||
@@ -444,6 +444,38 @@ else
|
||||
pass "a root under .claude/worktrees/ runs its own files and skips nested worktrees"
|
||||
fi
|
||||
|
||||
# --- 11. A file that emits fewer results than its own plan promised fails the
|
||||
# run. This is the third aggregation signal, and the only one left standing in
|
||||
# exactly the case run-bats.sh's own comment puts in its threat model: a bats
|
||||
# formatter or wrapper that swallows the exit status. A process printing `1..10`,
|
||||
# three `ok` lines and exiting 0 emits no `not ok` and no non-zero status, so both
|
||||
# other halves stay silent -- the plan was already being computed for the
|
||||
# aggregate zero-count guard and was then thrown away, so the run was counted as
|
||||
# "6 tests, 0 failures" and went green with fourteen tests silently gone.
|
||||
echo ""
|
||||
echo "--- a file emitting fewer results than its plan fails the run ---"
|
||||
DIR11="$(make_fake_repo)"
|
||||
FIXTURES+=("$DIR11")
|
||||
seed_bats_files "$DIR11"
|
||||
install_stub_bats "$DIR11" <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
echo "1..10"
|
||||
echo "ok 1 first"
|
||||
echo "ok 2 second"
|
||||
echo "ok 3 third"
|
||||
exit 0
|
||||
EOF
|
||||
run_fake "$DIR11"
|
||||
if [[ $FAKE_RC -eq 0 ]]; then
|
||||
fail "a file delivering 3 of its 10 planned tests exited 0 — a truncated run reported as a pass"
|
||||
elif ! grep -q "planned 10 test(s) but emitted 3 result line(s)" <<< "$FAKE_OUT"; then
|
||||
fail "the run failed but not with the plan-shortfall message: $FAKE_OUT"
|
||||
elif grep -q "^6 tests, 0 failures$" <<< "$FAKE_OUT"; then
|
||||
pass "a plan promising more tests than were delivered fails the run and names the shortfall"
|
||||
else
|
||||
fail "the plan-shortfall run failed with the wrong count: $FAKE_OUT"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
#!/usr/bin/env bash
|
||||
# Regression test for scripts/vale-wrap.sh: Vale's `text.frontmatter.description`
|
||||
# Regression test for plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh: Vale's `text.frontmatter.description`
|
||||
# NLP scope silently stops matching when the description value is a YAML block
|
||||
# scalar spanning 2+ physical lines. vale-wrap.sh flattens it to one line before
|
||||
# handing off to the real vale binary — this asserts that actually happens.
|
||||
|
||||
Reference in New Issue
Block a user