tools: (testcoverage.sh) refactor to enable multiprocess execution - #4656

Reviewed and tested locally (ran both the old and new script, sequential and --parallel, on this tree).

First: the speedup is real and reproduces here — 1m52s → 36s on a full run. The per-program cache-file design is the right way to parallelize this, and splitting the monolithic generate_report loop into analyze_tool() / get_all_test_scripts() is a nice readability win. The EXIT trap also fixes genuine temp-file leakage (the old script left test-coverage-raw-report-* and test-coverage-summary-report-* behind in $PWD on any early exit), and the double-percent bug is gone (old printed 54.55%%).

Three things should be fixed before this is merged, though.

1. The script now always exits 0 (major)

cleanup() ends with exit 0 and is newly registered on EXIT, so it clobbers every non-zero status. generate_report() also hardcodes return 0 (was return $error).

testcoverage.sh . -m losetup rfkill lsipc   old: exit 1   new: exit 0
testcoverage.sh . --bogus-option            old: exit 1   new: exit 0   <- even getopt errors

This contradicts the script's own header comment ("If an issue has been encountered with any tool's tests, ... the script will exit with a non-zero status code") and breaks any gating on make testcoverage.

Suggested fix: drop exit 0 from cleanup (or capture local rc=$? first and exit $rc), and restore error propagation from analyze_tool — in parallel mode collect it from wait -n's status instead of discarding it.

2. --parallel and sequential produce different numbers (major)

analyze_tool() declares only progname dir_name percentage frac notes as local. test_scripts, has_ts, has_ts_dir, prog_l_opts, ts_l_opts and summary_filepath remain global. Sequentially they leak from one program to the next; in parallel each forked job starts with them empty.

Minimal repro:

testcoverage.sh . -m cal cfdisk              ->  cfdisk  20.00% (1/5)
testcoverage.sh . -m --parallel cal cfdisk   ->  cfdisk   0.00% (0/0)

Over a full run that is Overall test coverage: 46.30% vs 46.15%, plus 7 rows with differing NOTES.

Parallel is the correct one. cfdisk has no test subdirectory, so test_scripts is never reassigned and it inherits cal's scripts — which means cfdisk gets credited with --color harvested from tests/ts/cal/color's UL_TESTCOVERAGE_MANUAL_VALIDATION line.

The leak itself is pre-existing (the old script also reports 20% for cfdisk), but this PR turns it into a silent mode-dependent discrepancy. Adding those names to the local declaration fixes both modes at once — this is what the # TODO: Uppercase global variables note is pointing at, and I think it belongs in this commit rather than a follow-up.

3. "Total share of tested programs" regressed (major)

count_untested_progs() treats any row whose NOTES matches missing test subdirectory as untested, even when the program has real measured coverage from cross-tests:

testcoverage.sh . umount mount   old: 100.00% (2/2)   new: 50.00% (1/2)

...while the very same report shows umount 35.29% (6/17).

Related: num_tested_progs is assigned from find_all_summary_files | wc -l and then never used — the next line recomputes the value a different way.

Smaller things

• count_untested_progs() greps TMP_COVERAGE_SUMMARY_REPORT_FILE, i.e. the column-rendered table, so it only works if print_report already ran. Grepping field 4 of the pipe-delimited TMP_COVERAGE_RAW_REPORT_FILE would be stable and would drop the implicit ordering dependency.
• nproc is a new unchecked dependency — the script validates mktemp but not this one. If it is missing, max_cpus is empty and while (($(jobs -rp | wc -l) >= max_cpus)) is a syntax error. max_cpus=$(nproc 2>/dev/null || echo 4), and ideally only evaluate it when OPT_PARALLEL is set.
• progress_status is printed in the parent before forking, so in parallel mode the counter races to 100% while jobs are still running.
• The --save-report filename has minute granularity, so two runs within the same minute silently overwrite each other. %S would fix it.
• get_cross_test_long_opts "$progname" | sort | uniq is redundant — the function already ends in sort | uniq. Note the old code sorted/uniq'd the combined ts_l_opts and the new code does not; harmless (it only bloats the alternation regex), but it looks unintended.
• Leftover # TODO: Uppercase global variables in a submitted commit.
• On the diff being ugly: it is mostly that the wholesale whitespace/continuation reformatting is mixed in with the functional refactor. Splitting it into a "reformat" commit plus a "refactor for parallel execution" commit would make this much easier to read — no need to apologise for it, just split it :)

Also FWIW, the ShellCheck bot's SC2046 complaint about cat $(find_all_summary_files) is indeed a false alarm — the filenames are <progname>.testcoverage.summary, so there is nothing to word-split. find ... -exec cat {} + would avoid needing the suppression comment, but it loses the sort, so the current form is fine.

Nothing missing on the build-system side: testcoverage is an autotools-only convenience target with no meson equivalent.

— assisted by Claude Code