Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion test/check_ledger.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,10 @@ harness_selftest 410-a-check-must-have-been-red a check observed red gains the d
harness_selftest 410-a-check-must-have-been-red a check the ledger has never seen is refused never -
harness_selftest 410-a-check-must-have-been-red a clean status says nothing and does not fail the major never -
harness_selftest 410-a-check-must-have-been-red a date that is not a date is refused rather than stored never -
harness_selftest 410-a-check-must-have-been-red a deliberate break says so with --mutation never -
harness_selftest 410-a-check-must-have-been-red a gate over a nonexistent log is an integrity failure, not a pass never -
harness_selftest 410-a-check-must-have-been-red a later green run does not erase an observation never -
harness_selftest 410-a-check-must-have-been-red a log carrying a FAIL is refused when no reason is given never -
harness_selftest 410-a-check-must-have-been-red a name that appeared while another disappeared is reported as a rename never -
harness_selftest 410-a-check-must-have-been-red a named mutation is recorded against the check that reddened never -
harness_selftest 410-a-check-must-have-been-red a real refusal is a different status from an integrity failure never -
Expand All @@ -633,6 +635,7 @@ harness_selftest 410-a-check-must-have-been-red an empty log is one too, because
harness_selftest 410-a-check-must-have-been-red an integrity failure says regenerating will not help never -
harness_selftest 410-a-check-must-have-been-red an older observation does not overwrite a newer one never -
harness_selftest 410-a-check-must-have-been-red and CI collects from the retained path rather than the deleted one never -
harness_selftest 410-a-check-must-have-been-red and a genuine observation says so with --reds-are-real never -
harness_selftest 410-a-check-must-have-been-red and a level prior carries no distance, so zero is silent never -
harness_selftest 410-a-check-must-have-been-red and a log that does not reconcile with its own checks run: never -
harness_selftest 410-a-check-must-have-been-red and a newer one does never -
Expand All @@ -654,6 +657,7 @@ harness_selftest 410-a-check-must-have-been-red and it says why, rather than fal
harness_selftest 410-a-check-must-have-been-red and never says a change introduces a file at a ref that is not there never -
harness_selftest 410-a-check-must-have-been-red and none of them ends in a tab never -
harness_selftest 410-a-check-must-have-been-red and not against one that stayed green never -
harness_selftest 410-a-check-must-have-been-red and nothing is written, so a refused merge is not half-applied never -
harness_selftest 410-a-check-must-have-been-red and one that stayed green keeps its debt never -
harness_selftest 410-a-check-must-have-been-red and only when there is a base, so a push build does not fail on it never -
harness_selftest 410-a-check-must-have-been-red and records neither as ever having been red never -
Expand All @@ -663,6 +667,7 @@ harness_selftest 410-a-check-must-have-been-red and the history it is about to l
harness_selftest 410-a-check-must-have-been-red and the message says how to fix it, because regenerating is the intended action never -
harness_selftest 410-a-check-must-have-been-red and the refusal names both values never -
harness_selftest 410-a-check-must-have-been-red and the refusal names how many failed, so the author can narrow the run never -
harness_selftest 410-a-check-must-have-been-red and the refusal names the check that reddened never -
harness_selftest 410-a-check-must-have-been-red and the refusal names the raise never -
harness_selftest 410-a-check-must-have-been-red and the runner names no remote at that call site never -
harness_selftest 410-a-check-must-have-been-red auto refuses when GITHUB_BASE_REF names a ref that is not here never -
Expand All @@ -671,7 +676,8 @@ harness_selftest 410-a-check-must-have-been-red auto with no base ref and no ups
harness_selftest 410-a-check-must-have-been-red both failure arms fail the major never -
harness_selftest 410-a-check-must-have-been-red but that suite is counted as not covered, which is the debt never -
harness_selftest 410-a-check-must-have-been-red control: a well-formed log still merges never -
harness_selftest 410-a-check-must-have-been-red control: the same log merges without --mutation never -
harness_selftest 410-a-check-must-have-been-red control: an all-PASS log still merges with no flag at all never -
harness_selftest 410-a-check-must-have-been-red control: the same log merges with a different reason, so the refusal above is --mutation-across-two-checks and not the log never -
harness_selftest 410-a-check-must-have-been-red each says what was wrong with the input never -
harness_selftest 410-a-check-must-have-been-red every committed row has five fields never -
harness_selftest 410-a-check-must-have-been-red every row has five fields and no trailing tab never -
Expand Down
2 changes: 1 addition & 1 deletion test/check_ledger_budget.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 756
checks_never_observed_red 762
35 changes: 35 additions & 0 deletions test/pgc_ledger.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,38 @@ def cmd_merge(args):
f"collateral damage as evidence. Merge without --mutation, or narrow the "
f"run to the check the mutation targets.")

# A RED NEEDS A REASON (#946). `merge` already refuses a log that does not
# RECONCILE, and reconciliation is not the property that matters: both logs that
# poisoned this ledger on the day it landed reconciled. One was 827 records
# against `checks run: 827` with fifteen checks red because the tree was copied
# without `.git`; the other was one FAIL from an unfinished change. Two
# independent routes on day one, from the two people who knew the tool best, and
# a third -- a run against a stale .so -- costs no imagination at all.
#
# An environment red and a real regression are IDENTICAL in the log. Nothing in a
# RESULT record says which, so the tool makes the caller assert it rather than
# guess, the same way check_ledger_budget.txt names a census apart from a ceiling.
#
# NOT "refuse FAILs unless --mutation". A genuine CI red is the most valuable row
# this ledger can hold and it has no mutation to name, so that rule would refuse
# precisely the entry the ledger exists for -- the deadlock the budget file
# already argues against for checks_never_observed_red.
#
# Refused BEFORE any row is built, so a declined merge is never half-applied.
if not args.mutation and not args.reds_are_real:
reddened = sorted({key for _p, seen in runs
for key, vs in seen.items() if "FAIL" in vs})
if reddened:
listed = "\n".join(f" {s}\t{p}\t{n}" for s, p, n in reddened[:6])
more = "" if len(reddened) <= 6 else f"\n ... and {len(reddened) - 6} more"
raise LedgerError(
f"{len(reddened)} check(s) are red in these logs and nothing says "
f"why:\n{listed}{more}\n A log can reconcile perfectly and still "
f"be evidence about your environment rather than about the code -- a "
f"tree without .git, an unfinished change, a stale .so. Pass "
f"--mutation NAME if you broke it deliberately, or --reds-are-real if "
f"this is a genuine observation of the code under test.")

for path, seen in runs:
for key, verdicts in sorted(seen.items()):
if key not in rows:
Expand Down Expand Up @@ -591,6 +623,9 @@ def main(argv=None):
m.add_argument("--ledger", required=True)
m.add_argument("--date", default="unknown")
m.add_argument("--mutation", default="")
m.add_argument("--reds-are-real", action="store_true",
help="the FAIL records in these logs are a genuine observation "
"of the code under test, not an artifact of the environment")
m.add_argument("logs", nargs="+")
m.set_defaults(fn=cmd_merge)

Expand Down
15 changes: 15 additions & 0 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2037,6 +2037,21 @@ that the mutation kills that check. A run with more than one failing check is
refused with the count, and a single failure still carries the mutation on the
check that reddened.

### `test_a_reconciling_log_with_a_red_is_not_evidence_on_its_own`

`merge` already refuses a log that does not **reconcile**, and reconciliation is not
the property that matters: both logs that poisoned this ledger on the day it landed
reconciled. One was 827 records against `checks run: 827`, with fifteen checks red
because the tree had been copied without `.git`; the other was a single `FAIL` from
an unfinished change.

An environment red and a real regression are identical in the log, so the tool cannot
tell them apart and makes the caller say which it is: `--mutation NAME` for a
deliberate break, `--reds-are-real` for a genuine observation. Refusing reds outright
was rejected — a real CI red is the most valuable row the ledger holds and has no
mutation to name. An all-`PASS` log still merges with no flag, which is the control.
See #946.

### `test_two_runs_of_a_check_are_not_a_duplicate_of_it`

Merging logs first cannot tell *the same check in two runs* from *the same name twice in
Expand Down
61 changes: 52 additions & 9 deletions test/pytest/test_mutation_ledger.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def test_two_runs_of_a_check_are_not_a_duplicate_of_it(tmp_path, expect):
twice = _w(tmp_path, "twice.log",
"RESULT\tdemo\tpart1\tsame\tPASS\t\n"
"RESULT\tdemo\tpart1\tsame\tFAIL\t\nchecks run: 2\n")
out, _ = _run("merge", "--ledger", _w(tmp_path, "l2.tsv", ""), "--date", "2026-09-10", twice)
out, _ = _run("merge", "--reds-are-real", "--ledger", _w(tmp_path, "l2.tsv", ""), "--date", "2026-09-10", twice)
expect.num(out.count("duplicate check name in one run, so one ledger row covers 2: "
"demo\tpart1\tsame"), 1,
"the same name twice in ONE log is a duplicate, and is named")
Expand All @@ -158,7 +158,7 @@ def test_renames_are_grouped_by_part_and_scanned_against_one_run(tmp_path, expec
"RESULT\tdemo\tpartA\tnew A\tPASS\t\n"
"RESULT\tdemo\tpartB\tstable B\tPASS\t\n"
"RESULT\tdemo\tpartB\tadded B\tPASS\t\nchecks run: 3\n")
_run("merge", "--ledger", ledger, "--date", "2026-09-01", before)
_run("merge", "--reds-are-real", "--ledger", ledger, "--date", "2026-09-01", before)

out, rc = _run("rename-scan", "--ledger", ledger, after)
expect.num(out.count("possible rename: old A -> new A"), 1,
Expand Down Expand Up @@ -354,14 +354,14 @@ def stored():
return f[3]
return None

_run("merge", "--ledger", led, "--date", "2026-09-10", red)
_run("merge", "--ledger", led, "--date", "2026-09-01", red)
_run("merge", "--reds-are-real", "--ledger", led, "--date", "2026-09-10", red)
_run("merge", "--reds-are-real", "--ledger", led, "--date", "2026-09-01", red)
expect.text(stored(), "2026-09-10", "an older observation does not overwrite a newer one")
_run("merge", "--ledger", led, "--date", "2026-09-20", red)
_run("merge", "--reds-are-real", "--ledger", led, "--date", "2026-09-20", red)
expect.text(stored(), "2026-09-20", "and a newer one does")
_run("merge", "--ledger", led, red)
_run("merge", "--reds-are-real", "--ledger", led, red)
expect.text(stored(), "2026-09-20", "and an undated merge does not erase a known date")
expect.num(_run("merge", "--ledger", led, "--date", "not-a-date", red)[1], 2,
expect.num(_run("merge", "--reds-are-real", "--ledger", led, "--date", "not-a-date", red)[1], 2,
"a date that is not a date is refused rather than stored")


Expand All @@ -380,12 +380,55 @@ def test_a_mutation_names_one_check_not_every_casualty(tmp_path, expect):
expect.num(rc, 2, "--mutation across two failing checks in one run is refused")
expect.num(out.count("2 checks failed"), 1,
"and the refusal counts them, so the author can narrow the run")
expect.num(_run("merge", "--ledger", led, "--date", "2026-09-10", two)[1], 0,
"control: the same log merges without --mutation")
expect.num(_run("merge", "--reds-are-real", "--ledger", led, "--date", "2026-09-10", two)[1], 0,
"control: the same log merges with a different reason, so the\n refusal above is --mutation-across-two-checks and not the log")

one = _w(tmp_path, "one.log", RED)
_run("merge", "--ledger", led, "--date", "2026-09-10", "--mutation", "M", one)
rows = [l.split("\t") for l in pathlib.Path(led).read_text().splitlines() if l.strip()]
tagged = [r[2] for r in rows if len(r) > 4 and "M" in r[4].split(";")]
expect.rows([[n] for n in sorted(tagged)], [["first check"]],
"and a single failure still carries it, on the check that reddened")


def test_a_reconciling_log_with_a_red_is_not_evidence_on_its_own(tmp_path, expect):
"""#946: self-consistency is not the property that matters.

`merge` already refuses a log that does not reconcile. Both of the logs that
poisoned this ledger on the day it landed RECONCILED: @jdatcmd's was 827 records
against `checks run: 827` with fifteen checks red because the tree was copied
without `.git`, and @OffgridwithJD's was one FAIL from an unfinished change. A
rule about reconciliation would have caught neither.

An environment red and a real regression are IDENTICAL in the log, so the tool
cannot tell them apart and must make the caller say which it is -- the same move
`check_ledger_budget.txt` makes when it names a census apart from a ceiling.
"""
led = _w(tmp_path, "l.tsv", "")
red = _w(tmp_path, "red.log", RED)

out, rc = _run("merge", "--ledger", led, "--date", "2026-09-10", red)
expect.num(rc, 2, "a log carrying a FAIL is refused when no reason is given")
expect.at_least(out.count("first check"), 1,
"and the refusal names the check that reddened")
expect.text("absent" if not pathlib.Path(led).read_text().strip() else "written",
"absent", "and nothing is written, so a refused merge is not half-applied")

# THE TWO WAYS TO SAY WHY, both of which must still work. Refusing reds outright
# would refuse the most valuable row the ledger can hold -- a genuine CI red, which
# has no mutation to name -- and that is the deadlock the budget file already
# argues against for checks_never_observed_red.
led2 = _w(tmp_path, "l2.tsv", "")
expect.num(_run("merge", "--ledger", led2, "--date", "2026-09-10",
"--mutation", "M", red)[1], 0,
"a deliberate break says so with --mutation")
led3 = _w(tmp_path, "l3.tsv", "")
expect.num(_run("merge", "--ledger", led3, "--date", "2026-09-10",
"--reds-are-real", red)[1], 0,
"and a genuine observation says so with --reds-are-real")

# THE CONTROL, without which this arm passes on a tool that refuses everything.
led4 = _w(tmp_path, "l4.tsv", "")
green = _w(tmp_path, "green.log", GREEN)
expect.num(_run("merge", "--ledger", led4, "--date", "2026-09-10", green)[1], 0,
"control: an all-PASS log still merges with no flag at all")
Loading
Loading