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/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 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