mirror of
https://github.com/kuhyx/testsAndMisc.git
synced 2026-11-10 19:47:12 +01:00
Prove kcov's under-report is TWO defects, one of them a numerator bug
Settles a question the campaign has worked around for three sessions:
rpi_nc_install.sh's 11.36% is an instrument failure, not a testing gap.
Measured against an independent instrument. Driving the lib under
PS4='+PS4:${BASH_SOURCE##*/}:${LINENO} ' with set -x traces 55 distinct lines
as executed, and 45 of them are lines kcov reports as UNCOVERED -- including
61, 62, 63, 66, 68, 102, 118 and 146. The lines run; kcov does not record
them.
That splits the problem in two, and they need different fixes:
(a) WRONG DENOMINATOR -- continuation lines of a multi-line quoted argument
counted as statements, which bash -x never calls executable at all.
Fixable in shell_coverage_report.py. Holds dwm_config.sh down.
(b) WRONG NUMERATOR -- ordinary statements executing unrecorded. Caps
rpi_nc_install.sh, and no test can move it.
The distinction is the point. A denominator filter is the obvious fix and I
nearly built one last session; it would have done nothing for (b).
Also confirmed no test can raise rpi_nc_install.sh: the file is one 227-line
function called twice, an `exit 88` sentinel proves the suite runs to
completion, and its assertions on save_config (line 219) and the password
message (line 227) PASS while kcov calls both lines uncovered.
kcov 43 is the current release, so upgrading is not an available fix.
The kcov material moved to docs/kcov-under-report.md (the 250-line cap), and
NEXT_SESSION_PROMPT.md now leads with the instrument work rather than the
next lib -- 201 in-scope libs, 98 passing, 103 exempt, and the instrument is
the bottleneck.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016sxxsDHWgdRhmdmzNMA6XM
This commit is contained in:
parent
bcb3e59fa7
commit
078d3463bd
@ -1,75 +1,84 @@
|
||||
# Next session: Phase 3 — keep clearing the shell-coverage allowlist
|
||||
# Next session: fix the coverage INSTRUMENT, then keep clearing the allowlist
|
||||
|
||||
> **Paste this whole file into a fresh Claude session opened in `~/testsAndMisc`.**
|
||||
> It is self-contained. Do not go looking for the previous session's context.
|
||||
|
||||
## Status in one table
|
||||
|
||||
| Thing | Number |
|
||||
| ------------------------------- | -------------------------------------- |
|
||||
| In-scope shell libs | **201** |
|
||||
| Passing the gate today | **98** |
|
||||
| Still allowlisted (exempt) | **103** |
|
||||
| Directories with a test harness | 7 |
|
||||
| Cleared this session | 2 (`unity_handler`, `transcribe_venv`) |
|
||||
|
||||
The gate itself is **built and ready to arm**. `is_covered()` reads `jail_args`
|
||||
and passes `--fail-on-case-error`. Only the allowlist stands between here and
|
||||
wiring it into pre-commit, pre-push and CI. CI is green.
|
||||
|
||||
## The scoping answers are SETTLED. Do not re-ask them.
|
||||
|
||||
| Question | Answer |
|
||||
| ------------------ | ---------------------------------------------------- |
|
||||
| **Scope** | Repo-wide gate. NOT a ratchet. |
|
||||
| **Bar** | **100%** line coverage. Shim every external. |
|
||||
| **Files** | **ALL** `.sh`, not just `lib/`. |
|
||||
| **Gates** | pre-commit **AND** pre-push **AND** CI. |
|
||||
| **Order** | Tests first; arm the gate only once 100% is reached. |
|
||||
| **Target picking** | Hardest first. |
|
||||
Repo-wide gate (not a ratchet) · **100%** line coverage · **ALL** `.sh` ·
|
||||
pre-commit **AND** pre-push **AND** CI · tests first, arm the gate last ·
|
||||
hardest first.
|
||||
|
||||
Because the gate arms last, it never freezes the repo. That ordering is what
|
||||
makes this safe.
|
||||
## START HERE — the instrument is the bottleneck, not the tests
|
||||
|
||||
## What is already true (verify, do not redo)
|
||||
**kcov has two separate defects. One is fixable today; the other caps libs at
|
||||
numbers no test can move.** Full detail and the seven DISPROVEN hypotheses are
|
||||
in `docs/kcov-under-report.md` — read it first and do not retry them.
|
||||
|
||||
- **(a) Wrong denominator.** Continuation lines inside a multi-line quoted
|
||||
argument (a `perl -0777 -pe '...'` block) are counted as statements. A
|
||||
`bash -x` tracer never reports them as executable at all. This is what holds
|
||||
`dwm_config.sh` down.
|
||||
- **(b) Wrong numerator.** Ordinary statements execute but are not recorded.
|
||||
**Proven against an independent instrument:** driving `rpi_nc_install.sh`
|
||||
under `PS4='+PS4:${BASH_SOURCE##*/}:${LINENO} '` with `set -x`, 55 lines
|
||||
trace as executed, and **45 of them are lines kcov calls uncovered** —
|
||||
including 61, 62, 63, 66, 68, 102, 118, 146. This is why that lib sits at
|
||||
11.36% while all 29 of its assertions pass.
|
||||
|
||||
### Step 1 — fix (a) in `meta/scripts/shell_coverage_report.py`
|
||||
|
||||
Exclude continuation lines of multi-line quoted arguments from the
|
||||
denominator. It already parses the kcov XML and has the source path from the
|
||||
`filename` attribute, so this is a local change.
|
||||
|
||||
### Step 2 — fix (b) by replacing kcov's numerator with a PS4 tracer
|
||||
|
||||
Inside the jail the trace must land on a path the caller controls: `/root` and
|
||||
`/var` are both bind-mounted to throwaway dirs, and `shell_coverage_jail.sh`
|
||||
sends every case's stdout to `/dev/null`. Bind a dedicated output dir, or
|
||||
teach the jail to copy the trace out before teardown.
|
||||
|
||||
### Acceptance test for ANY instrument change
|
||||
|
||||
```bash
|
||||
git log --oneline -5 # 0091608b, d277369a, 154b4166, b91be309 on main
|
||||
grep -vc '^#\|^$' meta/shell-coverage-allowlist.txt # 103 (was 105)
|
||||
# must still be 39/39 = 100.00%
|
||||
--measure dot_resolver_install.sh --min 100
|
||||
# must not regress below 98.63%
|
||||
--measure rpi_nc_ca.sh
|
||||
```
|
||||
|
||||
- **The allowlist went 105 -> 103.** `unity_handler.sh` and
|
||||
`transcribe_venv.sh` both measure **100%** and are OFF it, verified through
|
||||
`check_shell_coverage.sh` itself with the exemption deleted.
|
||||
- **`meta/scripts/shell_coverage_jail.sh`** measures a script's coverage while
|
||||
it runs **for real** in a user+mount namespace. New this session:
|
||||
`--fail-on-case-error` (default OFF). Without it a suite with a failing
|
||||
assertion exits 0.
|
||||
- **A suite declares its jail needs in a `jail_args` file** beside its
|
||||
`run_all.sh`. `ci_mirror.sh`, `shell-tests.yml` and `is_covered()` all read
|
||||
it. No marker = the suite runs bare.
|
||||
- **CI is green.** `shell-tests.yml` SKIPS jail_args-marked suites with a
|
||||
printed reason: kcov is not packaged for Ubuntu 24.04 and upstream ships no
|
||||
binary. The pre-push `ci-mirror` hook runs them for real instead.
|
||||
If either moves the wrong way, the change is wrong. Both are measured through
|
||||
`run_all.sh`, never a single test file.
|
||||
|
||||
### Measured so far (always through `run_all.sh` — see below)
|
||||
## If the instrument work stalls, there is always forward motion
|
||||
|
||||
| lib | coverage | state |
|
||||
| ------------------------- | ------------------- | ------------- |
|
||||
| `dot_resolver_install.sh` | **39/39 = 100.00%** | off allowlist |
|
||||
| `unity_handler.sh` | **32/32 = 100.00%** | off allowlist |
|
||||
| `transcribe_venv.sh` | **53/53 = 100.00%** | off allowlist |
|
||||
| `rpi_nc_ca.sh` | 72/73 = 98.63% | allowlisted |
|
||||
| `transcribe_pkgmgr.sh` | 49/50 = 98.00% | allowlisted |
|
||||
| `nc_php.sh` | 84/85 = 98.82% | allowlisted |
|
||||
| `clean_audio_filters.sh` | 83/93 = 89.25% | allowlisted |
|
||||
| `transcribe_deps.sh` | 48/52 = 92.31% | allowlisted |
|
||||
| `aw_autostart.sh` | 68/82 = 82.93% | allowlisted |
|
||||
| `dwm_config.sh` | 35/73 = 47.95% | allowlisted |
|
||||
| `rpi_nc_install.sh` | 10/88 = 11.36% | MIS-MEASURED |
|
||||
|
||||
Two directories now have harnesses:
|
||||
`single_use/features/lib/tests/` (jailed) and
|
||||
`single_use/misc/testsAndMisc-bash/lib/tests/` (no jail, 118 assertions).
|
||||
|
||||
## Start here
|
||||
|
||||
Highest-scored remaining, all in `features/lib/` where a harness exists:
|
||||
Two directories have libs and no harness at all:
|
||||
|
||||
```
|
||||
score lines file
|
||||
4x ~ linux_configuration/scripts/single_use/features/lib/*.sh (41 left)
|
||||
22 libs linux_configuration/scripts/single_use/fixes/lib/ (no harness yet)
|
||||
9 libs phone_focus_mode/lib/ (no harness yet)
|
||||
22 libs linux_configuration/scripts/single_use/fixes/lib/
|
||||
9 libs phone_focus_mode/lib/
|
||||
41 libs linux_configuration/scripts/single_use/features/lib/ (harness exists)
|
||||
```
|
||||
|
||||
### The invocation that works
|
||||
Building a harness for `fixes/lib` unblocks 22 libs at once, the way this
|
||||
session's `testsAndMisc-bash` harness unblocked 5.
|
||||
|
||||
## The invocation that works
|
||||
|
||||
```bash
|
||||
bash -c '
|
||||
@ -82,53 +91,57 @@ timeout 300s bash meta/scripts/shell_coverage_jail.sh \
|
||||
|
||||
**Always measure through `run_all.sh`, never a single test file.** That is the
|
||||
subject `is_covered()` builds, and the runner's number can be LOWER than the
|
||||
same suite's own — `rpi_nc_ca.sh` is 100% alone and 98.63% through the runner.
|
||||
Reporting the single-file figure is reporting a number the gate will not agree
|
||||
with.
|
||||
suite's own — `rpi_nc_ca.sh` is 100% alone and 98.63% through the runner.
|
||||
|
||||
## Traps that cost real time this session
|
||||
A suite declares its jail needs in a **`jail_args`** file beside its
|
||||
`run_all.sh`; `ci_mirror.sh`, `shell-tests.yml` and `is_covered()` all read it.
|
||||
No marker means the suite runs bare.
|
||||
|
||||
## Measured so far (all through `run_all.sh`)
|
||||
|
||||
| lib | coverage | state |
|
||||
| ------------------------- | ------------------- | ------------- |
|
||||
| `dot_resolver_install.sh` | **39/39 = 100.00%** | off allowlist |
|
||||
| `unity_handler.sh` | **32/32 = 100.00%** | off allowlist |
|
||||
| `transcribe_venv.sh` | **53/53 = 100.00%** | off allowlist |
|
||||
| `rpi_nc_ca.sh` | 72/73 = 98.63% | allowlisted |
|
||||
| `nc_php.sh` | 84/85 = 98.82% | allowlisted |
|
||||
| `transcribe_pkgmgr.sh` | 49/50 = 98.00% | allowlisted |
|
||||
| `transcribe_deps.sh` | 48/52 = 92.31% | allowlisted |
|
||||
| `clean_audio_filters.sh` | 83/93 = 89.25% | allowlisted |
|
||||
| `aw_autostart.sh` | 68/82 = 82.93% | allowlisted |
|
||||
| `dwm_config.sh` | 35/73 = 47.95% | defect (a) |
|
||||
| `rpi_nc_install.sh` | 10/88 = 11.36% | defect (b) |
|
||||
|
||||
The last two are instrument-limited, not test-limited. **Writing more tests
|
||||
for them will not move the number** — their assertions already pass and their
|
||||
writes provably land on disk.
|
||||
|
||||
## Traps that cost real time
|
||||
|
||||
- **`--fail-on-case-error` or you are flying blind.** The jail suppresses a
|
||||
suite's stdout entirely, so assertions can fail invisibly. Always pass it.
|
||||
suite's stdout, so assertions fail invisibly without it.
|
||||
- **To find WHERE a suite dies, use an `exit <n>` sentinel.** Temporarily end
|
||||
the suite with `exit 42` at a chosen point; the jail surfaces the code. This
|
||||
is the only way to tell a tracing failure from an aborted suite.
|
||||
the suite with `exit 42`; the jail surfaces the code. The only way to tell a
|
||||
tracing failure from an aborted suite.
|
||||
- **`rm -f "$TEST_TMPDIR/bin/foo"` does NOT hide foo.** bash caches executable
|
||||
locations, so `command -v foo` keeps succeeding. Both harnesses now ship
|
||||
`_t_unstub`, which also runs `hash -r`. Three assertions in this repo were
|
||||
asserting the opposite of their own names because of this.
|
||||
- **A prepended stub dir cannot hide a real binary either.** If the host truly
|
||||
has the tool (`pacman`, `aw-qt`, `/usr/lib/libsndfile.so`), REPLACE PATH or
|
||||
jail it. Otherwise the "not installed" case tests nothing.
|
||||
- **A stub must materialise what the real tool creates.** A record-only stub
|
||||
in a `[[ ! -s $file ]]` fallback chain silently exercises the NEXT branch.
|
||||
locations. Both harnesses ship `_t_unstub`, which also runs `hash -r`. Three
|
||||
assertions in this repo were asserting the opposite of their own names.
|
||||
- **A prepended stub dir cannot hide a real binary.** If the host genuinely has
|
||||
the tool (`pacman`, `aw-qt`, `/usr/lib/libsndfile.so`), REPLACE PATH or jail
|
||||
it, or the "not installed" case tests nothing.
|
||||
- **A stub must materialise what the real tool creates.** A record-only stub in
|
||||
a `[[ ! -s $file ]]` fallback chain silently exercises the NEXT branch.
|
||||
- **A top-level stub function SHADOWS the subject's own** for every later
|
||||
assertion. Put redefinitions in a subshell.
|
||||
- **`2>/dev/null` swallows a PS4 trace too** — it cost a bogus "0 lines traced"
|
||||
reading before the redirect was noticed.
|
||||
- **`shfmt` reformats after your own check passes** — re-check the 250-line cap
|
||||
after staging.
|
||||
- **Commits exceed a 2-minute foreground timeout.** Use `run_in_background`.
|
||||
- **Two-strike rule.** Two failed attempts at the same line -> stop, document,
|
||||
- **Two-strike rule.** Two failed attempts at the same thing -> stop, document,
|
||||
keep the working state.
|
||||
|
||||
## THE OPEN PROBLEM — read before trusting any number
|
||||
|
||||
**kcov silently under-reports, and it is contagious across processes in one
|
||||
jail.** `rpi_nc_ca.sh` measures 100% alone but 98.63% through `run_all.sh`;
|
||||
bisection shows `test_dwm_config.sh` running first is what costs it line 141.
|
||||
`dwm_config.sh` reports 47.95% while an `exit 43` sentinel proves four of its
|
||||
functions run to completion.
|
||||
|
||||
**One artifact is CONFIRMED:** continuation lines inside a multi-line quoted
|
||||
argument (a `perl -0777 -pe '...'` block) are counted as statements that never
|
||||
run — the wrong denominator. **Six other hypotheses are DISPROVEN** (heredocs,
|
||||
`$(...)`, stdin-reading stubs, `cd` relocation, heredoc-into-stubbed-external,
|
||||
and the exec-ing `sudo` shim / pipelines). Full details and the disproof list
|
||||
are in `docs/shell-split-verification.md`. **Do not re-run those.**
|
||||
|
||||
The failure is one-directional: it only ever under-reports, so it can keep a
|
||||
lib on the allowlist but can never promote an untested one. That is why the
|
||||
campaign can continue around it. **Never "fix" a number by weakening a test.**
|
||||
|
||||
## Rules that will bite you
|
||||
|
||||
- **No suppressions, ever.** Fix SC2016/SC2155 at the source.
|
||||
@ -138,24 +151,23 @@ campaign can continue around it. **Never "fix" a number by weakening a test.**
|
||||
`docs/superpowers/contracts/` once **>=4 code files** are staged.
|
||||
- **Put the MEASURED number in it**, never a rounded or hoped-for one.
|
||||
- **Stage narrowly.** A concurrent session had 8 unrelated files in the shared
|
||||
index this session; `git restore --staged` them, do not commit them.
|
||||
- **New test files need the exec bit** — and `git add --chmod=+x` only changes
|
||||
the INDEX. `chmod +x` the working tree too or the shebang hook fails.
|
||||
- **`CI_GREEN_SKIP=1`** is the documented escape hatch when THIS commit is the
|
||||
fix for a red baseline. It must also be exported for the pre-push mirror.
|
||||
index this session; `git restore --staged` them.
|
||||
- **New test files need the exec bit** — `git add --chmod=+x` only changes the
|
||||
INDEX; `chmod +x` the working tree too or the shebang hook fails.
|
||||
- **`CI_GREEN_SKIP=1`** is the documented escape hatch when THIS commit fixes a
|
||||
red baseline. Export it for the pre-push mirror too.
|
||||
- Work directly on `main`. `git stash` and branch creation are blocked.
|
||||
- **Another Claude session may be working in this repo simultaneously.**
|
||||
|
||||
## Not caused by this campaign
|
||||
|
||||
**`Python tests` CI has been failing since 2026-08-17** — `pip` hits
|
||||
`error: resolution-too-deep` installing `meta/requirements.txt`. It needs
|
||||
dependency pinning and is deliberately excluded from `check_ci_green.sh`. Do
|
||||
not fold it into Phase 3.
|
||||
`error: resolution-too-deep` installing `meta/requirements.txt`. Needs
|
||||
dependency pinning; deliberately excluded from `check_ci_green.sh`. Do not
|
||||
fold it into this work.
|
||||
|
||||
## When the allowlist reaches zero
|
||||
|
||||
Only then, wire `check_shell_coverage.sh` into **pre-commit, pre-push and CI**.
|
||||
`is_covered()` now reads `jail_args` and passes `--fail-on-case-error`, so the
|
||||
gate is ready; what remains is the allowlist. Note it runs a full jailed suite
|
||||
per lib, so an `--all` sweep is minutes and belongs in CI, not a commit hook.
|
||||
Wire `check_shell_coverage.sh` into pre-commit, pre-push and CI. The gate is
|
||||
already correct; it runs a full jailed suite per lib, so an `--all` sweep is
|
||||
minutes and belongs in CI, not a commit hook.
|
||||
|
||||
99
docs/kcov-under-report.md
Normal file
99
docs/kcov-under-report.md
Normal file
@ -0,0 +1,99 @@
|
||||
# kcov under-reports shell coverage: two distinct defects
|
||||
|
||||
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.
|
||||
|
||||
### kcov silently under-reports some subjects: never trust a percentage alone
|
||||
|
||||
`rpi_nc_install.sh` measures **10/88 = 11.36%** under
|
||||
`meta/scripts/shell_coverage_jail.sh`, reporting lines 13–60 as covered and
|
||||
everything from 61 on as not. The code past line 61 provably runs: the suite's
|
||||
29 assertions pass, and both `/root/.nextcloud_db_password` (line 61) and
|
||||
`/etc/apache2/sites-available/nextcloud.conf` (a heredoc ending at line 100)
|
||||
exist on disk after a run. The two sibling libs in the same directory measure
|
||||
sensibly, so this is not universal — and nobody has bounded which subjects it
|
||||
affects.
|
||||
|
||||
**Five hypotheses have each been tested against a minimal reproduction and
|
||||
DISPROVEN. Do not retry them:**
|
||||
|
||||
1. kcov traces correctly **past a heredoc**.
|
||||
2. kcov traces correctly **into a `$(...)` command substitution**.
|
||||
3. kcov traces correctly **past a heredoc-fed stub reading stdin via `$(cat)`**.
|
||||
4. kcov traces correctly **past a `cd` that relocates the process** (including
|
||||
`cd /tmp`, where the jail's own working dir lives) — lines after the `cd`
|
||||
are recorded.
|
||||
5. kcov traces correctly **past a heredoc piped into a stubbed external** —
|
||||
`mysql -u root <<EOF` with a `cat >/dev/null` stub, under strict mode.
|
||||
Lines after it are recorded.
|
||||
|
||||
**One CONFIRMED artifact (hypothesis 6), reproduced minimally:** kcov counts
|
||||
the _continuation lines of a multi-line quoted argument_ as instrumentable
|
||||
statements that never run. A `perl -0777 -i -pe '<newline>...s!a!b!;<newline>'`
|
||||
block reports its inner lines at zero hits while the line after it is covered.
|
||||
This accounts for `dwm_config.sh` lines 64-66 and 98-101, and for any
|
||||
`done < <(...)` process substitution. These lines are **not statements**, so
|
||||
the denominator is wrong, not the numerator.
|
||||
|
||||
That artifact does NOT explain the second pattern: ordinary statements inside
|
||||
functions that provably execute. In `dwm_config.sh`, `build_install`,
|
||||
`build_pointer_confine`, `write_session_files` and `verify` all run -- proved
|
||||
with an `exit 43` sentinel placed after their assertions, which fired -- yet
|
||||
their bodies report zero hits. `rpi_nc_install.sh` behaves the same way. A
|
||||
sixth hypothesis (that the jail's `exec`-ing `sudo` shim or a pipeline loses
|
||||
the trace) was tested minimally and **DISPROVEN**: both trace correctly.
|
||||
|
||||
**The second pattern is a NUMERATOR bug, and an independent instrument proves
|
||||
it.** Driving `rpi_nc_install.sh` directly under `PS4='+PS4:${BASH_SOURCE##*/}:${LINENO} '`
|
||||
with `set -x`, 55 distinct lines of the lib trace as executed -- including
|
||||
61, 62, 63, 66, 68, 102, 118 and 146, every one of which kcov reports as
|
||||
uncovered. 45 of kcov's "uncovered" lines were seen executing this way. The
|
||||
lines run; kcov does not record them.
|
||||
|
||||
So there are two separate defects, and they need different fixes:
|
||||
|
||||
- **(a) wrong denominator** -- continuation lines inside a multi-line quoted
|
||||
argument are counted as statements. A `bash -x` tracer never reports them as
|
||||
executable at all. Fixable in `meta/scripts/shell_coverage_report.py` by
|
||||
excluding them; affects `dwm_config.sh`'s perl blocks.
|
||||
- **(b) wrong numerator** -- ordinary statements execute but are not recorded.
|
||||
This is what caps `rpi_nc_install.sh` at 11.36% and no test can move it.
|
||||
Fixing it means replacing or supplementing kcov's numerator with a PS4 /
|
||||
`DEBUG`-trap tracer.
|
||||
|
||||
A PS4 tracer inside the jail needs its output on a path the caller controls:
|
||||
`/root` and `/var` are both bind-mounted to throwaway dirs, and the jail sends
|
||||
each case's stdout to `/dev/null`.
|
||||
|
||||
The practical rule while both stand:
|
||||
|
||||
> **A suspicious percentage means re-measure, not re-write the test.** If a
|
||||
> suite's assertions pass while its number looks absurd, suspect the
|
||||
> instrument before suspecting the tests. **Never "fix" a number by weakening
|
||||
> an assertion.**
|
||||
|
||||
Note the failure is _silent and one-directional_: it under-reports, so it can
|
||||
only ever keep a lib on the allowlist that deserves to come off. It cannot
|
||||
promote an untested lib. That is why the campaign can continue around it.
|
||||
|
||||
**The under-report is contagious across processes in one jail.** Measured on
|
||||
`rpi_nc_ca.sh`: run alone its suite reports **73/73 = 100.00%**, reproduced
|
||||
twice. Run through `run_all.sh` it reports **72/73 = 98.63%**, also twice --
|
||||
and bisecting the five sibling suites shows a single culprit, `test_dwm_config.sh`.
|
||||
Run `dwm_config` first and `rpi_nc_ca` loses line 141 (`cat <<'EOF'`, an
|
||||
ordinary statement); run any other sibling first and it keeps it. The CA
|
||||
suite's assertions all pass either way, confirmed with an exit sentinel, so
|
||||
nothing is actually untested.
|
||||
|
||||
This matters for the gate: `is_covered()` measures through `run_all.sh`, so
|
||||
the _runner's_ number is the one that counts, and it can be lower than the
|
||||
same suite's own number for reasons that have nothing to do with the tests.
|
||||
Never report the single-file figure as the lib's coverage.
|
||||
|
||||
**Use an `exit <n>` sentinel to tell the two apart.** The jail discards a
|
||||
case's stdout, so a suite's own report is invisible; but a non-zero exit is
|
||||
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.
|
||||
@ -165,74 +165,9 @@ Getting the entry under the cap needs `parse_args` restructured rather than
|
||||
relocated — a refactor, not a verbatim move, so it is out of scope for this
|
||||
method. Attempt it with a test harness, or accept an entry that stays over.
|
||||
|
||||
### kcov silently under-reports some subjects: never trust a percentage alone
|
||||
### kcov silently under-reports: see `docs/kcov-under-report.md`
|
||||
|
||||
`rpi_nc_install.sh` measures **10/88 = 11.36%** under
|
||||
`meta/scripts/shell_coverage_jail.sh`, reporting lines 13–60 as covered and
|
||||
everything from 61 on as not. The code past line 61 provably runs: the suite's
|
||||
29 assertions pass, and both `/root/.nextcloud_db_password` (line 61) and
|
||||
`/etc/apache2/sites-available/nextcloud.conf` (a heredoc ending at line 100)
|
||||
exist on disk after a run. The two sibling libs in the same directory measure
|
||||
sensibly, so this is not universal — and nobody has bounded which subjects it
|
||||
affects.
|
||||
|
||||
**Five hypotheses have each been tested against a minimal reproduction and
|
||||
DISPROVEN. Do not retry them:**
|
||||
|
||||
1. kcov traces correctly **past a heredoc**.
|
||||
2. kcov traces correctly **into a `$(...)` command substitution**.
|
||||
3. kcov traces correctly **past a heredoc-fed stub reading stdin via `$(cat)`**.
|
||||
4. kcov traces correctly **past a `cd` that relocates the process** (including
|
||||
`cd /tmp`, where the jail's own working dir lives) — lines after the `cd`
|
||||
are recorded.
|
||||
5. kcov traces correctly **past a heredoc piped into a stubbed external** —
|
||||
`mysql -u root <<EOF` with a `cat >/dev/null` stub, under strict mode.
|
||||
Lines after it are recorded.
|
||||
|
||||
**One CONFIRMED artifact (hypothesis 6), reproduced minimally:** kcov counts
|
||||
the _continuation lines of a multi-line quoted argument_ as instrumentable
|
||||
statements that never run. A `perl -0777 -i -pe '<newline>...s!a!b!;<newline>'`
|
||||
block reports its inner lines at zero hits while the line after it is covered.
|
||||
This accounts for `dwm_config.sh` lines 64-66 and 98-101, and for any
|
||||
`done < <(...)` process substitution. These lines are **not statements**, so
|
||||
the denominator is wrong, not the numerator.
|
||||
|
||||
That artifact does NOT explain the second pattern: ordinary statements inside
|
||||
functions that provably execute. In `dwm_config.sh`, `build_install`,
|
||||
`build_pointer_confine`, `write_session_files` and `verify` all run -- proved
|
||||
with an `exit 43` sentinel placed after their assertions, which fired -- yet
|
||||
their bodies report zero hits. `rpi_nc_install.sh` behaves the same way. A
|
||||
sixth hypothesis (that the jail's `exec`-ing `sudo` shim or a pipeline loses
|
||||
the trace) was tested minimally and **DISPROVEN**: both trace correctly.
|
||||
|
||||
The cause of that second pattern remains unknown. The practical rule:
|
||||
|
||||
> **A suspicious percentage means re-measure, not re-write the test.** If a
|
||||
> suite's assertions pass while its number looks absurd, suspect the
|
||||
> instrument before suspecting the tests. **Never "fix" a number by weakening
|
||||
> an assertion.**
|
||||
|
||||
Note the failure is _silent and one-directional_: it under-reports, so it can
|
||||
only ever keep a lib on the allowlist that deserves to come off. It cannot
|
||||
promote an untested lib. That is why the campaign can continue around it.
|
||||
|
||||
**The under-report is contagious across processes in one jail.** Measured on
|
||||
`rpi_nc_ca.sh`: run alone its suite reports **73/73 = 100.00%**, reproduced
|
||||
twice. Run through `run_all.sh` it reports **72/73 = 98.63%**, also twice --
|
||||
and bisecting the five sibling suites shows a single culprit, `test_dwm_config.sh`.
|
||||
Run `dwm_config` first and `rpi_nc_ca` loses line 141 (`cat <<'EOF'`, an
|
||||
ordinary statement); run any other sibling first and it keeps it. The CA
|
||||
suite's assertions all pass either way, confirmed with an exit sentinel, so
|
||||
nothing is actually untested.
|
||||
|
||||
This matters for the gate: `is_covered()` measures through `run_all.sh`, so
|
||||
the _runner's_ number is the one that counts, and it can be lower than the
|
||||
same suite's own number for reasons that have nothing to do with the tests.
|
||||
Never report the single-file figure as the lib's coverage.
|
||||
|
||||
**Use an `exit <n>` sentinel to tell the two apart.** The jail discards a
|
||||
case's stdout, so a suite's own report is invisible; but a non-zero exit is
|
||||
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.
|
||||
Split out on 2026-08-22 when this file hit the 250-line cap. **Read it before
|
||||
trusting any coverage percentage**: two distinct kcov defects are documented
|
||||
there, one confirmed against an independent instrument, along with seven
|
||||
hypotheses that are already DISPROVEN and must not be retried.
|
||||
|
||||
@ -0,0 +1,50 @@
|
||||
{
|
||||
"intent": "Answer the question the campaign has been working around for three sessions: is rpi_nc_install.sh's 11.36% a testing gap or an instrument failure? Proven to be the latter, against an independent instrument, and the kcov problem is now split into two defects with different fixes.",
|
||||
"scope": [
|
||||
"docs/kcov-under-report.md: new, split out of shell-split-verification.md for the 250-line cap",
|
||||
"docs/shell-split-verification.md: the kcov section replaced by a pointer",
|
||||
"NEXT_SESSION_PROMPT.md: rewritten to lead with the instrument work",
|
||||
"Non-goal: fixing either defect -- this commit establishes WHICH defect is which, so the next session does not fix the wrong one"
|
||||
],
|
||||
"changes": [
|
||||
"MEASURED, not hypothesised: driving rpi_nc_install.sh directly under PS4='+PS4:${BASH_SOURCE##*/}:${LINENO} ' with set -x traces 55 distinct lines as executed, and 45 of them are lines kcov reports as uncovered -- including 61, 62, 63, 66, 68, 102, 118 and 146. The lines run; kcov does not record them.",
|
||||
"This splits the problem in two. (a) WRONG DENOMINATOR: continuation lines of a multi-line quoted argument counted as statements, which a bash -x tracer never calls executable -- fixable in shell_coverage_report.py, and it is what holds dwm_config.sh down. (b) WRONG NUMERATOR: ordinary statements executing unrecorded, which is what caps rpi_nc_install.sh and which no test can move.",
|
||||
"The distinction matters because a denominator filter -- the obvious fix, and the one I nearly built last session -- does nothing for (b). Knowing which defect a lib suffers from tells you whether writing tests can help it.",
|
||||
"Confirmed no test can raise rpi_nc_install.sh: the file is ONE 227-line function, called twice, and an `exit 88` sentinel proves the suite runs to completion with all 29 assertions passing. Its assertions on save_config (line 219) and the password message (line 227) PASS while kcov calls both lines uncovered."
|
||||
],
|
||||
"verification": [
|
||||
{
|
||||
"command": "minimal repro: perl -0777 block under kcov, then the same file under PS4='+LINE:${LINENO} ' bash -x",
|
||||
"result": "pass",
|
||||
"evidence": "kcov reports lines 6,7,8 as instrumentable with hits=0; the PS4 tracer never emits them at all. Establishes defect (a) as a denominator error rather than a missed execution."
|
||||
},
|
||||
{
|
||||
"command": "drive rpi_nc_install.sh's phase_install_nextcloud under PS4 with every external stubbed, outside the jail",
|
||||
"result": "pass",
|
||||
"evidence": "55 distinct lines traced, max line 166. Spot-checked against kcov's uncovered list: 61, 62, 63, 66, 68, 102, 118, 146 all SEEN EXECUTING. 45 of kcov's uncovered lines appear in the PS4 set. Establishes defect (b)."
|
||||
},
|
||||
{
|
||||
"command": "append `exit 88` before _t_report in test_rpi_nc_install.sh, run under --fail-on-case-error, restore",
|
||||
"result": "pass",
|
||||
"evidence": "'case exited 88' fired: the suite reaches its end with all 29 assertions passing. Combined with the PS4 trace, this rules out 'the tests do not exercise the code' as an explanation."
|
||||
},
|
||||
{
|
||||
"command": "kcov --version; pacman -Ss kcov",
|
||||
"result": "pass",
|
||||
"evidence": "kcov 43, which is the current release in extra/. Upgrading is not an available fix."
|
||||
},
|
||||
{
|
||||
"command": "bash meta/scripts/check_file_length.sh --all after the doc split",
|
||||
"result": "pass",
|
||||
"evidence": "all files within 250 lines: shell-split-verification.md 173, kcov-under-report.md 99, NEXT_SESSION_PROMPT.md 173."
|
||||
}
|
||||
],
|
||||
"risks": [
|
||||
"The PS4 measurement ran OUTSIDE the jail with a simplified fixture, so it reached line 166 and returned 1 rather than running the full 227 lines. That is sufficient for the discriminating question (do lines kcov calls uncovered execute at all?) but it is not a full-coverage figure and must not be quoted as one.",
|
||||
"Replacing kcov's numerator with a PS4 tracer is a bigger change than a denominator filter and could introduce its own inaccuracies. The acceptance test is recorded in the handoff: dot_resolver_install.sh must stay at 39/39 and rpi_nc_ca.sh must not fall below 98.63%."
|
||||
],
|
||||
"rollback": [
|
||||
"git revert the commit; the kcov material returns to shell-split-verification.md and the handoff loses the (a)/(b) split.",
|
||||
"After rollback, confirm shell-split-verification.md is back over 250 lines -- that cap violation is why the split happened."
|
||||
]
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user