Skip to content

test(parity): a divergence must fail the job, not just print - #51

Merged
blairham merged 2 commits into
mainfrom
parity-gate
Sep 8, 2026
Merged

blairham merged 2 commits into
mainfrom
parity-gate

Conversation

@blairham

@blairham blairham commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Found while adding a regression check for #50. This is the reason that bug shipped.

The parity suite could not fail

Every comparison is recorded with addResult and rendered into the report, but nothing ever asserts on one — no t.Error anywhere in the suite, and TestMain forces a non-zero exit only when zero checks ran. A real divergence printed [FAIL], lowered the percentage, wrote it into the JSON artifact, and exited 0.

Which makes the job description in ci.yml untrue as written:

Parity is this project's entire claim, so a change that breaks it fails before review rather than after a merge to main.

It did not. It reported.

Measured

On a tree carrying two genuine divergences (missing config made to exit 0 instead of 1):

report exit
before 76/78 — FAILURES: 2 checks failed 0
after 76/78 — FAILURES: 2 checks failed 1

And a clean tree is 78/78 exit 0 both ways — so this tightens the gate without moving the bar. Verified against real upstream 4.6.2 with PARITY_REQUIRE=1.

Not hypothetical

pre-commit run --files a b has been broken in every release while the headline read 78/78 (#50). The suite had no multi-path case — and even once one exists, only the filesystem assertion discriminates: both binaries exit 1, one because the hook failed and one because it could not parse. A gate that cannot fail plus a check that cannot discriminate is how a 100% parity claim and a broken common flag coexist.

docs/parity.md now says a divergence fails the run, and says plainly that this was not always true.

Verification

make check clean, make lint 0 issues, clean parity run exits 0 at 78/78.

Refs #43

The parity suite could not fail. Every comparison was recorded with
addResult and rendered into the report, but nothing ever asserted on
one: no t.Error, and TestMain forced a non-zero exit only when *zero*
checks ran. So a real divergence printed [FAIL], lowered the percentage
in the report, and exited 0.

That makes the job in ci.yml untrue as described -- "Parity is this
project's entire claim, so a change that breaks it fails before review
rather than after a merge to main." It did not. It reported.

Measured, on a tree carrying two genuine divergences (missing config
made to exit 0 instead of 1):

  without this change   76/78, "FAILURES: 2 checks failed", exit 0
  with this change      76/78, same report,                 exit 1

and a clean tree is 78/78 exit 0 both ways, so this tightens the gate
without moving the bar.

This is not hypothetical. `pre-commit run --files a b` was broken in
every release while the headline read 78/78 -- the suite had no
multi-path case, and even if it had, only the filesystem assertion
discriminates: both binaries exit 1, one because the hook failed and
one because it could not parse.

Refs #43
The four --files comparisons added in #50 are checks like any other, so
the headline and the parity table have to count them. Left stale, the
number in the docs and the number the suite produces drift apart, which
is the exact failure this file exists to prevent.
@blairham
blairham merged commit 0c324e0 into main Sep 8, 2026
5 checks passed
@blairham
blairham deleted the parity-gate branch September 8, 2026 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant