meta: measurable shell coverage, and dns_iptables.sh at 100%

kcov works here once invoked correctly, so shell coverage is measurable
after all. shell_coverage.sh wraps it as a gate; the XML parsing lives in
shell_coverage_report.py per the rule against inlining another language's
logic in a shell script.

Two kcov traps are documented in the wrapper because both fail silently.
It must be handed the test script DIRECTLY: given `bash script.sh` it
instruments the bash binary, finds no shell source, and reports 0/0 at
0.00% -- indistinguishable from "nothing is covered". And per-line detail
exists only in cov.xml; coverage.json carries the percentage alone, so it
cannot say which lines are missing.

First measurement put dns_iptables.sh at 68/74 = 91.89%, with all six
uncovered lines in error and edge branches: chain creation failing, jump
insertion failing, the stale-jump de-dupe log, and the v6 trusted-resolver
ACCEPT. The stub grew failure injection and jump seeding to reach them,
and four new cases take it to 74/74 = 100.00%.

The gate fails closed: deleting the three ensure_chain failure cases drops
it to 93.24%, names lines 23, 24, 37, 40, 41, and exits 1.

Not wired into pre-commit -- kcov is absent on CI, and most shell files
here have no test at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY
This commit is contained in:
Krzysztof Rudnicki 2026-08-17 15:29:08 +02:00
parent ce139b5867
commit a174b2a3c4
6 changed files with 294 additions and 3 deletions

View File

@ -0,0 +1,17 @@
{
"title": "measurable shell coverage, and dns_iptables.sh at 100%",
"objective": "Provide a working kcov-based line-coverage gate for shell libraries, then use it to close every uncovered line in dns_iptables.sh.",
"acceptance_criteria": [
"shell_coverage.sh reports dns_iptables.sh at 100.00% and exits 0",
"Removing coverage-critical tests drops it below 100%, names the uncovered lines, and exits 1",
"test_dns_iptables.sh reports 24 passed, 0 failed",
"The subject under test is unchanged: verify_shell_split.sh against 728301c still reports all 14 functions verbatim",
"pre-commit passes on every changed file with zero lint suppressions"
],
"out_of_scope": [
"Wiring coverage into pre-commit or CI — kcov is not installed on CI and most shell files have no test",
"Branch coverage, which kcov does not provide for shell",
"Any behavioural change to dns_iptables.sh"
],
"verifier": "bash meta/scripts/shell_coverage.sh phone_focus_mode/lib/tests/test_dns_iptables.sh dns_iptables.sh"
}

View File

@ -0,0 +1,56 @@
{
"intent": "Make shell line coverage measurable in this repo, and bring dns_iptables.sh to 100% now that it can be measured.",
"scope": [
"meta/scripts/shell_coverage.sh + shell_coverage_report.py — new coverage gate",
"phone_focus_mode/lib/tests/dns_iptables_harness.sh — failure injection and jump seeding added to the iptables stub",
"phone_focus_mode/lib/tests/test_dns_iptables.sh — 4 new cases, 20 -> 24",
"Non-goal: wiring coverage into pre-commit. kcov is not installed on CI and most shell files have no test at all."
],
"changes": [
"Added shell_coverage.sh, which runs a test under kcov and gates on a minimum percentage, defaulting to 100.",
"Two kcov behaviours are documented in the script because both are silent traps: it must be handed the test script DIRECTLY (given `bash script.sh` it instruments the bash binary and reports 0/0 at 0.00%, which reads like zero coverage rather than zero measurement), and per-line detail exists only in cov.xml, not in the coverage.json summary.",
"XML parsing lives in shell_coverage_report.py rather than inline, per the repo rule against embedding another language's logic in a shell script.",
"Extended the iptables stub with failure injection (_fail_op) and stale-jump seeding (_seed_jumps), which is what makes ensure_chain's error and de-dupe branches reachable.",
"Added 4 cases: chain creation failing, OUTPUT jump insertion failing, several stale jumps collapsing to one, and the v6 mirror of the ACCEPT-before-REJECT ordering rule."
],
"verification": [
{
"command": "bash meta/scripts/shell_coverage.sh phone_focus_mode/lib/tests/test_dns_iptables.sh dns_iptables.sh",
"result": "pass",
"evidence": "'dns_iptables.sh: 74/74 lines = 100.00%', exit 0. Before the 4 new cases it measured 68/74 = 91.89% with lines 23, 24, 37, 40, 41 and 99 uncovered — every one an error or edge branch."
},
{
"command": "gate check: delete the three ensure_chain failure cases and re-measure",
"result": "fail-as-expected",
"evidence": "'dns_iptables.sh: 69/74 lines = 93.24%', 'uncovered: 23, 24, 37, 40, 41', 'coverage 93.24% is below the required 100.00%', exit 1. Restoring the cases returns 74/74. The gate fails closed and names the missing lines."
},
{
"command": "bash phone_focus_mode/lib/tests/test_dns_iptables.sh",
"result": "pass",
"evidence": "24 passed, 0 failed."
},
{
"command": "bash meta/scripts/verify_shell_split.sh 728301c phone_focus_mode/dns_enforcer.sh phone_focus_mode/dns_enforcer.sh phone_focus_mode/dns_iptables.sh",
"result": "pass",
"evidence": "'IDENTICAL: all 14 top-level functions moved verbatim' — the subject is unchanged by the harness work; only the tests grew."
},
{
"command": "bash phone_focus_mode/lib/tests/test_monitor.sh and test_tether_enforcer.sh",
"result": "pass",
"evidence": "9 passed / 0 failed and 17 passed / 0 failed, unchanged."
},
{
"command": "pre-commit run --files <the four changed files>",
"result": "pass",
"evidence": "All hooks pass with zero suppressions. Four ruff findings were fixed at the source: S314 by switching to defusedxml (already a pinned dependency), N817 by dropping the ET acronym alias, INP001 by adding the shebang its sibling scripts carry, and a T201 autofix that had silently replaced the tool's own output with `pass` was repaired by using sys.stdout.write, matching extract_shell_functions.py."
}
],
"risks": [
"kcov measures lines, not branches. A line containing `[ -n \"$x\" ] && do_thing` counts as covered when only one side is taken, so 100% here is weaker than the 100% branch coverage the Python gate enforces. The mutation checks are what cover that gap.",
"The gate is not wired into pre-commit, so nothing forces coverage to stay at 100% for this file. Doing so would require kcov on CI and would fail immediately for every shell file that has no test."
],
"rollback": [
"git revert the commit. The two tooling scripts disappear and the test returns to 20 assertions; the subject under test is untouched either way.",
"After rollback, run bash phone_focus_mode/lib/tests/test_dns_iptables.sh and confirm it still passes, since the tests do not depend on the coverage tooling."
]
}

71
meta/scripts/shell_coverage.sh Executable file
View File

@ -0,0 +1,71 @@
#!/usr/bin/env bash
# ============================================================================
# Measure line coverage of a shell library from its test script, via kcov.
#
# shell_coverage.sh <test-script> <subject-basename> [min-percent]
#
# Reports covered/total and the uncovered line numbers, and exits 1 when the
# result is below min-percent (default 100).
#
# Two things about kcov that cost an hour to find, both worth keeping:
#
# 1. It must be given the test script DIRECTLY, not `bash <script>`. Handed
# `bash foo.sh` it instruments the bash binary, finds no shell source,
# and reports 0/0 at 0.00% -- which reads exactly like "nothing is
# covered" rather than "nothing was measured".
# 2. Per-line detail is only in cov.xml. The summary coverage.json carries
# just the percentage, so it cannot tell you WHICH lines are missing.
#
# It does follow into subprocesses, so a harness that stages the subject into
# a temp dir and runs each case as a child still gets measured -- which is why
# --include-pattern matches on basename rather than an absolute path.
# ============================================================================
set -euo pipefail
readonly SCRIPT_NAME="${0##*/}"
usage() {
echo "Usage: $SCRIPT_NAME <test-script> <subject-basename> [min-percent]"
echo
echo " test-script the test to run, e.g. phone_focus_mode/lib/tests/test_x.sh"
echo " subject-basename the file to measure, e.g. dns_iptables.sh"
echo " min-percent fail below this (default 100)"
exit 0
}
[[ $# -ge 1 && ( "$1" == "-h" || "$1" == "--help" ) ]] && usage
if [[ $# -lt 2 ]]; then
echo "Error: need a test script and a subject basename" >&2
exit 1
fi
readonly TEST_SCRIPT="$1"
readonly SUBJECT="$2"
readonly MIN_PERCENT="${3:-100}"
if ! command -v kcov >/dev/null 2>&1; then
echo "Error: kcov is not installed (pacman -S kcov)" >&2
exit 1
fi
if [[ ! -f "$TEST_SCRIPT" ]]; then
echo "Error: no such test script: $TEST_SCRIPT" >&2
exit 1
fi
OUT_DIR="$(mktemp -d)"
readonly OUT_DIR
cleanup() { rm -rf "$OUT_DIR"; }
trap cleanup EXIT
# Direct invocation, per note 1 above.
if ! kcov --include-pattern="$SUBJECT" "$OUT_DIR" "$TEST_SCRIPT" >"$OUT_DIR/run.log" 2>&1; then
echo "Error: the test script failed under kcov; its output follows" >&2
tail -20 "$OUT_DIR/run.log" >&2
exit 1
fi
python3 "$(dirname "$0")/shell_coverage_report.py" "$OUT_DIR" "$SUBJECT" "$MIN_PERCENT"

View File

@ -0,0 +1,67 @@
#!/usr/bin/env python3
"""Report line coverage for one shell file from a kcov output directory.
Reads kcov's cobertura-style ``cov.xml`` rather than its ``coverage.json``:
the JSON summary carries only a percentage, so it cannot say which lines are
missing, which is the only part that helps close a gap.
Exits 1 when coverage is below the requested minimum, so the caller can use
it as a gate.
"""
from __future__ import annotations
from pathlib import Path
import sys
from defusedxml import ElementTree
def _measure(out_dir: Path, subject: str) -> tuple[int, int, list[str]]:
"""Return (covered, total, uncovered line numbers) for *subject*.
kcov writes one report per instrumented process. A harness that stages
the subject into a temp dir produces several, so the entries are merged:
a line counts as covered if any process hit it.
"""
hits: dict[str, int] = {}
for xml_path in out_dir.rglob("cov.xml"):
for cls in ElementTree.parse(xml_path).iter("class"):
filename = cls.get("filename", "")
if Path(filename).name != subject:
continue
for line in cls.iter("line"):
number = line.get("number")
if number is None:
continue
hits[number] = hits.get(number, 0) + int(line.get("hits", "0"))
uncovered = sorted((n for n, h in hits.items() if h == 0), key=int)
return len(hits) - len(uncovered), len(hits), uncovered
def main() -> None:
"""Print the coverage summary and gate on the minimum percentage."""
out_dir, subject, minimum = Path(sys.argv[1]), sys.argv[2], float(sys.argv[3])
covered, total, uncovered = _measure(out_dir, subject)
if total == 0:
sys.exit(
f"{subject}: kcov instrumented no lines. It was probably invoked as "
f"`kcov ... bash <script>`; hand it the script directly instead."
)
percent = 100.0 * covered / total
# sys.stdout.write rather than print, matching extract_shell_functions.py:
# this is the tool's result, not debug output, and ruff's T201 autofix
# silently deletes bare print() calls.
sys.stdout.write(f"{subject}: {covered}/{total} lines = {percent:.2f}%\n")
if uncovered:
sys.stdout.write(f" uncovered: {', '.join(uncovered)}\n")
if percent < minimum:
sys.exit(f"coverage {percent:.2f}% is below the required {minimum:.2f}%")
if __name__ == "__main__":
main()

View File

@ -68,19 +68,45 @@ jump="${store}/jump"
touch "$rules"
printf '%s %s\n' "$bin" "$*" >>"${IPT_STATE}/calls.log"
# Failure injection. A test writes the operation it wants to fail into
# ${IPT_STATE}/fail_<op>; the stub then reports that operation as failing,
# which is how the error branches inside ensure_chain become reachable.
_fails() { [[ -f "${IPT_STATE}/fail_$1" ]]; }
# Pre-seeded jump count, so the de-dupe path (more than one existing jump)
# can be exercised. Defaults to the single flag file.
_jumps_file="${store}/jumps"
[[ -f "$_jumps_file" ]] || { [[ -f "$jump" ]] && echo 1 >"$_jumps_file"; }
case "$1" in
-L) [[ -f "${store}/chain" ]] && exit 0 || exit 1 ;;
-N) touch "${store}/chain"; exit 0 ;;
-N)
_fails N && exit 1
touch "${store}/chain"
exit 0
;;
-F) : >"$rules"; exit 0 ;;
-A)
shift
printf -- '-A %s\n' "$*" >>"$rules"
exit 0
;;
-I) touch "$jump"; exit 0 ;;
-I)
_fails I && exit 1
touch "$jump"
echo 1 >"$_jumps_file"
exit 0
;;
-D)
# Succeeds once per existing jump, so the de-dupe loop terminates.
if [[ -f "$jump" ]]; then rm -f "$jump"; exit 0; fi
n=0
[[ -f "$_jumps_file" ]] && read -r n <"$_jumps_file"
if ((n > 0)); then
echo $((n - 1)) >"$_jumps_file"
((n - 1 == 0)) && rm -f "$jump"
exit 0
fi
rm -f "$jump"
exit 1
;;
-C) [[ -f "$jump" ]] && exit 0 || exit 1 ;;
@ -130,3 +156,19 @@ _reset_ipt() {
rm -rf "${RUN}/ipt"
mkdir -p "${RUN}/ipt"
}
# Make the next run report the given iptables operation as failing, so the
# error branches in ensure_chain are reachable. "N" is chain creation,
# "I" is inserting the OUTPUT jump.
_fail_op() {
touch "${RUN}/ipt/fail_$1"
}
# Pre-seed the chain as existing with N stale OUTPUT jumps, which is the
# lock-race state ensure_chain's de-dupe loop exists to clean up.
_seed_jumps() {
local bin="$1" count="$2"
mkdir -p "${RUN}/ipt/${bin}"
touch "${RUN}/ipt/${bin}/chain" "${RUN}/ipt/${bin}/jump"
echo "$count" >"${RUN}/ipt/${bin}/jumps"
}

View File

@ -102,6 +102,32 @@ CASE
)" \
"is idempotent across repeated calls"
_reset_ipt
_fail_op N
_t_eq "1" "$(_with_subject <<'CASE'
ensure_chain iptables >/dev/null 2>&1; echo $?
CASE
)" \
"fails when the chain cannot be created"
_reset_ipt
_fail_op I
_t_eq "1" "$(_with_subject <<'CASE'
ensure_chain iptables >/dev/null 2>&1; echo $?
CASE
)" \
"fails when the OUTPUT jump cannot be inserted"
_reset_ipt
# The lock-race state the de-dupe loop exists for: several stale jumps, which
# must collapse to exactly one without the call failing.
_seed_jumps iptables 3
_t_eq "0" "$(_with_subject <<'CASE'
ensure_chain iptables >/dev/null 2>&1; echo $?
CASE
)" \
"collapses several stale OUTPUT jumps and still succeeds"
printf '\nfill_chain_v4 / fill_chain_v6\n'
_reset_ipt
@ -134,6 +160,18 @@ CASE
)" \
"fills v6 to 2 fixed + 4 for the single DoH v6 IP"
_reset_ipt
# The v6 mirror of the ACCEPT-before-REJECT rule. Needs a trusted v6 literal,
# since trusted_dot_ips filters by family.
_t_eq "ACCEPT" "$(TEST_DOT_HOST=dns.example.net TEST_DOT_IPS='9.9.9.9 2620:fe::9' \
_with_subject <<'CASE'
ensure_chain ip6tables >/dev/null 2>&1
fill_chain_v6 >/dev/null 2>&1
ip6tables -S "$DNS_IPT_CHAIN" | head -1 | grep -o "ACCEPT\|REJECT"
CASE
)" \
"puts the trusted v6 resolver ACCEPT before the blanket REJECT"
printf '\nchain_intact\n'
_reset_ipt