From 6ee9502a83bdc39a667699e3df91219fd55cda8d Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Fri, 11 Sep 2026 20:59:53 +0000 Subject: [PATCH 1/2] test: compute the per-file coverage table from the tracefile (#974) The nightly's 'least covered first' table printed columnar_parquet_codec.c | 2.0% 100|3200% 2| - 0 for a file whose records say LF:100 LH:100 FNF:2 FNH:2 BRF:64 BRH:47. A file at 100% presented as 2.0% and ranked the worst in the tree, a function rate above 100% on its face, and an empty branch column where the tracefile carries 13,900 branches. Four of the files it named as least covered were between 94% and 100%. It came from lcov --list. The same step's lcov --summary, four lines earlier on the same tracefile, was correct -- and so is lcov --list here on the run's own uploaded tracefile. The builds differ only in distro patch level (the runner installs 2.0-4ubuntu2; this was checked on 2.0-1) and which patch does it has not been bisected. So the table is computed. A rate is a division of two integers the tracefile states outright, and LF: cannot be got wrong by a patch to --list. Cross-checked against lcov --list on all 39 files in that tracefile and independently against the raw counters: no disagreement in either. lcov --summary keeps its job. The format carries hit/found rather than only the total, because '93.7% 22765' cannot be checked by a reader and '93.7% 21320/22765' can. An absent counter prints '-', not 0.0%: a header with no branches at 0.0% sorts to the top and reads as the least covered file in the tree. TWELVE ARMS DRIVE THE GENERATOR over a synthetic tracefile rather than grepping it, because a static check that the runner calls the right script cannot tell whether the script is correct, and 'the table is wrong' was the defect. Four of those arms failed when first written, and each was the arm's fault: - two matched MY OWN COMMENTS in the runner explaining the replaced call -- a guard defeated by the sentence documenting it, and the third instance today of a comment carrying a token tripping a grep that wanted the call. They count over code with comments stripped now, under a premise that stripping comments did not strip the code. - one asserted a cell by field number; absent cells collapse to one token each, so the dashes are $4 and $5, not $6. The property is asserted first and the position second. - one matched a substring: 0\.0% is inside 50.0%, which is the line rate of the fixture that has no branches. Anchored, with a control over a genuinely zero-covered file proving the anchor did not defeat the assertion. harness_selftest 815 checks 0 FAIL, docs_style 9/9, shellcheck clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 45 +++++++ test/pgc_coverage_table.py | 116 ++++++++++++++++++ test/run_coverage.sh | 12 +- .../250-the-coverage-runner-must-refuse.sh | 89 ++++++++++++++ 4 files changed, 258 insertions(+), 4 deletions(-) create mode 100755 test/pgc_coverage_table.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 07b1d775..9adad267 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1619,6 +1619,51 @@ true until the next version shipped. removed from the reader's `PATH` to prove the skip branch can be reached, since an `else` that cannot run proves nothing either. +- The per-file coverage table is computed from the tracefile, so it no longer ranks + the best-covered files as the worst (#974). + + The nightly's "least covered first" table printed + + columnar_parquet_codec.c | 2.0% 100|3200% 2| - 0 + + for a file whose records say `LF:100 LH:100 FNF:2 FNH:2 BRF:64 BRH:47`. A file at + 100% presented as 2.0% and sorted to the top of a list headed least covered, with a + function rate above 100% on its face and an empty branch column where the tracefile + carries 13,900 branches. Four of the files it named as least covered were between + 94% and 100%. A coverage table is read to decide where to spend effort, and this one + inverted the ranking. + + The table came from `lcov --list`. The run's own `lcov --summary`, four lines + earlier and on the same tracefile, was correct, and so was `lcov --list` run here on + the run's own uploaded tracefile. The two builds differ only in distro patch level + -- the runner installs `2.0-4ubuntu2`, this was checked on `2.0-1` -- and which + patch does it has not been bisected. + + So the table is computed instead. A rate is a division of two integers the tracefile + states outright, and `LF:` cannot be got wrong by a patch to `--list`. Cross-checked + against `lcov --list` on all 39 files in that tracefile and independently against the + raw counters: no disagreement in either comparison. `lcov --summary` keeps its job. + + The format now carries hit/found rather than only the total, because `93.7% 22765` + cannot be checked by a reader and `93.7% 21320/22765` can -- and a reader who can + check the number is the only one who will notice when it is wrong again. + + An absent counter prints `-`, not `0.0%`. A header with no branches at 0.0% would + sort to the top and read as the least covered file in the tree, which is how a table + misleads while every individual number in it is defensible. + + Twelve arms in `250-the-coverage-runner-must-refuse` drive the generator over a + synthetic tracefile rather than grepping it, because a static check that the runner + calls the right script cannot tell whether the script is correct, and "the table is + wrong" was the defect. Three of them failed when first written: two matched the + comments in the runner that explain the replaced call, and the third asserted a cell + by the wrong field number. They are counted over code with comments stripped now, + under a premise that stripping comments did not strip the code. + + A fourth failed on a substring: `0\.0%` matches inside `50.0%`, which is the line + rate of the fixture that has no branches. It is anchored, and a control with a + genuinely zero-covered file proves the anchor did not defeat the assertion. + ## [1.0-alpha3] - 2026-09-02 ### Added diff --git a/test/pgc_coverage_table.py b/test/pgc_coverage_table.py new file mode 100755 index 00000000..85792d45 --- /dev/null +++ b/test/pgc_coverage_table.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +"""Print an lcov tracefile's per-file coverage, least covered first. + +WHY NOT `lcov --list`. Because it gave wrong answers on the nightly runner and +right ones here, from the same tracefile, on two builds of the same lcov 2.0 -- +`2.0-4ubuntu2` against `2.0-1`. The nightly's table printed +`columnar_parquet_codec.c | 2.0% 100|3200% 2| - 0` for a file whose records say +`LF:100 LH:100 FNF:2 FNH:2 BRF:64 BRH:47`: a file at 100% presented as 2.0% and +sorted to the top of a list headed "least covered first". Four of the files it +named as least covered were between 94% and 100%. + +A rate is a division of two integers the tracefile states outright, so computing it +here removes the one moving part -- whichever distro patch changes `--list` cannot +change `LF:`. `lcov --summary` stays where it is: it was correct on both builds. + +THE FORMAT SHOWS hit/total, not just the total. `93.7% 22765` cannot be checked by +a reader; `93.7% 21320/22765` can, and a reader who can check the number is the +only one who will notice when it is wrong again. + +An absent counter prints `-`, which is what a file with no branches has: not 0.0%, +which would sort it to the top as if it were the worst covered thing in the tree. +""" + +import argparse +import pathlib +import sys + +# The six counters this reads, and nothing else. `DA`/`BRDA`/`FN`/`FNDA` are the +# per-line detail the totals are computed from; re-deriving the totals from them +# would be a second implementation that can disagree with the first. +_PAIRS = (("lines", "LF", "LH"), ("functions", "FNF", "FNH"), ("branches", "BRF", "BRH")) + + +def read_tracefile(path): + """-> [{name, lines:(hit,found), functions:(...), branches:(...)}], in file order.""" + records, cur = [], None + for raw in pathlib.Path(path).read_text(errors="replace").splitlines(): + if raw.startswith("SF:"): + cur = {"name": raw[3:], "counters": {}} + elif cur is None: + continue + elif raw == "end_of_record": + records.append(cur) + cur = None + else: + key, _, value = raw.partition(":") + if key in ("LF", "LH", "FNF", "FNH", "BRF", "BRH"): + try: + cur["counters"][key] = int(value) + except ValueError: + pass # a malformed counter is absent, not zero + return records + + +def rate(hit, found): + """A rate, or None when there is nothing to rate. + + None is NOT zero. A header with no branches would sort first under 0.0% and be + read as the least covered file in the tree, which is how a table misleads while + every individual number in it is defensible. + """ + if not found: + return None + return 100.0 * hit / found + + +def rows(records): + out = [] + for rec in records: + c = rec["counters"] + row = {"name": rec["name"].rsplit("/", 1)[-1]} + for label, found_key, hit_key in _PAIRS: + found, hit = c.get(found_key), c.get(hit_key) + if found is None or hit is None: + row[label] = (None, None, None) + else: + row[label] = (rate(hit, found), hit, found) + out.append(row) + return out + + +def cell(value): + pct, hit, found = value + if pct is None: + return f"{'-':>7} {'':>13}" + return f"{pct:6.1f}% {f'{hit}/{found}':>13}" + + +def main(argv=None): + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("tracefile") + ap.add_argument("--limit", type=int, default=20, + help="rows to print (0 for all); the count printed names what was dropped") + args = ap.parse_args(argv) + + table = rows(read_tracefile(args.tracefile)) + if not table: + print(" no SF records in the tracefile, so there is nothing to rank") + return 1 + + # Sort by line rate. A file with no line counter at all has no place in a + # ranking by line rate, so it goes last rather than first. + table.sort(key=lambda r: (r["lines"][0] is None, r["lines"][0] if r["lines"][0] is not None else 0)) + + shown = table if args.limit <= 0 else table[:args.limit] + print(f" {'file':<34}{'lines':<22}{'functions':<22}{'branches'}") + for r in shown: + print(f" {r['name']:<34}{cell(r['lines'])} {cell(r['functions'])} {cell(r['branches'])}") + if len(shown) < len(table): + print(f" ({len(table) - len(shown)} further file(s) not shown, all covered at least as" + f" well as the last row)") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/test/run_coverage.sh b/test/run_coverage.sh index 65e621f3..2615579b 100755 --- a/test/run_coverage.sh +++ b/test/run_coverage.sh @@ -286,11 +286,15 @@ echo lcov --summary "$OUT/coverage.info" --rc branch_coverage=1 \ --ignore-errors inconsistent 2>&1 | grep -vE "^(Reading|lcov: (WARNING|Note))" | sed 's/^/ /' echo +# COMPUTED FROM THE TRACEFILE, not parsed out of `lcov --list` (#974). That +# command printed a 100%-covered file as 2.0% and ranked it least covered on the +# nightly runner, while giving correct rows for the same tracefile on another build +# of the same lcov 2.0. The counters it would have to get wrong -- `LF:`, `LH:` -- +# are stated outright in the file, so the division happens here. +# +# `lcov --summary` above is untouched: it was right on both builds. echo "-- per file, least covered first" -lcov --list "$OUT/coverage.info" --rc branch_coverage=1 \ - --ignore-errors inconsistent 2>/dev/null \ - | awk '/\|/ && !/Total:/ && !/^Filename/ {print}' \ - | sort -t'|' -k2 -n | head -20 | sed 's/^/ /' +python3 "$SRCDIR/test/pgc_coverage_table.py" "$OUT/coverage.info" --limit 20 echo echo "report: $OUT/html/index.html" diff --git a/test/selftest/250-the-coverage-runner-must-refuse.sh b/test/selftest/250-the-coverage-runner-must-refuse.sh index 67c76116..2a0b3215 100644 --- a/test/selftest/250-the-coverage-runner-must-refuse.sh +++ b/test/selftest/250-the-coverage-runner-must-refuse.sh @@ -143,3 +143,92 @@ check "premise: the containment and the copy were both located" \ check "the copy-back refuses a destination outside the tree (#740)" \ "$([ -n "$_cov_contain" ] && [ -n "$_cov_cp" ] && [ "$_cov_contain" -lt "$_cov_cp" ] && echo yes || echo no)" "yes" + +# ---- the per-file table must be computed, not parsed out of lcov --list ------ +# +# #974. The nightly's "least covered first" table printed +# `columnar_parquet_codec.c | 2.0% 100|3200% 2| - 0` for a file whose records say +# `LF:100 LH:100 FNF:2 FNH:2 BRF:64 BRH:47`. A file at 100% presented as 2.0% and +# ranked the worst in the tree, four of the named files between 94% and 100%, and +# function rates above 100% on their face. The same command on the same tracefile +# gave correct rows on lcov 2.0-1 and wrong ones on the runner's 2.0-4ubuntu2, so +# the table moved with a distro patch rather than with the data. +# +# A rate is a division of two integers the tracefile states outright, so the fix is +# to do the division. `lcov --summary` keeps its job: it was right on both builds. +# +# THE ARMS DRIVE THE GENERATOR over a synthetic tracefile rather than grepping it. +# A static check that the runner calls the right script cannot tell whether the +# script is correct, and "the table is wrong" was the defect. +_covtab="$TESTDIR/pgc_coverage_table.py" +check "premise: the table generator is present and parses" \ + "$([ -f "$_covtab" ] && python3 -c "import ast,sys;ast.parse(open(sys.argv[1]).read())" "$_covtab" 2>/dev/null && echo yes || echo no)" \ + "yes" + +# COUNTED OVER CODE, NOT OVER TEXT. The first version of these three greps counted +# comment lines, and two of them failed on MY OWN comments in the runner explaining +# why the call was replaced -- a guard defeated by the sentence documenting it. That +# is the third time today a comment carrying a token tripped a grep that wanted the +# call: a `pgc_summary` mention in #969 and a `shellcheck` mention in #972 were the +# others. Strip comments first, so a future explanation cannot redden the arm. +_cov_code() { # _cov_code PATTERN -> count of matching NON-comment lines + grep -vE '^[[:space:]]*#' "$_cov" | grep -cE "$1" +} +check "the coverage runner computes the table instead of parsing lcov --list (#974)" \ + "$(_cov_code 'pgc_coverage_table\.py')" "1" +check "and no lcov --list call survives in the runner (#974)" \ + "$(_cov_code 'lcov --list')" "0" +check "while the lcov --summary call stays, being correct on both builds (#974)" \ + "$(_cov_code 'lcov --summary')" "1" +# The premise for all three: stripping comments must not strip the code. Without it +# a grep that matches nothing reports the same 0 as a grep over an emptied file. +check "premise: stripping comments leaves the runner's code behind" \ + "$([ "$(grep -vE '^[[:space:]]*#' "$_cov" | grep -cE 'genhtml|lcov')" -ge 2 ] && echo yes || echo no)" \ + "yes" + +# Counters chosen so every cell is distinguishable from every failure mode: +# worst.c partially covered, must sort FIRST +# perfect.c 100% lines -- the shape the nightly printed as 2.0% +# nobranch.h no branch counter at all -- must print `-`, never 0.0%, or it +# sorts to the top and reads as the least covered file in the tree +_covtf="$(mktemp "${TMPDIR:-/tmp}/pgc-covtab.XXXXXX")" +{ + printf 'TN:\nSF:/x/src/worst.c\nFNF:4\nFNH:1\nBRF:10\nBRH:2\nLF:100\nLH:25\nend_of_record\n' + printf 'TN:\nSF:/x/src/perfect.c\nFNF:2\nFNH:2\nBRF:64\nBRH:47\nLF:100\nLH:100\nend_of_record\n' + printf 'TN:\nSF:/x/src/nobranch.h\nFNF:0\nFNH:0\nLF:8\nLH:4\nend_of_record\n' +} > "$_covtf" +_covout="$(python3 "$_covtab" "$_covtf" --limit 0 2>&1)" + +check "premise: the generator produced a row for each of the three files" \ + "$(printf '%s\n' "$_covout" | grep -cE '^\s+(worst\.c|perfect\.c|nobranch\.h)')" "3" + +check "the least covered file sorts first (#974)" \ + "$(printf '%s\n' "$_covout" | grep -oE '(worst|perfect|nobranch)\.[ch]' | head -1)" "worst.c" + +check "a file at 100% reads 100.0%, not a small number (#974)" \ + "$(printf '%s\n' "$_covout" | awk '/perfect\.c/ {print $2}')" "100.0%" + +check "the rate carries hit/found so a reader can check it (#974)" \ + "$(printf '%s\n' "$_covout" | awk '/worst\.c/ {print $2, $3}')" "25.0% 25/100" + +# The one that matters for ordering: an absent counter is not a zero. A `-` keeps +# the file where its line rate puts it; a 0.0% would claim it is the worst branch +# coverage in the tree on the strength of having no branches. +# +# THE PROPERTY FIRST, POSITION SECOND. The first version asserted the dash by field +# number and got it wrong -- absent cells collapse to one token each, so the dashes +# are $4 and $5, not $6. The property does not depend on where the cell lands. +# ANCHORED, because `0\.0%` is a SUBSTRING of `50.0%` and this row's line rate is +# exactly that. The unanchored form reported one match and read as the generator +# printing a zero rate for an absent counter, which it does not. +check "a row with absent counters never reads as 0.0% (#974)" \ + "$(printf '%s\n' "$_covout" | awk '/nobranch\.h/' | grep -cE '(^|[^0-9.])0\.0%')" "0" +check "an absent counter prints a dash instead (#974)" \ + "$(printf '%s\n' "$_covout" | awk '/nobranch\.h/ {print $4, $5}')" "- -" + +check "and the function rate is computed too, not copied (#974)" \ + "$(printf '%s\n' "$_covout" | awk '/worst\.c/ {print $4, $5}')" "25.0% 1/4" + +rm -f "$_covtf" +unset _covtab _covtf _covout +unset -f _cov_code From c302c670af32d9f56ace978a9347d44e75515c02 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Fri, 11 Sep 2026 21:02:50 +0000 Subject: [PATCH 2/2] test: regenerate the ledger for the twelve coverage-table checks (#974) Twelve new check names in harness_selftest, which the ledger covers, so the gate refuses the PR until the ledger has seen them. #972 merged first, so this is the PR that paid the rebase. The sequence was: drop my earlier ledger commit, rebase onto main, re-run the suite on the REBASED tree, guard that log, merge it, and DERIVE the census from the resulting file. The derived value happens to equal main's 847 plus twelve, and it is not set that way -- the number has only ever been right on purpose when read back from the ledger. rows 847 -> 859 checks_never_observed_red 847 -> 859 awk -F'\t' '$4=="never"' | wc -l suites_not_covered 250 unchanged; this adds no suite Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/check_ledger.tsv | 12 ++++++++++++ test/check_ledger_budget.txt | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 30752640..ef8e0c5c 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -175,17 +175,29 @@ harness_selftest 240-the-nightly-enumeration-must-not premise: at least three ni harness_selftest 240-the-nightly-enumeration-must-not premise: the nightly paragraph was located and is not empty never - harness_selftest 240-the-nightly-enumeration-must-not premise: the nightly workflow and the testing doc are both present never - harness_selftest 250-the-coverage-runner-must-refuse GCOV_PREFIX is exported before the suites run (#740) never - +harness_selftest 250-the-coverage-runner-must-refuse a file at 100% reads 100.0%, not a small number (#974) never - +harness_selftest 250-the-coverage-runner-must-refuse a row with absent counters never reads as 0.0% (#974) never - +harness_selftest 250-the-coverage-runner-must-refuse an absent counter prints a dash instead (#974) never - +harness_selftest 250-the-coverage-runner-must-refuse and no lcov --list call survives in the runner (#974) never - +harness_selftest 250-the-coverage-runner-must-refuse and the function rate is computed too, not copied (#974) never - harness_selftest 250-the-coverage-runner-must-refuse premise: both stray-counter probes were located never - harness_selftest 250-the-coverage-runner-must-refuse premise: both the counter refusal and the lcov capture were located never - +harness_selftest 250-the-coverage-runner-must-refuse premise: stripping comments leaves the runner's code behind never - harness_selftest 250-the-coverage-runner-must-refuse premise: the containment and the copy were both located never - harness_selftest 250-the-coverage-runner-must-refuse premise: the coverage runner is present and parses never - +harness_selftest 250-the-coverage-runner-must-refuse premise: the generator produced a row for each of the three files never - harness_selftest 250-the-coverage-runner-must-refuse premise: the guard's count directory and the capture's were both located never - harness_selftest 250-the-coverage-runner-must-refuse premise: the redirect, the suite invocation and the copy-back were located never - +harness_selftest 250-the-coverage-runner-must-refuse premise: the table generator is present and parses never - harness_selftest 250-the-coverage-runner-must-refuse the copy-back refuses a destination outside the tree (#740) never - harness_selftest 250-the-coverage-runner-must-refuse the counters are returned beside their objects before the refusal (#740) never - +harness_selftest 250-the-coverage-runner-must-refuse the coverage runner computes the table instead of parsing lcov --list (#974) never - harness_selftest 250-the-coverage-runner-must-refuse the coverage runner refuses zero counters before it calls lcov (#740) never - +harness_selftest 250-the-coverage-runner-must-refuse the least covered file sorts first (#974) never - +harness_selftest 250-the-coverage-runner-must-refuse the rate carries hit/found so a reader can check it (#974) never - harness_selftest 250-the-coverage-runner-must-refuse the refusal looks in GCOV_PREFIX before the tree-wide walk (#740) never - harness_selftest 250-the-coverage-runner-must-refuse the zero-counter guard counts the directory lcov captures (#740) never - +harness_selftest 250-the-coverage-runner-must-refuse while the lcov --summary call stays, being correct on both builds (#974) never - harness_selftest 260-an-ordered-comparison-must-use-the and it is the same suites, not merely the same count never - harness_selftest 260-an-ordered-comparison-must-use-the every diff_query_ordered site actually names an ORDER BY never - harness_selftest 260-an-ordered-comparison-must-use-the every suite using the ordered oracle asserts its premise never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index e923ee03..81741bf1 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -34,4 +34,4 @@ suites_not_covered 250 # Without that it is a hand-maintained count that drifts, which is the failure # this repository has spent a day proving. It is not a ceiling; it is a # measurement that must be true. -checks_never_observed_red 847 +checks_never_observed_red 859