Wire the shell ratchet into pre-commit, and test the coverage tooling

check_shell_coverage.sh has existed for a while and was never in
.pre-commit-config.yaml, so nothing ran it: a new shell library with no
suite beside it could land freely. Wiring it was unaffordable while the
gate measured a jail-run 100% bar (--all took over 600s). After 578c4a6c
dropped that, --all is 0.31s and the per-file mode this hook uses is
milliseconds, so it goes in.

Also fixes a bug 578c4a6c introduced: the failure path still interpolated
${COVERAGE_BAR}, which that commit deleted, so every rejection printed
"COVERAGE_BAR: unbound variable" and advised a 100% line-coverage bar
that no longer exists. The message now describes what is enforced.

Three libs blocked the wiring -- meta/scripts/lib/shell_coverage_{bar,
case_runner,jail_setup}.sh, the code that BUILDS the jail, and so the one
corner of the repo the jail cannot measure itself. They are pure
file-writers, so they get a suite that calls them against a temp $JAIL
and asserts on what was written: 48 assertions, nothing mounted, no
unshare, no kcov.

Several assertions check that an emitted file contains an UNEXPANDED
${BASH_SOURCE} or $1 -- the whole point of the quoted heredocs the jail
relies on. Written as single-quoted literals that is SC2016 on every
line, so the needles compose the dollar from an exported constant
instead. No suppressions were added.

  check_shell_coverage.sh --all:  3 BLOCK  ->  0 BLOCK
  hook, repo-wide:                0.16s
  new suite:                      48 assertions, green

Proved the ratchet still bites: an untested lib dropped into
mtk_root/lib is refused with "needs: .../tests/run_all.sh". Probe removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PJrqKERAo5QUeEHWS5Asgp
This commit is contained in:
Krzysztof Rudnicki 2026-08-23 15:51:29 +02:00
parent 578c4a6cdd
commit 46c8f065e8
9 changed files with 413 additions and 5 deletions

View File

@ -134,6 +134,19 @@ repos:
types: [shell]
require_serial: true
exclude: ^(\.git/|phone_focus_mode/lib/tests/|tests/)
- id: shell-coverage-ratchet
name: Every shell library has a test suite beside it
# The ratchet from meta/shell-coverage-allowlist.txt: a lib under a
# lib/ directory needs a tests/run_all.sh, unless it is allowlisted.
# Wired here on 2026-08-23. It existed but was never in pre-commit,
# so it had been silently unenforced -- a new untested lib could land
# freely. That was only affordable to fix once 578c4a6c dropped the
# jail-measured 100% bar: --all went from >600s to 0.31s, and the
# per-file mode this hook uses is milliseconds.
entry: bash meta/scripts/check_shell_coverage.sh
language: system
types: [shell]
exclude: ^third_party/
# ===========================================================================
# NOQA BLOCKER - Zero tolerance for noqa/type:ignore suppression comments

View File

@ -0,0 +1,19 @@
{
"title": "Wire the shell coverage ratchet into pre-commit and test the coverage tooling",
"objective": "Make the shell ratchet enforceable at commit time rather than existing but never running, and close the gap where the three libs that build the coverage jail had no tests of their own.",
"acceptance_criteria": [
"A new shell library in a directory with no tests/run_all.sh is refused by pre-commit.",
"A shell library in a directory that has a suite, or one on the allowlist, is accepted.",
"check_shell_coverage.sh --all reports zero BLOCK entries and completes in under 5 seconds.",
"check_shell_coverage.sh emits no 'unbound variable' error on its failure path.",
"The new suite runs green standalone via bash meta/scripts/lib/tests/run_all.sh.",
"No lint suppression (noqa, shellcheck disable, per-file ignore) is added anywhere.",
"The allowlist is not grown; entries may only be removed."
],
"out_of_scope": [
"Restoring the jail-measured 100% bar removed in 578c4a6c.",
"Writing suites for the 8 allowlisted directories that still have none.",
"Changing shell_coverage_jail.sh, which is still used by ci_mirror.sh."
],
"verifier": "bash meta/scripts/lib/tests/run_all.sh exits 0 with 48 assertions; pre-commit run shell-coverage-ratchet --all-files passes; creating an untested lib under mtk_root/lib and running the hook on it fails with a 'needs: .../tests/run_all.sh' message; shellcheck -x and shfmt -l are clean on every touched file."
}

View File

@ -0,0 +1,44 @@
{
"intent": "Wire check_shell_coverage.sh into pre-commit so the shell ratchet cannot silently rot, and give the coverage tooling itself the suite it was missing.",
"scope": [
".pre-commit-config.yaml (new shell-coverage-ratchet hook)",
"meta/scripts/check_shell_coverage.sh (stale message referencing a removed constant)",
"meta/scripts/lib/tests/ (new suite: harness + 3 test files + run_all.sh)"
],
"changes": [
"Added the shell-coverage-ratchet hook, types: [shell], excluding third_party/. It was previously never wired, so a new untested shell library could land freely.",
"FIX: check_shell_coverage.sh line 177 interpolated ${COVERAGE_BAR}, which no longer exists after 578c4a6c, producing 'COVERAGE_BAR: unbound variable' on every failure path. The message now describes the presence bar it actually enforces.",
"New suite for the three meta/scripts/lib coverage-tooling libs, which are the one corner of the repo the jail cannot measure itself: 48 assertions over is_covered, build_jail, write_cases, build_mount_script, reconcile_pass_failures and write_case_runner.",
"Assertions on unexpanded ${BASH_SOURCE}/$1 literals compose the dollar sign from an exported DOLLAR constant rather than writing single-quoted literals, so the files are SC2016-clean with no suppressions."
],
"verification": [
{
"command": "bash meta/scripts/lib/tests/run_all.sh",
"result": "pass",
"evidence": "48 assertions across 3 files, 0 failed (5 + 26 + 17)"
},
{
"command": "pre-commit run shell-coverage-ratchet --all-files",
"result": "pass",
"evidence": "Passed in 0.16s; check_shell_coverage.sh --all now reports 0 BLOCK entries, down from 3, and runs in 0.31s"
},
{
"command": "Created an untested lib at mtk_root/lib/zz_probe.sh and ran the hook on it",
"result": "pass",
"evidence": "Hook failed with 'needs: linux_configuration/scripts/mtk_root/lib/tests/run_all.sh'. Probe file removed; the ratchet is not toothless."
},
{
"command": "shellcheck -x and shfmt -l over the new files",
"result": "pass",
"evidence": "clean, no noqa/disable directives added"
}
],
"risks": [
"The hook now runs on every commit touching a shell file. Measured at 0.16s repo-wide, so the cost is negligible.",
"A lib added to a directory that has no suite is now blocked at commit time. That is the intended ratchet behaviour, and all 8 such directories are already allowlisted."
],
"rollback": [
"Remove the shell-coverage-ratchet block from .pre-commit-config.yaml; the gate returns to being unenforced.",
"After rollback, confirm `pre-commit run --all-files` no longer lists the hook."
]
}

View File

@ -168,16 +168,16 @@ check_files() {
return 0
fi
echo "$SCRIPT_NAME: ${#offenders[@]} shell librar(ies) below the coverage bar:" >&2
echo "$SCRIPT_NAME: ${#offenders[@]} shell librar(ies) without a suite:" >&2
for path in "${offenders[@]}"; do
printf ' %s\n' "$path" >&2
printf ' needs: %s/tests/run_all.sh\n' "$(dirname "$path")" >&2
done
echo >&2
echo " A shell library must ship with a suite that drives it to
${COVERAGE_BAR}% line coverage. Extend" >&2
echo " the nearest existing tests/ directory rather than cloning a" >&2
echo " harness -- jscpd fails above 2% duplication." >&2
echo " A shell library must ship with a suite that sources and" >&2
echo " exercises it. Extend the nearest existing tests/ directory" >&2
echo " rather than cloning a harness -- jscpd fails above 2%" >&2
echo " duplication." >&2
return 1
}

View File

@ -0,0 +1,73 @@
# shellcheck shell=bash
# lib/tests/coverage_tool_harness.sh — fixture for the shell-coverage tooling.
#
# Sourced, not executed. These three libs are pure file-writers and one
# predicate: given $JAIL (a temp dir) they emit run_cases.sh, cases and
# mounts.sh, and is_covered answers from the filesystem alone. Nothing here
# mounts anything, runs kcov, or needs a namespace -- the jail itself is
# exercised end-to-end by the suites that already run under it.
set -uo pipefail
PASS=0
FAIL=0
_t_pass() {
PASS=$((PASS + 1))
printf ' OK: %s\n' "$1"
}
_t_fail() {
FAIL=$((FAIL + 1))
printf ' FAIL: %s\n' "$1"
}
_t_eq() {
if [[ "$2" == "$1" ]]; then
_t_pass "$3"
else
_t_fail "$3 (want '$1', got '$2')"
fi
}
_t_has() {
if [[ "$1" == *"$2"* ]]; then
_t_pass "$3"
else
_t_fail "$3 (want a substring '$2')"
fi
}
_t_lacks() {
if [[ "$1" != *"$2"* ]]; then
_t_pass "$3"
else
_t_fail "$3 (did not want substring '$2')"
fi
}
# A throwaway $JAIL for one test file, removed on exit. The libs write into it
# unconditionally, so every case needs its own clean directory.
_t_new_jail() {
JAIL="$(mktemp -d -t covtool-XXXXXX)"
export JAIL
mkdir -p "$JAIL/trace"
}
_t_drop_jail() {
if [[ -n "${JAIL:-}" && "$JAIL" == /tmp/covtool-* ]]; then
rm -rf "$JAIL"
fi
}
# The literal dollar sign, built rather than written. Several assertions here
# check that a file contains an UNEXPANDED ${BASH_SOURCE} or $1, and writing
# that as a single-quoted literal trips SC2016 on every line. Composing the
# needle keeps the intent obvious and the file suppression-free.
DOLLAR='$'
export DOLLAR
readonly DOLLAR
_t_summary() {
printf '\n%s: %d passed, %d failed\n' "$(basename "$0")" "$PASS" "$FAIL"
[[ $FAIL -eq 0 ]]
}

View File

@ -0,0 +1,26 @@
#!/usr/bin/env bash
# lib/tests/run_all.sh — run every shell-coverage tooling test in one go, and
# exit non-zero if any of them fails.
#
# These three libs build and measure the coverage jail, so they are the one
# corner of the repo the jail cannot check itself. The tests are pure: they
# call the file-writing functions against a temp $JAIL and assert on what was
# written. Nothing here mounts, unshares, or runs kcov.
set -uo pipefail
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
failed=0
for test_script in "${SCRIPT_DIR}"/test_*.sh; do
printf '\n### %s\n' "$(basename "$test_script")"
if ! "$test_script"; then
failed=$((failed + 1))
fi
done
if [[ $failed -gt 0 ]]; then
printf '\n%d test file(s) FAILED\n' "$failed"
exit 1
fi
printf '\nAll shell-coverage tooling test files passed.\n'

View File

@ -0,0 +1,55 @@
#!/usr/bin/env bash
# write_case_runner: the script that executes cases INSIDE the jail.
set -uo pipefail
HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# shellcheck source=./coverage_tool_harness.sh
. "$HERE/coverage_tool_harness.sh"
# shellcheck source=../shell_coverage_case_runner.sh
. "$HERE/../shell_coverage_case_runner.sh"
_t_new_jail
write_case_runner
runner="$JAIL/run_cases.sh"
_t_eq "yes" "$([[ -x "$runner" ]] && echo yes)" "the runner is written executable"
body="$(cat "$runner")"
# It runs in the child namespace, so it must be syntactically valid on its own.
bash -n "$runner" 2>/dev/null
_t_eq "0" "$?" "the emitted runner parses as bash"
# Both passes must be present, and they cannot be merged: kcov's ptrace and
# xtrace destroy each other's numbers.
_t_has "$body" "kcov --include-pattern" "the kcov pass invokes kcov with an include pattern"
_t_has "$body" "BASH_ENV=" "the trace pass delivers tracing via BASH_ENV"
_t_lacks "$body" "SHELLOPTS=xtrace" "tracing is NOT delivered via SHELLOPTS (readonly, and dropped when privileged)"
# fd 199, deliberately high: a subject doing `exec 9>...` would clobber a low fd
# and silently truncate the trace.
_t_has "$body" "BASH_XTRACEFD=199" "the trace uses the high fd 199"
_t_has "$body" '199>>' "and the runner opens 199 itself so the subject cannot swallow it"
# Every case is bounded and has stdin closed: a subject that reads stdin
# otherwise blocks on the caller's terminal for the whole timeout.
_t_has "$body" "timeout --kill-after=10s" "each case runs under a bounded timeout"
_t_has "$body" "</dev/null" "each case has stdin closed"
# A timed-out or failing case must be RECORDED, not swallowed: swallowing it is
# what let a deliberately-broken assertion exit 0 through the push gate.
_t_has "$body" "case timed out" "a timeout is reported"
_t_has "$body" "case exited" "a non-zero case is reported"
_t_has "$body" "case_failures" "and both are recorded to a case_failures file"
_t_has "$body" "124" "SIGTERM-timeout (124) is treated as a timeout"
_t_has "$body" "137" "SIGKILL-timeout (137) is treated as a timeout too"
# PATH is pinned to the jail's shim dir FIRST, so a shimmed tool wins over the
# host's real one.
_t_has "$body" "PATH=\"${DOLLAR}jail/bin:" "the jail's bin/ is first on PATH"
# The heredoc is quoted, so the runner's own variables survive to the child
# shell rather than being expanded when the file is written.
_t_has "$body" "${DOLLAR}1" "the runner reads its arguments at run time"
_t_lacks "$body" "$JAIL" "the writing shell's JAIL path is not baked in"
_t_drop_jail
_t_summary

View File

@ -0,0 +1,63 @@
#!/usr/bin/env bash
# is_covered: the shell-coverage ratchet's predicate.
set -uo pipefail
HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# shellcheck source=./coverage_tool_harness.sh
. "$HERE/coverage_tool_harness.sh"
# shellcheck source=../shell_coverage_bar.sh
. "$HERE/../shell_coverage_bar.sh"
tmp="$(mktemp -d -t covbar-XXXXXX)"
trap 'rm -rf "$tmp"' EXIT
# A lib whose directory has no tests/ at all.
mkdir -p "$tmp/nosuite/lib"
: >"$tmp/nosuite/lib/thing.sh"
if is_covered "$tmp/nosuite/lib/thing.sh"; then
_t_fail "a lib with no tests/ dir is uncovered"
else
_t_pass "a lib with no tests/ dir is uncovered"
fi
# A tests/ directory that exists but has no run_all.sh: still uncovered. The
# suite is identified by its runner, not by the directory.
mkdir -p "$tmp/emptytests/lib/tests"
: >"$tmp/emptytests/lib/thing.sh"
: >"$tmp/emptytests/lib/tests/test_thing.sh"
if is_covered "$tmp/emptytests/lib/thing.sh"; then
_t_fail "a tests/ dir without run_all.sh is uncovered"
else
_t_pass "a tests/ dir without run_all.sh is uncovered"
fi
# The covered case.
mkdir -p "$tmp/good/lib/tests"
: >"$tmp/good/lib/thing.sh"
: >"$tmp/good/lib/tests/run_all.sh"
if is_covered "$tmp/good/lib/thing.sh"; then
_t_pass "a lib beside a tests/run_all.sh is covered"
else
_t_fail "a lib beside a tests/run_all.sh is covered"
fi
# The runner is looked up beside the LIB, not anywhere up the tree: a suite one
# directory up must not cover a nested lib.
mkdir -p "$tmp/nested/lib/tests" "$tmp/nested/lib/inner"
: >"$tmp/nested/lib/tests/run_all.sh"
: >"$tmp/nested/lib/inner/thing.sh"
if is_covered "$tmp/nested/lib/inner/thing.sh"; then
_t_fail "a suite one level up does not cover a nested lib"
else
_t_pass "a suite one level up does not cover a nested lib"
fi
# A run_all.sh that is a DIRECTORY is not a runner.
mkdir -p "$tmp/dirrunner/lib/tests/run_all.sh"
: >"$tmp/dirrunner/lib/thing.sh"
if is_covered "$tmp/dirrunner/lib/thing.sh"; then
_t_fail "a directory named run_all.sh is not a suite"
else
_t_pass "a directory named run_all.sh is not a suite"
fi
_t_summary

View File

@ -0,0 +1,115 @@
#!/usr/bin/env bash
# build_jail, write_cases, build_mount_script and reconcile_pass_failures.
set -uo pipefail
HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# shellcheck source=./coverage_tool_harness.sh
. "$HERE/coverage_tool_harness.sh"
EXTRA_SHIMS=()
BIND_PATHS=()
CASES=()
# shellcheck source=../shell_coverage_jail_setup.sh
. "$HERE/../shell_coverage_jail_setup.sh"
# --- build_jail -------------------------------------------------------------
# It mints its own $JAIL via mktemp, so it is the one function not given one.
EXTRA_SHIMS=(mytool)
build_jail
_t_eq "yes" "$([[ -d "$JAIL/cov" && -d "$JAIL/trace" && -d "$JAIL/bin" ]] && echo yes)" \
"build_jail creates cov/, trace/ and bin/"
xtrace="$(cat "$JAIL/xtrace_env.sh")"
_t_has "$xtrace" 'PS4=' "the BASH_ENV file assigns PS4"
_t_has "$xtrace" 'set -x' "the BASH_ENV file re-applies set -x"
# The heredoc is quoted so these reach the traced shell unexpanded; if they had
# been expanded at write time the trace loses its per-line file:line prefix.
_t_has "$xtrace" "${DOLLAR}{BASH_SOURCE}" "BASH_SOURCE is left unexpanded for the traced shell"
_t_has "$xtrace" "${DOLLAR}{LINENO}" "LINENO is left unexpanded for the traced shell"
_t_eq "yes" "$([[ -x "$JAIL/bin/systemctl" ]] && echo yes)" \
"a default shim is written and executable"
_t_eq "yes" "$([[ -x "$JAIL/bin/mytool" ]] && echo yes)" \
"an EXTRA_SHIMS entry is shimmed too"
# A shim records the call and exits 0, so a subject calling systemctl does not
# die inside the jail.
"$JAIL/bin/systemctl" start nothing.service
_t_has "$(cat "$JAIL/calls.log")" "systemctl start nothing.service" \
"a shim logs its arguments to calls.log"
_t_eq "0" "$(
"$JAIL/bin/pkill" -9 anything >/dev/null 2>&1
echo $?
)" \
"a shim exits 0 rather than failing the subject"
built_jail="$JAIL"
# --- write_cases ------------------------------------------------------------
_t_new_jail
CASES=("" "--flag one" "two")
write_cases
_t_eq "3" "$(wc -l <"$JAIL/cases")" "write_cases writes one line per case"
_t_eq "--flag one" "$(sed -n '2p' "$JAIL/cases")" "a case keeps its arguments verbatim"
_t_eq "" "$(sed -n '1p' "$JAIL/cases")" "the empty case is preserved as a blank line"
_t_drop_jail
# --- build_mount_script -----------------------------------------------------
_t_new_jail
BIND_PATHS=(/etc /var/lib)
build_mount_script
mounts="$(cat "$JAIL/mounts.sh")"
_t_has "$mounts" "mkdir -p /etc" "each bind target is mkdir'd first"
_t_has "$mounts" "mount --bind" "each bind emits a mount --bind"
_t_eq "2" "$(grep -c 'mount --bind' "$JAIL/mounts.sh")" "one mount per bind path"
# A failed bind must be fatal: a silent failure sends the subject's writes at
# the REAL path, which is the misdiagnosis the jail exists to prevent.
_t_has "$mounts" "exit 1" "a failed bind exits rather than warning"
# Distinct source dirs, so two binds cannot collide on one mnt dir.
_t_has "$mounts" "mnt0" "the first bind uses mnt0"
_t_has "$mounts" "mnt1" "the second bind uses mnt1"
_t_drop_jail
# An empty BIND_PATHS is legal and yields an empty script, not an error.
_t_new_jail
BIND_PATHS=()
build_mount_script
_t_eq "0" "$(grep -c 'mount --bind' "$JAIL/mounts.sh" || true)" \
"no bind paths yields no mounts"
_t_drop_jail
# --- reconcile_pass_failures ------------------------------------------------
# Equal failure counts: the trace is complete, so it passes and the kcov file
# becomes the one --fail-on-case-error reads.
_t_new_jail
printf '1\n' >"$JAIL/case_failures.kcov"
printf '1\n' >"$JAIL/case_failures.trace"
(reconcile_pass_failures) >/dev/null 2>&1
_t_eq "0" "$?" "equal failures in both passes is not an error"
reconcile_pass_failures >/dev/null 2>&1
_t_eq "yes" "$([[ -f "$JAIL/case_failures" ]] && echo yes)" \
"the kcov failures are collapsed into case_failures"
_t_drop_jail
# More trace failures than kcov failures means a PARTIAL trace: it must fail
# loudly, because a short trace under-reports and looks like real uncovered code.
_t_new_jail
: >"$JAIL/case_failures.kcov"
printf '1\n2\n' >"$JAIL/case_failures.trace"
out="$( (reconcile_pass_failures) 2>&1)"
rc=$?
_t_eq "1" "$rc" "a trace pass with MORE failures exits 1"
_t_has "$out" "trace is partial" "and says the trace is partial"
_t_has "$out" "raise --timeout" "and names the usual fix"
_t_drop_jail
# Neither pass failed: nothing to reconcile and no case_failures file.
_t_new_jail
(reconcile_pass_failures) >/dev/null 2>&1
_t_eq "0" "$?" "no failure files at all is not an error"
reconcile_pass_failures >/dev/null 2>&1
_t_eq "no" "$([[ -f "$JAIL/case_failures" ]] && echo yes || echo no)" \
"a clean run writes no case_failures file"
_t_drop_jail
rm -rf "$built_jail"
_t_summary