diff --git a/tests/run-bats.sh b/tests/run-bats.sh index 9e374ad..9cf171e 100755 --- a/tests/run-bats.sh +++ b/tests/run-bats.sh @@ -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 diff --git a/tests/test-adr0020-contract.sh b/tests/test-adr0020-contract.sh index 8be0c1d..5f822df 100755 --- a/tests/test-adr0020-contract.sh +++ b/tests/test-adr0020-contract.sh @@ -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 ]] diff --git a/tests/test-no-pipefail-early-exit-grep.sh b/tests/test-no-pipefail-early-exit-grep.sh index fe44666..bae7693 100755 --- a/tests/test-no-pipefail-early-exit-grep.sh +++ b/tests/test-no-pipefail-early-exit-grep.sh @@ -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[@]}"})" diff --git a/tests/test-run-bats.sh b/tests/test-run-bats.sh index 0f637b4..e28eedf 100644 --- a/tests/test-run-bats.sh +++ b/tests/test-run-bats.sh @@ -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 ]] diff --git a/tests/test-vale-wrap.sh b/tests/test-vale-wrap.sh index 1a60646..0880325 100755 --- a/tests/test-vale-wrap.sh +++ b/tests/test-vale-wrap.sh @@ -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.