mirror of
https://github.com/kuhyx/testsAndMisc.git
synced 2026-11-10 23:07:03 +01:00
Krzysztof kuhy Rudnicki
078d3463bd
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
174 lines
9.0 KiB
Markdown
174 lines
9.0 KiB
Markdown
# Shell split verification: how a green split is still broken
|
|
|
|
Split out of `docs/shell-split-recipes.md` when that file hit the same 250-line
|
|
cap it exists to serve. That file covers **how to make** a split; this one
|
|
covers **how to find out it is wrong**. Read both before the next
|
|
`linux_configuration` or `phone_focus_mode` split.
|
|
|
|
Every section below cost a failed run, a revert, or — once — a live system
|
|
mutation.
|
|
|
|
### A `--skip-install` flag is not evidence the script is safe to run
|
|
|
|
`shell_check.sh` gates only `install_if_missing`'s pacman branch on
|
|
`SKIP_INSTALL`. The AUR branch inside `install_linters` runs regardless, so a
|
|
backgrounded "baseline" run installed four packages before it was killed
|
|
mid-`makepkg`. Decision 6 was violated by a flag that looked like permission.
|
|
|
|
Before running any script to capture a baseline, grep the flag's variable and
|
|
confirm it gates **every** mutation path, not just the first one:
|
|
|
|
```bash
|
|
grep -n 'SKIP_INSTALL\|DRY_RUN' <script> # then read each hit
|
|
```
|
|
|
|
`--list-only` was the genuinely safe path here, because it returns before
|
|
`install_linters` is reached at all.
|
|
|
|
### Check whether the target is DEPLOYED as a single file — the trace cannot
|
|
|
|
Before designing seams, find out whether some other script copies the target
|
|
somewhere and runs it from there. An entry+`lib/` shape ships an entry whose
|
|
deployed `SCRIPT_DIR` has no `lib/`, so it dies on its first `source` under
|
|
`set -e` — while your trace stays green, because the trace runs the repo copy
|
|
where `lib/` exists.
|
|
|
|
```bash
|
|
grep -rn '<target-basename>' linux_configuration/ --include='*.sh' \
|
|
| grep -v '<the target itself>:'
|
|
```
|
|
|
|
Confirmed blocked on this, not theoretically: `install_leechblock.sh` (the
|
|
pacman installer `cp`s it to `/usr/local/share/digital_wellbeing/` and
|
|
`pacman_wrapper.sh:831` prefers that copy on **every pacman invocation**) and
|
|
`block_compulsive_opening.sh` (`install_all` copies itself to
|
|
`/usr/local/bin`). A target invoked as `$CONFIG_DIR/scripts/...` is fine.
|
|
|
|
### A stubbed run cannot reach a bug in a seam that passes state
|
|
|
|
`meta/scripts/verify_shell_split.sh` and the hand-rolled `declare -F` probe
|
|
both **source the libs and never call the functions**. That is enough to prove
|
|
every `source` line resolves, and nothing more. The `analyze_repo.sh` split
|
|
passed `bash -n`, `shellcheck`, a function-set diff, a line-set diff and the
|
|
stubbed probe — and still shipped a script that aborted immediately after
|
|
language detection. Two separate bugs, neither reachable without executing:
|
|
|
|
- **`set -e` and the function tail.** Eight `((LANG_FILES[x] > 0)) && HAS_Y=true`
|
|
lines were top-level statements, where a false `&&` merely leaves `$?`
|
|
non-zero. Wrapped into a function they become its _return value_, and the
|
|
last one (java, false for most repos) killed the script. Any time you wrap
|
|
former top-level code in a function, check what its **last statement** is;
|
|
append `|| true` if it is a bare conditional.
|
|
- **A self-referencing nameref.** `local -n LANG_CODE_FILES="$1"` pointing at a
|
|
global of the same name still moves data on bash 5.3, but warns
|
|
`circular name reference` on _every access_ — straight into the script's
|
|
output. Name the local something else (`_code_files`).
|
|
|
|
So: when a seam passes state, run the thing. Stub only the parts that mutate
|
|
the system, and diff the generated artifacts against a detached worktree at the
|
|
pre-split commit. That comparison is what proved the fix — same file set, every
|
|
text artifact byte-identical.
|
|
|
|
## The harness and the fixtures
|
|
|
|
Everything about _running_ the check — `trace_shell_split.sh`, the `--prefix`
|
|
mode for scripts whose job is placing files, the regression fixtures, the
|
|
entry-point and `shfmt` traps — moved to
|
|
**`docs/shell-split-harness.md`** when this file hit the same 250-line cap it
|
|
exists to serve. This file stays the _why_; that one is the _how_.
|
|
|
|
### Prove the move was verbatim: `meta/scripts/verify_shell_split.sh`
|
|
|
|
The Python splits in this repo are verified with a throwaway `ast` identity
|
|
check. There is no `ast` for bash, so the shell equivalent normalises through
|
|
`shfmt -mn` (minify: strips comments and indentation, keeps logic) and compares
|
|
a hash per top-level function:
|
|
|
|
```bash
|
|
bash meta/scripts/verify_shell_split.sh HEAD <old-path> <new-path>...
|
|
# IDENTICAL: all N top-level functions moved verbatim
|
|
```
|
|
|
|
It reports which function changed, so a diff points at the seam rather than the
|
|
whole file. Run it after every split, and again after `shfmt`/pre-commit
|
|
autofixes — reordering an import or reflowing a line is exactly what it is
|
|
designed to see through.
|
|
|
|
**Verify the harness before trusting it.** A normalizer that silently passes
|
|
everything is worse than none. The three checks it must survive:
|
|
|
|
| input | expected |
|
|
| ------------------------------------------------ | --------------------------------------- |
|
|
| unchanged copy of the file | `IDENTICAL` |
|
|
| comments/blank lines/indentation changed | `IDENTICAL` |
|
|
| one character of logic changed (`"$1"` → `"$2"`) | `DIFFERENCE FOUND`, naming the function |
|
|
|
|
The first attempt at that mutation test used `sed 's/exit 1/exit 2/'` on a file
|
|
containing no `exit 1`. It reported `IDENTICAL` — not because the harness was
|
|
broken, but because the mutation never applied. Confirm the mutation actually
|
|
changed the file (`diff` it) before concluding anything about the harness.
|
|
|
|
### Deployment triage of the over-cap scripts (2026-08-17)
|
|
|
|
Run before designing any seam, per the section above. `deploy.sh` copies a
|
|
**hardcoded per-file list** into `/data/local/tmp/focus_mode` and never deploys
|
|
`lib/`:
|
|
|
|
- **`phone_focus_mode/*` (8 over-cap scripts)** — deployed individually.
|
|
A new sibling must be added to `deploy.sh`'s copy list **in the same commit**,
|
|
or focus mode dies on the phone at its first `source`. These scripts source
|
|
only `config.sh`, which is on that list, so a sibling-file seam is workable —
|
|
an entry+`lib/` seam is not.
|
|
- **`install_leechblock.sh`, `block_compulsive_opening.sh`** — copied to
|
|
`/usr/local/…` and preferred there by `pacman_wrapper.sh:831` on every pacman
|
|
invocation. Teach the installer to deploy the directory first, in its own
|
|
commit. Highest blast radius; do these last.
|
|
- **`hosts/install.sh`, `setup_hosts_guard.sh`, `lib/monitor.sh`** — install
|
|
helper scripts to `/usr/local/sbin` or `/usr/local/bin`; check whether the
|
|
file being split is itself the deployed one.
|
|
- **The other 16** (`fresh-install/main.sh`, `nvidia_troubleshoot.sh`,
|
|
`setup_passwordless_system.sh`, `clean_audio.sh`, `enforce_vbox_hosts.sh`,
|
|
`migrate_hosts_guard_to_guard_lib.sh`, `libre_translate.sh`,
|
|
`diagnose_pacman_hook_stall.sh`, `install_plagiarism_tools.sh`,
|
|
`steam_compatibility.sh`, `deploy.sh`, `setup_night_lockdown.sh`,
|
|
`pacman_wrapper.sh`, `generate_study_materials.sh`,
|
|
`check_and_enable_services.sh`, `setup_midnight_shutdown.sh`) — no deployed
|
|
copy found, so an entry+`lib/` shape is safe.
|
|
|
|
### A lib must contain the readers of every global it assigns
|
|
|
|
`install_pacman_wrapper.sh:29` records the constraint: a **definitions-only**
|
|
lib "assigns without referencing, which is SC2034, and the repo forbids
|
|
suppressions." The rule that follows from it is not "assignments stay in the
|
|
entry script" — it is that a file must not assign a variable it never reads.
|
|
A lib holding both the writer and its readers is clean.
|
|
|
|
Getting this inverted is easy and costs several revert cycles.
|
|
`libre_translate.sh` was split with `parse_args` (the writer) left in the entry
|
|
and `health_check` / `sample_request` / `start_container_ephemeral` (the
|
|
readers) moved to the lib — the worst of both, and SC2034 fired on nine
|
|
globals. The fix is to colocate: move the writer **and** its consumers into the
|
|
same file.
|
|
|
|
The pre-commit hook is `shellcheck` with **no `-x`**, so each file is analysed
|
|
standalone and a `# shellcheck source=` directive will not make the checker
|
|
follow the link. Verify by running `shellcheck <lib>` on its own before
|
|
committing; the hook will not be any more forgiving.
|
|
|
|
This is the shell analogue of the `monkeypatch.setattr` target trap: the
|
|
question is always which file's namespace owns the name.
|
|
|
|
**`libre_translate.sh` (488) is parked.** What remains after its clean seams
|
|
are taken is `parse_args` (111 lines), `write_env_file`, `detect_container_user`
|
|
and `main`, which between them assign and read the same configuration globals.
|
|
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: see `docs/kcov-under-report.md`
|
|
|
|
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.
|