mirror of
https://github.com/kuhyx/testsAndMisc.git
synced 2026-11-10 20:27:02 +01:00
Fix both kcov defects: dwm_config 47.95% -> 98.48%, rpi_nc_install 11.36% -> 96.59%
The instrument, not the tests, was the bottleneck. Coverage is now
(PS4-traced lines | kcov hits) & (kcov line set - continuation lines).
The trace deliberately does NOT supply the denominator. A trace reports only
lines that RAN, so using it for both halves would make every subject 100.00%
and arm a repo-wide gate that checks nothing.
Three measured findings, each of which yields a silently wrong trace:
1. kcov's ptrace and bash's xtrace cannot share a process. Under
SHELLOPTS=xtrace every line kcov recorded as hits="1" comes back hits="0"
-- reproduced A/B on one subject. Hence two passes, two namespaces.
2. SHELLOPTS is readonly inside bash, so `export SHELLOPTS=xtrace` fails and
tracing never turns on; and `set -x` does not reach child processes.
3. Decisive: bash under `unshare --map-root-user` runs PRIVILEGED and
DISCARDS an inherited PS4, falling back to "+ " while still honouring
xtrace. The trace loses its file:line prefix and every subject reads 0%.
Fixed by delivering PS4 and `set -x` through a BASH_ENV file, which also
solves child-process propagation.
A fourth trap, in the report: kcov records filename="dwm_config.sh", a bare
basename. The source path comes from the trace's ${BASH_SOURCE} or a unique
repo search; an ambiguous basename yields no exclusion rather than a guess.
Measured through run_all.sh, nothing down:
dot_resolver_install.sh 39/39 = 100.00% (acceptance, unchanged)
rpi_nc_ca.sh 73/73 = 100.00% (was 72/73 = 98.63%)
dwm_config.sh 65/66 = 98.48% (was 35/73 = 47.95%)
rpi_nc_install.sh 85/88 = 96.59% (was 10/88 = 11.36%)
rpi_nc_ca reaching 100% is the load-bearing check: its missing line 141 was
cross-process contagion, and the trace pass has no kcov in it to contaminate.
dwm_config's remaining line 39 is a `done < <(...)`. Left in the denominator
on purpose -- removing lines inflates coverage, the fail-open direction, so a
second exclusion rule needs per-line justification, not a pattern from one lib.
Both new Python modules hold 100% branch coverage (29 tests).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
078d3463bd
commit
81191f40e6
@ -1,5 +1,12 @@
|
||||
# kcov under-reports shell coverage: two distinct defects
|
||||
|
||||
> **RESOLVED 2026-08-22.** Both defects are fixed; the instrument now
|
||||
> corrects for them and the numbers below are the *historical* readings that
|
||||
> motivated the work. Kept because the disproven hypotheses are still worth
|
||||
> not retrying, and because the two root causes are the kind that silently
|
||||
> come back. What the fix does, and the three findings that make it work, is
|
||||
> at the end of this file under "How it was fixed".
|
||||
|
||||
Split out of `docs/shell-split-verification.md` on 2026-08-22 to hold both
|
||||
files under the repo's 250-line cap. This is the campaign's instrument
|
||||
problem; read it before trusting any coverage percentage.
|
||||
@ -97,3 +104,59 @@ surfaced by `--fail-on-case-error`. Temporarily ending the suite with
|
||||
`exit 42` at a chosen point turns "did execution reach here?" into a yes/no
|
||||
the jail will answer. That is what proved the second pattern is a tracing
|
||||
failure and not an aborted suite.
|
||||
|
||||
## How it was fixed (2026-08-22)
|
||||
|
||||
The instrument now runs **two passes over the same cases, in two fresh
|
||||
namespaces**, and combines them:
|
||||
|
||||
* **denominator** = kcov's instrumentable line set, minus continuation lines
|
||||
of multi-line quoted arguments (defect (a), detected by tracking quote
|
||||
state across the file in `meta/scripts/lib/shell_coverage_lines.py`);
|
||||
* **numerator** = (PS4-traced lines ∪ kcov's own hits) ∩ denominator
|
||||
(defect (b)).
|
||||
|
||||
The trace cannot supply the denominator. A trace only ever reports lines that
|
||||
*ran*, so using it for both halves would make every subject 100.00% and gate
|
||||
on nothing. kcov's line set is what keeps the gate meaningful; kcov's *hits*
|
||||
are what could not be trusted.
|
||||
|
||||
### Three findings, each of which silently produces a WRONG trace
|
||||
|
||||
1. **kcov and xtrace cannot share a process.** Under `SHELLOPTS=xtrace`,
|
||||
every line kcov had recorded as `hits="1"` comes back `hits="0"` —
|
||||
reproduced minimally, A/B, on the same subject. Hence two passes.
|
||||
2. **`SHELLOPTS` is a readonly variable inside bash.** `export
|
||||
SHELLOPTS=xtrace` fails with "readonly variable" and tracing never turns
|
||||
on. It only works placed in the environment *of* the process.
|
||||
3. **Decisive: bash under `unshare --user --map-root-user` runs in PRIVILEGED
|
||||
mode and DISCARDS an inherited `PS4`**, falling back to the default `+ `,
|
||||
while still honouring `SHELLOPTS=xtrace` and `BASH_XTRACEFD`. The trace
|
||||
then carries no `file:line` prefix at all, so every subject reads as 0%
|
||||
covered. This is why the trace is delivered through a **`BASH_ENV` file**
|
||||
that assigns `PS4` and runs `set -x` from *inside* each shell — which also
|
||||
solves propagation into child processes, since `set -x` does not inherit.
|
||||
|
||||
A fourth trap, in the report rather than the jail: **kcov records
|
||||
`filename="dwm_config.sh"`, a bare basename, not a path.** The source
|
||||
location has to come from the trace (which carries `${BASH_SOURCE}`) or from
|
||||
a repo search; an ambiguous basename yields no exclusion rather than a guess.
|
||||
|
||||
### Measured effect, all through `run_all.sh`
|
||||
|
||||
| lib | before | after |
|
||||
| --- | --- | --- |
|
||||
| `dot_resolver_install.sh` | 39/39 = 100.00% | 39/39 = 100.00% (acceptance) |
|
||||
| `rpi_nc_ca.sh` | 72/73 = 98.63% | **73/73 = 100.00%** |
|
||||
| `dwm_config.sh` | 35/73 = 47.95% | **65/66 = 98.48%** |
|
||||
| `rpi_nc_install.sh` | 10/88 = 11.36% | **85/88 = 96.59%** |
|
||||
|
||||
`rpi_nc_ca.sh` reaching 100.00% is the load-bearing check: its missing line
|
||||
141 was the *cross-process contagion* described above, and the trace pass has
|
||||
no kcov in it to be contaminated. Nothing went down.
|
||||
|
||||
`dwm_config.sh`'s remaining uncovered line 39 is a `done < <(...)` process
|
||||
substitution — the other half of defect (a). It is deliberately still in the
|
||||
denominator: every line removed from a denominator inflates coverage, which
|
||||
is the fail-open direction, so a second exclusion rule should be justified
|
||||
per line rather than generalised from one lib.
|
||||
|
||||
@ -0,0 +1,18 @@
|
||||
{
|
||||
"title": "Fix both kcov defects: correct denominator, and replace its numerator with a PS4 trace",
|
||||
"objective": "Make the shell-coverage instrument report the truth, so the allowlist campaign measures tests rather than tooling. kcov counted continuation lines of multi-line quoted arguments as statements (wrong denominator) and failed to record ordinary statements that provably execute (wrong numerator); the two together held dwm_config.sh at 47.95% and rpi_nc_install.sh at 11.36% regardless of how many tests were written.",
|
||||
"acceptance_criteria": [
|
||||
"dot_resolver_install.sh still measures 39/39 = 100.00% through run_all.sh, gating at --min 100.",
|
||||
"rpi_nc_ca.sh measures 73/73 = 100.00% -- it was 72/73 = 98.63% only because of cross-process contagion from test_dwm_config.sh, which a kcov-free trace pass cannot suffer.",
|
||||
"No lib's percentage goes DOWN. A drop means a lost trace, not a stricter instrument.",
|
||||
"The denominator is never derived from the trace. A PS4 trace reports only lines that ran, so trace-as-denominator would make every subject 100% and gate on nothing; kcov's line set is what keeps the gate meaningful.",
|
||||
"meta/scripts/lib/shell_coverage_lines.py and meta/scripts/shell_coverage_report.py both hold 100% branch coverage.",
|
||||
"pre-commit passes on every touched file with no suppression added."
|
||||
],
|
||||
"out_of_scope": [
|
||||
"Adding a second denominator-exclusion rule for `done < <(...)` process substitution. Every line removed from a denominator inflates coverage -- the fail-open direction -- so it must be justified per line, not generalised from one lib.",
|
||||
"Wiring check_shell_coverage.sh into pre-commit/pre-push/CI, which waits for the allowlist to reach zero.",
|
||||
"The unrelated red `Python tests` CI job (pip resolution-too-deep since 2026-08-17)."
|
||||
],
|
||||
"verifier": "bash meta/scripts/shell_coverage_jail.sh --subject linux_configuration/scripts/single_use/features/lib/tests/run_all.sh $(jail_args) --measure dot_resolver_install.sh --min 100 --fail-on-case-error -- \"\" and the same for rpi_nc_ca.sh (both must exit 0), plus python3 -m pytest python_pkg/tests/test_shell_coverage_lines.py python_pkg/tests/test_shell_coverage_report.py (expects 29 passed), plus pre-commit run --files on the touched files."
|
||||
}
|
||||
@ -0,0 +1,70 @@
|
||||
{
|
||||
"intent": "Make shell coverage measure the tests instead of the instrument: correct kcov's denominator (continuation lines of multi-line quoted arguments are not statements) and replace its unreliable numerator with a PS4 xtrace, so libs stop being capped at numbers no test can move.",
|
||||
"scope": [
|
||||
"meta/scripts/shell_coverage_jail.sh - runs two passes and reconciles their failures",
|
||||
"meta/scripts/lib/shell_coverage_case_runner.sh - per-pass case execution; BASH_ENV trace delivery",
|
||||
"meta/scripts/lib/shell_coverage_jail_setup.sh - trace dir, xtrace_env.sh, reconcile_pass_failures",
|
||||
"meta/scripts/lib/shell_coverage_lines.py - new: statement/executed line classification",
|
||||
"meta/scripts/shell_coverage_report.py - union arithmetic and source resolution",
|
||||
"python_pkg/tests/test_shell_coverage_lines.py, python_pkg/tests/test_shell_coverage_report.py - new tests",
|
||||
"docs/kcov-under-report.md - records the fix and the three root causes",
|
||||
"Non-goal: wiring the gate into pre-commit/pre-push/CI (waits for the allowlist to reach zero)",
|
||||
"Non-goal: excluding `done < <(...)` from the denominator (fail-open direction; needs per-line justification)"
|
||||
],
|
||||
"changes": [
|
||||
"Two passes over the same cases in two fresh namespaces: kcov's ptrace and bash's xtrace are mutually exclusive, measured A/B -- under SHELLOPTS=xtrace every kcov hit collapses from 1 to 0. Pass one supplies the instrumentable line set, pass two the executed set.",
|
||||
"Coverage = (PS4-traced lines UNION kcov hits) INTERSECT (kcov line set MINUS continuation lines). The trace deliberately does not supply the denominator: it can only report lines that ran, so trace-as-denominator would make every subject 100% and gate on nothing.",
|
||||
"Trace delivered via a BASH_ENV file that assigns PS4 and runs `set -x` inside each shell. Required because bash under `unshare --map-root-user` runs privileged and DISCARDS an inherited PS4 (falling back to the default '+ ', losing the file:line prefix entirely), and because `set -x` does not propagate to child processes while SHELLOPTS is readonly inside bash.",
|
||||
"Continuation lines detected by tracking single/double quote state across the source, honouring backslash escapes in double quotes only and treating an unquoted # as end-of-line.",
|
||||
"Source path resolved from the trace's ${BASH_SOURCE} or a unique repo search, because kcov records only a bare basename; an ambiguous basename yields no exclusion rather than a wrong guess.",
|
||||
"The trace pass failing more cases than the kcov pass is a hard error: a partially-traced case under-reports and looks exactly like the defect being fixed.",
|
||||
"Traced lines absent from kcov's line set are counted and reported rather than silently dropped -- a non-zero count is a third defect the design cannot correct for."
|
||||
],
|
||||
"verification": [
|
||||
{
|
||||
"command": "shell_coverage_jail.sh --measure dot_resolver_install.sh --min 100 --fail-on-case-error (through run_all.sh)",
|
||||
"result": "pass",
|
||||
"evidence": "dot_resolver_install.sh: 39/39 lines = 100.00%, exit 0. Acceptance baseline unchanged by the instrument change."
|
||||
},
|
||||
{
|
||||
"command": "shell_coverage_jail.sh --measure rpi_nc_ca.sh --min 100 --fail-on-case-error (through run_all.sh)",
|
||||
"result": "pass",
|
||||
"evidence": "rpi_nc_ca.sh: 73/73 lines = 100.00%, exit 0 -- was 72/73 = 98.63%. Line 141 was lost to cross-process contagion from test_dwm_config.sh; the trace pass has no kcov in it to contaminate. This is the discriminating check that the numerator fix landed."
|
||||
},
|
||||
{
|
||||
"command": "shell_coverage_jail.sh --measure dwm_config.sh (through run_all.sh)",
|
||||
"result": "pass",
|
||||
"evidence": "35/73 = 47.95% -> 65/66 = 98.48%. Denominator lost exactly the 7 continuation lines 64-66 and 98-101; numerator rose from 35 to 65. Remaining uncovered line 39 is a `done < <(...)`, left in the denominator deliberately."
|
||||
},
|
||||
{
|
||||
"command": "shell_coverage_jail.sh --measure rpi_nc_install.sh (through run_all.sh)",
|
||||
"result": "pass",
|
||||
"evidence": "10/88 = 11.36% -> 85/88 = 96.59%. The lib was never test-limited; its 29 assertions already passed."
|
||||
},
|
||||
{
|
||||
"command": "python3 -m pytest python_pkg/tests/test_shell_coverage_lines.py python_pkg/tests/test_shell_coverage_report.py",
|
||||
"result": "pass",
|
||||
"evidence": "29 passed."
|
||||
},
|
||||
{
|
||||
"command": "python3 -m coverage run --branch --source=meta/scripts -m pytest (the two new test modules); coverage report -m",
|
||||
"result": "pass",
|
||||
"evidence": "meta/scripts/lib/shell_coverage_lines.py 53 stmts 38 branches 100%; meta/scripts/shell_coverage_report.py 61 stmts 28 branches 100%."
|
||||
},
|
||||
{
|
||||
"command": "pre-commit run --files (the seven touched code files)",
|
||||
"result": "pass",
|
||||
"evidence": "All hooks pass. Three real lint findings were fixed at source, not suppressed: EXE001 (shebang without exec bit), PLR2004 (magic argv index named _ARGV_WITH_TRACE), E501 (long test signatures wrapped)."
|
||||
}
|
||||
],
|
||||
"risks": [
|
||||
"Every measurement now runs the suite twice, so an --all sweep takes roughly double the wall time. That is why the gate belongs in CI rather than a commit hook.",
|
||||
"xtrace is slower than an untraced run, so a case near its --timeout under kcov can exceed it under trace. This fails loudly via the pass-failure reconciliation rather than silently under-reporting.",
|
||||
"A subject that runs `set +x`, inspects $-, or opens fd 199 would corrupt its own trace. Neither construct exists in any current subject (grepped), and fd 199 was chosen over a low fd for this reason.",
|
||||
"The continuation-line detector is a quote-state scanner, not a bash parser. Over-detection would inflate coverage; it was validated against dwm_config.sh, where it flags exactly the 7 known lines and nothing else."
|
||||
],
|
||||
"rollback": [
|
||||
"git revert the commit; the previous instrument is self-contained in the same five files.",
|
||||
"After rollback, re-measure dot_resolver_install.sh (expect 39/39) and rpi_nc_ca.sh (expect the old 72/73 = 98.63%) to confirm the old behaviour is back, and restore the allowlist entries for any lib cleared on the strength of the new numbers."
|
||||
]
|
||||
}
|
||||
@ -11,6 +11,41 @@
|
||||
# command because it executes in the child namespace: a quoted inline block
|
||||
# would have to defer every expansion to the child shell, which reads as a
|
||||
# quoting bug and trips SC2016.
|
||||
#
|
||||
# TWO PASSES, and they cannot be merged. kcov instruments via ptrace; bash's
|
||||
# xtrace is a shell feature. Running both over one process was measured to
|
||||
# destroy kcov's numerator outright -- with SHELLOPTS=xtrace set, every line
|
||||
# kcov had recorded as hits="1" comes back hits="0". So:
|
||||
#
|
||||
# pass "kcov" -> the instrumentable LINE SET (the denominator) plus kcov's
|
||||
# own hit counts, which are correct for some lines and
|
||||
# missing for others (defect (b) in docs/kcov-under-report.md).
|
||||
# pass "trace" -> the EXECUTED line set, from a PS4 xtrace. This is the
|
||||
# authoritative numerator; kcov's hits are unioned in so a
|
||||
# line either instrument saw counts as covered.
|
||||
#
|
||||
# Tracing is delivered via BASH_ENV, not SHELLOPTS/PS4, and the reason is
|
||||
# measured, not stylistic. Three findings, each of which yields a silently
|
||||
# WRONG trace rather than an error:
|
||||
#
|
||||
# 1. `set -x` does NOT reach child bash processes, and the suites drive
|
||||
# their subjects several processes deep.
|
||||
# 2. SHELLOPTS is a READONLY variable inside bash, so `export
|
||||
# SHELLOPTS=xtrace` fails with "readonly variable" and tracing never
|
||||
# turns on. It only works placed in the environment OF the process.
|
||||
# 3. Decisive: bash started under `unshare --user --map-root-user` runs in
|
||||
# PRIVILEGED mode (uid != euid) and DISCARDS an inherited PS4, falling
|
||||
# back to the default "+ " -- while still honouring SHELLOPTS=xtrace.
|
||||
# The trace then carries no file:line prefix at all and every subject
|
||||
# reads as 0% covered.
|
||||
#
|
||||
# BASH_ENV survives all three: every non-interactive bash sources it at
|
||||
# startup, so PS4 is ASSIGNED from inside the shell (where privileged mode
|
||||
# cannot strip it) and `set -x` is re-applied per process, including children.
|
||||
#
|
||||
# BASH_XTRACEFD uses fd 199, not a low fd: subjects and harnesses open their
|
||||
# own descriptors, and a subject doing `exec 9>...` would silently clobber the
|
||||
# trace mid-run and truncate it.
|
||||
# ============================================================================
|
||||
|
||||
write_case_runner() {
|
||||
@ -26,6 +61,7 @@ subject_dir="$2"
|
||||
subject_base="$3"
|
||||
measure_base="$4"
|
||||
case_timeout="$5"
|
||||
pass="$6"
|
||||
. "$jail/mounts.sh"
|
||||
export PATH="$jail/bin:/usr/bin:/bin"
|
||||
export HOME="$jail/home"
|
||||
@ -33,22 +69,40 @@ USER="${USER:-root}"
|
||||
export USER
|
||||
export SUDO_USER="$USER"
|
||||
cd "$subject_dir" || exit 1
|
||||
case_index=0
|
||||
while IFS= read -r line; do
|
||||
read -r -a case_args <<<"$line"
|
||||
case_index=$((case_index + 1))
|
||||
# Bounded, and with stdin closed. Both matter: a subject that reads stdin
|
||||
# blocks forever on the caller's terminal (a `crontab -` fallback did
|
||||
# exactly this and hung a run for the full two minutes with no output),
|
||||
# and a subject that spins never yields at all. A timed-out case is
|
||||
# reported rather than swallowed, because a silent timeout looks
|
||||
# identical to an unreachable line in the coverage report.
|
||||
timeout --kill-after=10s "$case_timeout" \
|
||||
kcov --include-pattern="$measure_base" "$jail/cov" \
|
||||
"./$subject_base" ${case_args[@]+"${case_args[@]}"} \
|
||||
>/dev/null 2>&1 </dev/null
|
||||
rc=$?
|
||||
if [[ $pass == kcov ]]; then
|
||||
timeout --kill-after=10s "$case_timeout" \
|
||||
kcov --include-pattern="$measure_base" "$jail/cov" \
|
||||
"./$subject_base" ${case_args[@]+"${case_args[@]}"} \
|
||||
>/dev/null 2>&1 </dev/null
|
||||
rc=$?
|
||||
else
|
||||
# PS4 carries the full BASH_SOURCE, not ${BASH_SOURCE##*/}: two libs
|
||||
# in different directories can share a basename, and the report
|
||||
# filters by basename itself.
|
||||
#
|
||||
# The trace fd is opened HERE and inherited, so the subject's own
|
||||
# stdout/stderr redirections (the jail sends both to /dev/null)
|
||||
# cannot swallow the trace -- a 2>/dev/null once cost a bogus
|
||||
# "0 lines traced" reading.
|
||||
timeout --kill-after=10s "$case_timeout" \
|
||||
env "BASH_XTRACEFD=199" "BASH_ENV=$jail/xtrace_env.sh" \
|
||||
"./$subject_base" ${case_args[@]+"${case_args[@]}"} \
|
||||
>/dev/null 2>&1 </dev/null 199>>"$jail/trace/case.$case_index"
|
||||
rc=$?
|
||||
fi
|
||||
if [[ $rc -eq 124 || $rc -eq 137 ]]; then
|
||||
printf 'warn: case timed out after %s: %s\n' "$case_timeout" "$line" >&2
|
||||
printf '%s\n' "$rc" >>"$jail/case_failures"
|
||||
printf '%s\n' "$rc" >>"$jail/case_failures.$pass"
|
||||
elif [[ $rc -ne 0 ]]; then
|
||||
# A non-zero case is only noise when measuring (an installer that
|
||||
# exits 1 on a bogus arg still yields coverage), but it is the whole
|
||||
@ -56,7 +110,7 @@ while IFS= read -r line; do
|
||||
# caller decide; swallowing it made a deliberately-broken assertion
|
||||
# exit 0 through the push gate.
|
||||
printf 'warn: case exited %d: %s\n' "$rc" "$line" >&2
|
||||
printf '%s\n' "$rc" >>"$jail/case_failures"
|
||||
printf '%s\n' "$rc" >>"$jail/case_failures.$pass"
|
||||
fi
|
||||
done <"$jail/cases"
|
||||
INNER
|
||||
|
||||
@ -17,7 +17,23 @@ readonly DEFAULT_SHIMS=(
|
||||
|
||||
build_jail() {
|
||||
JAIL="$(mktemp -d)"
|
||||
mkdir -p "$JAIL/bin" "$JAIL/cov" "$JAIL/home" "$JAIL/etc"
|
||||
# cov holds kcov's output (the line SET); trace holds the PS4 xtrace
|
||||
# output (the executed-line set). They are two SEPARATE passes because
|
||||
# kcov's ptrace and SHELLOPTS=xtrace cannot share a process -- measured:
|
||||
# under xtrace every kcov hit count collapses to 0.
|
||||
mkdir -p "$JAIL/bin" "$JAIL/cov" "$JAIL/trace" "$JAIL/home" "$JAIL/etc"
|
||||
|
||||
# Sourced by every non-interactive bash in the trace pass (via BASH_ENV).
|
||||
# PS4 must be ASSIGNED here rather than inherited: bash under
|
||||
# `unshare --map-root-user` runs privileged and drops an inherited PS4,
|
||||
# which silently costs the trace its file:line prefix. `set -x` is
|
||||
# likewise re-applied per process, which is what reaches child shells.
|
||||
# Quoted heredoc: ${BASH_SOURCE}/${LINENO} must reach the traced shell
|
||||
# unexpanded so they evaluate per traced line.
|
||||
cat >"$JAIL/xtrace_env.sh" <<'XTRACE'
|
||||
PS4='+PS4:${BASH_SOURCE}:${LINENO} '
|
||||
set -x
|
||||
XTRACE
|
||||
|
||||
local tool
|
||||
for tool in "${DEFAULT_SHIMS[@]}" ${EXTRA_SHIMS[@]+"${EXTRA_SHIMS[@]}"}; do
|
||||
@ -107,3 +123,29 @@ build_mount_script() {
|
||||
done
|
||||
} >"$JAIL/mounts.sh"
|
||||
}
|
||||
|
||||
# A case that passes under kcov but fails under xtrace yields a PARTIAL trace,
|
||||
# which under-reports and looks exactly like the defect the trace pass exists
|
||||
# to fix. xtrace is slower, so a tight --timeout is the usual cause. Fail
|
||||
# loudly rather than silently measuring less.
|
||||
#
|
||||
# Also collapses the two per-pass failure files into the one --fail-on-case-error
|
||||
# reads, so that flag keeps its original meaning.
|
||||
reconcile_pass_failures() {
|
||||
local kcov_fail=0 trace_fail=0
|
||||
if [[ -s "$JAIL/case_failures.kcov" ]]; then
|
||||
kcov_fail="$(wc -l <"$JAIL/case_failures.kcov")"
|
||||
fi
|
||||
if [[ -s "$JAIL/case_failures.trace" ]]; then
|
||||
trace_fail="$(wc -l <"$JAIL/case_failures.trace")"
|
||||
fi
|
||||
if [[ $trace_fail -gt $kcov_fail ]]; then
|
||||
printf 'Error: trace pass had %d failing case(s) vs %d under kcov; the\n' \
|
||||
"$trace_fail" "$kcov_fail" >&2
|
||||
printf ' trace is partial. xtrace is slower -- raise --timeout.\n' >&2
|
||||
exit 1
|
||||
fi
|
||||
if [[ -s "$JAIL/case_failures.kcov" ]]; then
|
||||
cp "$JAIL/case_failures.kcov" "$JAIL/case_failures"
|
||||
fi
|
||||
}
|
||||
|
||||
109
meta/scripts/lib/shell_coverage_lines.py
Executable file
109
meta/scripts/lib/shell_coverage_lines.py
Executable file
@ -0,0 +1,109 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Line classification for shell coverage: what is a statement, and what ran.
|
||||
|
||||
Split out of ``shell_coverage_report.py`` to keep both files under the repo's
|
||||
250-line cap.
|
||||
|
||||
Two independent instruments feed the report, because neither is correct alone
|
||||
(see ``docs/kcov-under-report.md``):
|
||||
|
||||
* kcov supplies the *line set* -- which lines are instrumentable at all. Its
|
||||
hit counts are unreliable: ordinary statements execute without being
|
||||
recorded (defect (b)).
|
||||
* a PS4 xtrace supplies the *executed* set. A trace can only ever report lines
|
||||
that ran, so it cannot supply a denominator -- using it for both halves
|
||||
would make every subject 100% and gate on nothing.
|
||||
|
||||
So: denominator = kcov's line set minus non-statements; numerator =
|
||||
(traced lines | kcov's hits) & denominator.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
import re
|
||||
|
||||
# A PS4 trace line looks like: "+PS4:/abs/path/to/lib.sh:42 some command"
|
||||
# The leading '+' repeats with nesting depth, so it is matched loosely.
|
||||
_TRACE_RE = re.compile(r"\+*PS4:(?P<path>[^:]*):(?P<line>\d+)")
|
||||
|
||||
|
||||
def traced_lines(trace_dir: Path, subject: str) -> set[int]:
|
||||
"""Return the line numbers of *subject* seen executing in a PS4 trace."""
|
||||
seen: set[int] = set()
|
||||
if not trace_dir.is_dir():
|
||||
return seen
|
||||
for trace_file in sorted(trace_dir.iterdir()):
|
||||
if not trace_file.is_file():
|
||||
continue
|
||||
# errors="replace": a trace interleaves writes from concurrent
|
||||
# processes and can split a UTF-8 sequence. A decode error must not
|
||||
# discard the whole file, which would read as zero coverage.
|
||||
text = trace_file.read_text(encoding="utf-8", errors="replace")
|
||||
for match in _TRACE_RE.finditer(text):
|
||||
if Path(match.group("path")).name == subject:
|
||||
seen.add(int(match.group("line")))
|
||||
return seen
|
||||
|
||||
|
||||
def traced_paths(trace_dir: Path, subject: str) -> list[Path]:
|
||||
"""Return the distinct source paths a PS4 trace recorded for *subject*.
|
||||
|
||||
kcov's XML carries only a basename, so the trace is the one place the
|
||||
subject's real location is written down.
|
||||
"""
|
||||
found: list[Path] = []
|
||||
if not trace_dir.is_dir():
|
||||
return found
|
||||
for trace_file in sorted(trace_dir.iterdir()):
|
||||
if not trace_file.is_file():
|
||||
continue
|
||||
text = trace_file.read_text(encoding="utf-8", errors="replace")
|
||||
for match in _TRACE_RE.finditer(text):
|
||||
path = Path(match.group("path"))
|
||||
if path.name == subject and path not in found:
|
||||
found.append(path)
|
||||
return found
|
||||
|
||||
|
||||
def continuation_lines(source: Path) -> set[int]:
|
||||
"""Return lines that are *inside* a multi-line quoted argument.
|
||||
|
||||
kcov counts these as instrumentable statements that never run, which
|
||||
inflates the denominator and caps a subject's coverage below 100% no
|
||||
matter what the tests do (defect (a)). A `bash -x` tracer never reports
|
||||
them as executable at all, which is the correct reading: they are data
|
||||
inside an argument, not statements.
|
||||
|
||||
Detected by tracking quote state across the file: any line that *begins*
|
||||
while a quote opened on an earlier line is still open is a continuation.
|
||||
Only the opening line of such a construct is a statement.
|
||||
"""
|
||||
inside: set[int] = set()
|
||||
quote: str | None = None
|
||||
for number, raw in enumerate(
|
||||
source.read_text(encoding="utf-8", errors="replace").splitlines(), start=1
|
||||
):
|
||||
if quote is not None:
|
||||
inside.add(number)
|
||||
index = 0
|
||||
while index < len(raw):
|
||||
char = raw[index]
|
||||
if quote is None:
|
||||
if char == "#":
|
||||
# A comment outside quotes ends the line's significance.
|
||||
break
|
||||
if char == "\\":
|
||||
index += 2
|
||||
continue
|
||||
if char in ("'", '"'):
|
||||
quote = char
|
||||
elif char == quote:
|
||||
quote = None
|
||||
elif quote == '"' and char == "\\":
|
||||
# Only double quotes honour backslash escapes; inside single
|
||||
# quotes a backslash is a literal character.
|
||||
index += 2
|
||||
continue
|
||||
index += 1
|
||||
return inside
|
||||
@ -23,13 +23,11 @@
|
||||
# subject under `set -e` long before the interesting code.
|
||||
# 3. Mount only what the subject WRITES; preserve what it READS. A blanket
|
||||
# jail that masked /usr/local/bin cut pacman_wrapper.sh from 75.00% to
|
||||
# 28.95%, because the wrapper sources its sibling libs from there and
|
||||
# took its "libraries missing" escape hatch. An over-aggressive mount
|
||||
# looks exactly like unreachable code.
|
||||
# 28.95%: the wrapper sources its siblings from there and took its
|
||||
# "libraries missing" escape hatch. Over-mounting looks like dead code.
|
||||
#
|
||||
# Entry scripts must be executed in place: they resolve their libs via
|
||||
# BASH_SOURCE and common.sh via `readlink -f "$0"`, so staging a copy into a
|
||||
# temp dir (which works fine for a sourced lib) breaks them.
|
||||
# Entry scripts must be executed in place: they resolve libs via BASH_SOURCE
|
||||
# and common.sh via `readlink -f "$0"`, so staging a copy breaks them.
|
||||
# ============================================================================
|
||||
|
||||
set -euo pipefail
|
||||
@ -213,9 +211,18 @@ main() {
|
||||
done
|
||||
} >>"$JAIL/mounts.sh"
|
||||
|
||||
unshare --user --map-root-user --mount --fork \
|
||||
"$JAIL/run_cases.sh" "$JAIL" "$subject_dir" "$subject_base" "$measure_base" \
|
||||
"$CASE_TIMEOUT"
|
||||
# Two passes, two fresh namespaces; they cannot be merged (kcov's ptrace
|
||||
# and xtrace are mutually exclusive -- measured: under xtrace every kcov
|
||||
# hit collapses to 0). "kcov" gives the line set, "trace" gives the truth
|
||||
# about which of those lines ran. See lib/shell_coverage_case_runner.sh.
|
||||
local pass
|
||||
for pass in kcov trace; do
|
||||
unshare --user --map-root-user --mount --fork \
|
||||
"$JAIL/run_cases.sh" "$JAIL" "$subject_dir" "$subject_base" \
|
||||
"$measure_base" "$CASE_TIMEOUT" "$pass"
|
||||
done
|
||||
|
||||
reconcile_pass_failures
|
||||
|
||||
local tree_after
|
||||
tree_after="$(cd "$REPO_ROOT" && git status --porcelain)"
|
||||
@ -229,7 +236,7 @@ main() {
|
||||
# and suppressing it would hide the diagnosis behind the symptom.
|
||||
local report_rc=0
|
||||
python3 "$REPO_ROOT/meta/scripts/shell_coverage_report.py" \
|
||||
"$JAIL/cov" "$measure_base" "$MIN_PERCENT" || report_rc=$?
|
||||
"$JAIL/cov" "$measure_base" "$MIN_PERCENT" "$JAIL/trace" || report_rc=$?
|
||||
|
||||
if [[ $FAIL_ON_CASE_ERROR -eq 1 && -s "$JAIL/case_failures" ]]; then
|
||||
printf 'Error: %d case(s) exited non-zero; see the warn: lines above\n' \
|
||||
|
||||
@ -5,46 +5,119 @@ 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.
|
||||
|
||||
kcov alone is not trustworthy (``docs/kcov-under-report.md``): it counts
|
||||
continuation lines of multi-line quoted arguments as statements that never run
|
||||
(wrong denominator), and it fails to record ordinary statements that provably
|
||||
execute (wrong numerator). Both are corrected here -- the denominator by
|
||||
excluding non-statements, the numerator by unioning in a PS4 xtrace captured
|
||||
in a second pass.
|
||||
|
||||
Exits 1 when coverage is below the requested minimum, so the caller can use
|
||||
it as a gate.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from importlib.util import module_from_spec, spec_from_file_location
|
||||
from pathlib import Path
|
||||
import sys
|
||||
|
||||
from defusedxml import ElementTree
|
||||
|
||||
# Loaded by path rather than imported: meta/scripts/lib is not a package on
|
||||
# sys.path, and a sys.path.insert would need the import to sit below it,
|
||||
# which is an E402 the repo will not suppress.
|
||||
_LINES_PATH = Path(__file__).resolve().parent / "lib" / "shell_coverage_lines.py"
|
||||
_SPEC = spec_from_file_location("shell_coverage_lines", _LINES_PATH)
|
||||
if _SPEC is None or _SPEC.loader is None: # pragma: no cover - unreachable
|
||||
raise ImportError(_LINES_PATH)
|
||||
_LINES = module_from_spec(_SPEC)
|
||||
_SPEC.loader.exec_module(_LINES)
|
||||
continuation_lines = _LINES.continuation_lines
|
||||
traced_lines = _LINES.traced_lines
|
||||
|
||||
def _measure(out_dir: Path, subject: str) -> tuple[int, int, list[str]]:
|
||||
"""Return (covered, total, uncovered line numbers) for *subject*.
|
||||
|
||||
def _kcov_lines(out_dir: Path, subject: str) -> dict[int, int]:
|
||||
"""Return kcov's {line: hits} 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] = {}
|
||||
hits: dict[int, 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:
|
||||
if Path(cls.get("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"))
|
||||
key = int(number)
|
||||
hits[key] = hits.get(key, 0) + int(line.get("hits", "0"))
|
||||
return hits
|
||||
|
||||
uncovered = sorted((n for n, h in hits.items() if h == 0), key=int)
|
||||
return len(hits) - len(uncovered), len(hits), uncovered
|
||||
|
||||
def _find_source(subject: str, trace_dir: Path | None) -> Path | None:
|
||||
"""Locate *subject*'s source file so its non-statements can be excluded.
|
||||
|
||||
kcov records `filename="dwm_config.sh"` -- a bare BASENAME, not a path --
|
||||
so the XML cannot supply the source location. The PS4 trace can: it
|
||||
carries ${BASH_SOURCE}, an absolute path, for every line it reports.
|
||||
Falling back to a repo search keeps the exclusion working for a subject
|
||||
whose trace is empty.
|
||||
"""
|
||||
if trace_dir is not None:
|
||||
for candidate in _LINES.traced_paths(trace_dir, subject):
|
||||
if candidate.is_file():
|
||||
return candidate
|
||||
matches = sorted(Path(__file__).resolve().parents[2].rglob(subject))
|
||||
real = [m for m in matches if m.is_file() and ".git" not in m.parts]
|
||||
# Exactly one match, or the exclusion would be applied from the wrong
|
||||
# file. Ambiguity is reported by the caller as "no exclusions" rather
|
||||
# than silently guessing, which could only inflate coverage.
|
||||
return real[0] if len(real) == 1 else None
|
||||
|
||||
|
||||
def _measure(
|
||||
out_dir: Path, subject: str, trace_dir: Path | None
|
||||
) -> tuple[int, int, list[str], int]:
|
||||
"""Return (covered, total, uncovered line numbers, off-set trace count)."""
|
||||
hits = _kcov_lines(out_dir, subject)
|
||||
if not hits:
|
||||
return 0, 0, [], 0
|
||||
|
||||
source = _find_source(subject, trace_dir)
|
||||
excluded = continuation_lines(source) if source is not None else set()
|
||||
denominator = set(hits) - excluded
|
||||
|
||||
executed = {line for line, count in hits.items() if count > 0}
|
||||
traced: set[int] = set()
|
||||
if trace_dir is not None:
|
||||
traced = traced_lines(trace_dir, subject)
|
||||
|
||||
# Lines the trace saw executing that kcov never listed as instrumentable.
|
||||
# Dropped from the numerator (they are not in the denominator), but
|
||||
# surfaced: a non-zero count means kcov's LINE SET is wrong too, which is
|
||||
# a third defect this design cannot correct for.
|
||||
off_set = len(traced - set(hits) - excluded)
|
||||
|
||||
covered_set = (executed | traced) & denominator
|
||||
uncovered = sorted(denominator - covered_set)
|
||||
return len(covered_set), len(denominator), [str(n) for n in uncovered], off_set
|
||||
|
||||
|
||||
# argv: <cov-dir> <subject> <minimum> [<trace-dir>]. The trace dir is optional
|
||||
# so the report still runs against a kcov-only output directory.
|
||||
_ARGV_WITH_TRACE = 5
|
||||
|
||||
|
||||
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])
|
||||
trace_dir = Path(sys.argv[4]) if len(sys.argv) >= _ARGV_WITH_TRACE else None
|
||||
|
||||
covered, total, uncovered = _measure(out_dir, subject)
|
||||
covered, total, uncovered, off_set = _measure(out_dir, subject, trace_dir)
|
||||
if total == 0:
|
||||
sys.exit(
|
||||
f"{subject}: kcov instrumented no lines. It was probably invoked as "
|
||||
@ -58,6 +131,10 @@ def main() -> None:
|
||||
sys.stdout.write(f"{subject}: {covered}/{total} lines = {percent:.2f}%\n")
|
||||
if uncovered:
|
||||
sys.stdout.write(f" uncovered: {', '.join(uncovered)}\n")
|
||||
if off_set:
|
||||
sys.stdout.write(
|
||||
f" note: {off_set} traced line(s) are absent from kcov's line set\n"
|
||||
)
|
||||
|
||||
if percent < minimum:
|
||||
sys.exit(f"coverage {percent:.2f}% is below the required {minimum:.2f}%")
|
||||
|
||||
172
python_pkg/tests/test_shell_coverage_lines.py
Normal file
172
python_pkg/tests/test_shell_coverage_lines.py
Normal file
@ -0,0 +1,172 @@
|
||||
"""Tests for meta/scripts/lib/shell_coverage_lines.py.
|
||||
|
||||
This module decides what counts as a statement and what counts as executed,
|
||||
so both of its failure modes are silent and one-directional:
|
||||
|
||||
* over-detecting continuation lines shrinks the denominator and INFLATES
|
||||
coverage, which is the fail-open direction -- it could promote an untested
|
||||
lib off the allowlist;
|
||||
* losing trace lines shrinks the numerator, which merely keeps a lib on the
|
||||
allowlist that deserves to come off.
|
||||
|
||||
The first is the one worth pinning down hardest, so the quote-state cases
|
||||
below assert exact line sets rather than counts.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from types import ModuleType
|
||||
|
||||
_ROOT = Path(__file__).resolve().parents[2]
|
||||
_SCRIPT = _ROOT / "meta" / "scripts" / "lib" / "shell_coverage_lines.py"
|
||||
|
||||
|
||||
def _load() -> ModuleType:
|
||||
"""Import the module by path; it lives outside any importable package."""
|
||||
spec = importlib.util.spec_from_file_location("shell_coverage_lines", _SCRIPT)
|
||||
assert spec is not None
|
||||
assert spec.loader is not None
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
@pytest.fixture(name="mod")
|
||||
def _mod() -> ModuleType:
|
||||
return _load()
|
||||
|
||||
|
||||
def _write(tmp_path: Path, name: str, text: str) -> Path:
|
||||
path = tmp_path / name
|
||||
path.write_text(text, encoding="utf-8")
|
||||
return path
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# continuation_lines
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_single_line_quotes_are_not_continuations(
|
||||
mod: ModuleType, tmp_path: Path
|
||||
) -> None:
|
||||
"""A quote opened and closed on one line leaves no continuation."""
|
||||
src = _write(tmp_path, "a.sh", "echo 'hello'\necho \"world\"\n")
|
||||
assert mod.continuation_lines(src) == set()
|
||||
|
||||
|
||||
def test_multiline_single_quote_marks_inner_lines(
|
||||
mod: ModuleType, tmp_path: Path
|
||||
) -> None:
|
||||
"""This is the real dwm_config.sh perl-block shape: only the inner lines."""
|
||||
src = _write(tmp_path, "a.sh", "perl -pe '\ns/a/b/;\ns/c/d/;\n' file\necho after\n")
|
||||
# Line 1 opens the quote and IS a statement; 2-4 are inside it; 5 is after.
|
||||
assert mod.continuation_lines(src) == {2, 3, 4}
|
||||
|
||||
|
||||
def test_multiline_double_quote_marks_inner_lines(
|
||||
mod: ModuleType, tmp_path: Path
|
||||
) -> None:
|
||||
"""Double quotes span lines the same way single quotes do."""
|
||||
src = _write(tmp_path, "a.sh", 'x="\nline two\n"\necho after\n')
|
||||
assert mod.continuation_lines(src) == {2, 3}
|
||||
|
||||
|
||||
def test_comment_outside_quotes_ends_the_line(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""An apostrophe inside a comment must not open a quote state."""
|
||||
src = _write(tmp_path, "a.sh", "# don't be fooled\necho after\n")
|
||||
assert mod.continuation_lines(src) == set()
|
||||
|
||||
|
||||
def test_backslash_escape_outside_quotes_is_skipped(
|
||||
mod: ModuleType, tmp_path: Path
|
||||
) -> None:
|
||||
"""An escaped quote outside quotes does not open a quote state."""
|
||||
src = _write(tmp_path, "a.sh", "echo \\'\necho after\n")
|
||||
assert mod.continuation_lines(src) == set()
|
||||
|
||||
|
||||
def test_backslash_escape_inside_double_quotes(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""An escaped double quote inside double quotes does not close them."""
|
||||
src = _write(tmp_path, "a.sh", 'x="a\\"b\nstill inside\n"\n')
|
||||
assert mod.continuation_lines(src) == {2, 3}
|
||||
|
||||
|
||||
def test_backslash_inside_single_quotes_is_literal(
|
||||
mod: ModuleType, tmp_path: Path
|
||||
) -> None:
|
||||
"""Single quotes honour no escapes, so a trailing backslash closes nothing."""
|
||||
src = _write(tmp_path, "a.sh", "x='a\\'\necho after\n")
|
||||
assert mod.continuation_lines(src) == set()
|
||||
|
||||
|
||||
def test_hash_inside_quotes_is_not_a_comment(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""A # inside an open quote must not be treated as a comment."""
|
||||
src = _write(tmp_path, "a.sh", "x='# not a comment\nstill inside\n'\n")
|
||||
assert mod.continuation_lines(src) == {2, 3}
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# traced_lines / traced_paths
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_traced_lines_missing_dir_is_empty(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""A missing trace dir yields nothing rather than raising."""
|
||||
assert mod.traced_lines(tmp_path / "absent", "x.sh") == set()
|
||||
assert mod.traced_paths(tmp_path / "absent", "x.sh") == []
|
||||
|
||||
|
||||
def test_traced_lines_filters_by_basename(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""Only the subject's own lines are collected."""
|
||||
trace = tmp_path / "trace"
|
||||
trace.mkdir()
|
||||
(trace / "case.1").write_text(
|
||||
"+PS4:/a/b/subject.sh:12 cmd\n"
|
||||
"++PS4:/a/b/subject.sh:15 cmd\n"
|
||||
"+PS4:/a/b/other.sh:99 cmd\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
assert mod.traced_lines(trace, "subject.sh") == {12, 15}
|
||||
|
||||
|
||||
def test_traced_lines_skips_subdirectories(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""A directory inside the trace dir is not read as a trace file."""
|
||||
trace = tmp_path / "trace"
|
||||
(trace / "nested").mkdir(parents=True)
|
||||
(trace / "case.1").write_text("+PS4:/a/subject.sh:3 cmd\n", encoding="utf-8")
|
||||
assert mod.traced_lines(trace, "subject.sh") == {3}
|
||||
|
||||
|
||||
def test_traced_lines_survives_invalid_utf8(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""A split multi-byte sequence must not discard the whole trace."""
|
||||
trace = tmp_path / "trace"
|
||||
trace.mkdir()
|
||||
(trace / "case.1").write_bytes(b"+PS4:/a/subject.sh:7 cmd\n\xff\xfe\n")
|
||||
assert mod.traced_lines(trace, "subject.sh") == {7}
|
||||
|
||||
|
||||
def test_traced_paths_skips_subdirectories(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""A directory inside the trace dir is not read when resolving paths."""
|
||||
trace = tmp_path / "trace"
|
||||
(trace / "nested").mkdir(parents=True)
|
||||
(trace / "case.1").write_text("+PS4:/a/subject.sh:3 cmd\n", encoding="utf-8")
|
||||
assert mod.traced_paths(trace, "subject.sh") == [Path("/a/subject.sh")]
|
||||
|
||||
|
||||
def test_traced_paths_deduplicates(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""The same source path reported many times yields one entry."""
|
||||
trace = tmp_path / "trace"
|
||||
trace.mkdir()
|
||||
(trace / "case.1").write_text(
|
||||
"+PS4:/a/subject.sh:1 c\n+PS4:/a/subject.sh:2 c\n+PS4:/b/other.sh:2 c\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
assert mod.traced_paths(trace, "subject.sh") == [Path("/a/subject.sh")]
|
||||
233
python_pkg/tests/test_shell_coverage_report.py
Normal file
233
python_pkg/tests/test_shell_coverage_report.py
Normal file
@ -0,0 +1,233 @@
|
||||
"""Tests for meta/scripts/shell_coverage_report.py.
|
||||
|
||||
The report combines two instruments that are each wrong on their own
|
||||
(``docs/kcov-under-report.md``): kcov supplies the line SET but under-reports
|
||||
hits, and a PS4 trace supplies the executed set but cannot see a line that
|
||||
never ran. The arithmetic here is what makes the shell-coverage gate mean
|
||||
something, so the tests below pin the fail-open direction hardest: a trace
|
||||
must never be able to add a line that is not in the denominator.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from types import ModuleType
|
||||
|
||||
_ROOT = Path(__file__).resolve().parents[2]
|
||||
_SCRIPT = _ROOT / "meta" / "scripts" / "shell_coverage_report.py"
|
||||
|
||||
_XML = """<?xml version="1.0" ?>
|
||||
<coverage><packages><package><classes>
|
||||
<class name="s" filename="{name}">
|
||||
<lines>{lines}</lines>
|
||||
</class>
|
||||
</classes></package></packages></coverage>
|
||||
"""
|
||||
|
||||
|
||||
def _load() -> ModuleType:
|
||||
"""Import the script by path; it lives outside any importable package."""
|
||||
spec = importlib.util.spec_from_file_location("shell_coverage_report", _SCRIPT)
|
||||
assert spec is not None
|
||||
assert spec.loader is not None
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
@pytest.fixture(name="mod")
|
||||
def _mod() -> ModuleType:
|
||||
return _load()
|
||||
|
||||
|
||||
def _cov(tmp_path: Path, name: str, hits: dict[int, int]) -> Path:
|
||||
"""Write a minimal kcov cov.xml tree for *name*."""
|
||||
out = tmp_path / "cov" / "proc"
|
||||
out.mkdir(parents=True)
|
||||
lines = "".join(f'<line number="{n}" hits="{h}"/>' for n, h in sorted(hits.items()))
|
||||
(out / "cov.xml").write_text(_XML.format(name=name, lines=lines), encoding="utf-8")
|
||||
return tmp_path / "cov"
|
||||
|
||||
|
||||
def _trace(tmp_path: Path, source: Path, numbers: list[int]) -> Path:
|
||||
trace = tmp_path / "trace"
|
||||
trace.mkdir()
|
||||
(trace / "case.1").write_text(
|
||||
"".join(f"+PS4:{source}:{n} cmd\n" for n in numbers), encoding="utf-8"
|
||||
)
|
||||
return trace
|
||||
|
||||
|
||||
def test_kcov_hits_alone_are_used_without_a_trace(
|
||||
mod: ModuleType, tmp_path: Path
|
||||
) -> None:
|
||||
"""With no trace dir the result is kcov's own reading."""
|
||||
cov = _cov(tmp_path, "s.sh", {1: 1, 2: 0})
|
||||
covered, total, uncovered, off_set = mod._measure(cov, "s.sh", None)
|
||||
assert (covered, total, uncovered, off_set) == (1, 2, ["2"], 0)
|
||||
|
||||
|
||||
def test_trace_rescues_a_line_kcov_missed(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""Defect (b): a line kcov reports as 0 hits but that provably ran."""
|
||||
source = tmp_path / "s.sh"
|
||||
source.write_text("echo one\necho two\n", encoding="utf-8")
|
||||
cov = _cov(tmp_path, "s.sh", {1: 1, 2: 0})
|
||||
trace = _trace(tmp_path, source, [2])
|
||||
covered, total, uncovered, _ = mod._measure(cov, "s.sh", trace)
|
||||
assert (covered, total, uncovered) == (2, 2, [])
|
||||
|
||||
|
||||
def test_continuation_lines_leave_the_denominator(
|
||||
mod: ModuleType, tmp_path: Path
|
||||
) -> None:
|
||||
"""Defect (a): inner lines of a multi-line quoted argument are not statements."""
|
||||
source = tmp_path / "s.sh"
|
||||
# The real dwm_config.sh shape: the closing quote shares a line with the
|
||||
# rest of the command, so only the middle line is a continuation.
|
||||
source.write_text("perl -pe '\ns/a/b/;\n' f\necho after\n", encoding="utf-8")
|
||||
cov = _cov(tmp_path, "s.sh", {1: 1, 2: 0, 4: 1})
|
||||
# The trace must be non-empty: it is what tells the report where the
|
||||
# source lives, since kcov's XML carries only a basename.
|
||||
trace = _trace(tmp_path, source, [1, 4])
|
||||
covered, total, uncovered, _ = mod._measure(cov, "s.sh", trace)
|
||||
# Line 2 is inside the quote, so the denominator is 1 and 4 only.
|
||||
assert (covered, total, uncovered) == (2, 2, [])
|
||||
|
||||
|
||||
def test_traced_line_outside_the_line_set_is_counted_not_credited(
|
||||
mod: ModuleType, tmp_path: Path
|
||||
) -> None:
|
||||
"""A traced line kcov never listed cannot inflate coverage, but is reported."""
|
||||
source = tmp_path / "s.sh"
|
||||
source.write_text("echo one\necho two\necho three\n", encoding="utf-8")
|
||||
cov = _cov(tmp_path, "s.sh", {1: 1})
|
||||
trace = _trace(tmp_path, source, [1, 3])
|
||||
covered, total, _, off_set = mod._measure(cov, "s.sh", trace)
|
||||
assert (covered, total, off_set) == (1, 1, 1)
|
||||
|
||||
|
||||
def test_empty_kcov_output_reports_zero_total(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""No instrumented lines is a distinct, reportable state."""
|
||||
cov = _cov(tmp_path, "other.sh", {1: 1})
|
||||
assert mod._measure(cov, "s.sh", None) == (0, 0, [], 0)
|
||||
|
||||
|
||||
def test_line_without_a_number_is_skipped(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""A malformed <line> entry must not abort the report."""
|
||||
out = tmp_path / "cov" / "proc"
|
||||
out.mkdir(parents=True)
|
||||
(out / "cov.xml").write_text(
|
||||
_XML.format(name="s.sh", lines='<line hits="1"/><line number="4" hits="1"/>'),
|
||||
encoding="utf-8",
|
||||
)
|
||||
covered, total, _, _ = mod._measure(tmp_path / "cov", "s.sh", None)
|
||||
assert (covered, total) == (1, 1)
|
||||
|
||||
|
||||
def test_find_source_prefers_the_trace_path(mod: ModuleType, tmp_path: Path) -> None:
|
||||
"""kcov records only a basename, so the trace is the source of the path."""
|
||||
source = tmp_path / "s.sh"
|
||||
source.write_text("echo one\n", encoding="utf-8")
|
||||
assert mod._find_source("s.sh", _trace(tmp_path, source, [1])) == source
|
||||
|
||||
|
||||
def test_find_source_ignores_a_trace_path_that_is_gone(
|
||||
mod: ModuleType, tmp_path: Path
|
||||
) -> None:
|
||||
"""A traced path that no longer exists falls through to the repo search."""
|
||||
missing = tmp_path / "absent.sh"
|
||||
trace = _trace(tmp_path, missing, [1])
|
||||
# Nothing in the repo is called absent.sh, so the fallback finds nothing.
|
||||
assert mod._find_source("absent.sh", trace) is None
|
||||
|
||||
|
||||
def test_find_source_without_a_trace_searches_the_repo(mod: ModuleType) -> None:
|
||||
"""The fallback resolves a uniquely-named file in the repo."""
|
||||
found = mod._find_source("shell_coverage_jail.sh", None)
|
||||
assert found is not None
|
||||
assert found.name == "shell_coverage_jail.sh"
|
||||
|
||||
|
||||
def test_find_source_refuses_an_ambiguous_name(mod: ModuleType) -> None:
|
||||
"""Two files with one name yields no exclusion rather than a wrong guess."""
|
||||
assert mod._find_source("run_all.sh", None) is None
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# main(): the gate itself
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _run_main(
|
||||
mod: ModuleType,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
argv: list[str],
|
||||
) -> None:
|
||||
monkeypatch.setattr("sys.argv", ["shell_coverage_report.py", *argv])
|
||||
mod.main()
|
||||
|
||||
|
||||
def test_main_prints_the_summary(
|
||||
mod: ModuleType,
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
) -> None:
|
||||
"""A fully covered subject prints its percentage and passes the gate."""
|
||||
cov = _cov(tmp_path, "s.sh", {1: 1, 2: 1})
|
||||
_run_main(mod, monkeypatch, [str(cov), "s.sh", "100"])
|
||||
assert "s.sh: 2/2 lines = 100.00%" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_main_lists_uncovered_lines(
|
||||
mod: ModuleType,
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
) -> None:
|
||||
"""A gap is named, because the numbers alone do not help close it."""
|
||||
cov = _cov(tmp_path, "s.sh", {1: 1, 2: 0})
|
||||
with pytest.raises(SystemExit):
|
||||
_run_main(mod, monkeypatch, [str(cov), "s.sh", "100"])
|
||||
assert "uncovered: 2" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_main_notes_lines_outside_the_line_set(
|
||||
mod: ModuleType,
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
) -> None:
|
||||
"""A traced line kcov never listed is surfaced as a third-defect signal."""
|
||||
source = tmp_path / "s.sh"
|
||||
source.write_text("echo one\necho two\n", encoding="utf-8")
|
||||
cov = _cov(tmp_path, "s.sh", {1: 1})
|
||||
trace = _trace(tmp_path, source, [1, 2])
|
||||
_run_main(mod, monkeypatch, [str(cov), "s.sh", "100", str(trace)])
|
||||
assert "absent from kcov's line set" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_main_exits_when_below_the_minimum(
|
||||
mod: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Below the bar is a non-zero exit, which is what makes this a gate."""
|
||||
cov = _cov(tmp_path, "s.sh", {1: 1, 2: 0})
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
_run_main(mod, monkeypatch, [str(cov), "s.sh", "100"])
|
||||
assert "below the required" in str(excinfo.value)
|
||||
|
||||
|
||||
def test_main_exits_when_nothing_was_instrumented(
|
||||
mod: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Zero instrumented lines is an error, never a vacuous 100%."""
|
||||
cov = _cov(tmp_path, "other.sh", {1: 1})
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
_run_main(mod, monkeypatch, [str(cov), "s.sh", "100"])
|
||||
assert "instrumented no lines" in str(excinfo.value)
|
||||
Loading…
Reference in New Issue
Block a user