test: cover the exit-code wiring of the corpus checks - #2
Merged
Conversation
check_docs.py --fast, which CI runs before the tools suite, was already failing on main: three lines said 117 ascii-checked Rust files where check_ascii.py reports 118. The docs job fails before the test job runs, so this had to go first.
The three corpus scripts fail loudly without a corpus -- exit 2, not a silent skip -- so the gap was never SKIP-reads-as-pass. It was that the layer CI and developers actually trust, main() reading a verdict and choosing an exit code, had no tests at all. Their helpers were well covered; the wiring between helper and exit code was not, and this repo has been burned there twice: a counter that vanished while the run printed WARNING and exited 0, and a run that decoded nothing while printing OK. Each mutation reproduces one of those. Disabling the invariant gate in check_metrics_baseline pins a run with rounds=0 and an empty score into baseline.json and exits 0 -- the exact shape of the 13.02 regression. Disabling the dead-counter check prints "Decode errors: 0 ... over 0 decoded rows" and exits 0, which is the bug its own docstring cites. Ignoring problems() reports "0/1 replays validated" and exits 0. check_metrics_baseline's extract() also had no coverage of the nested valplay metrics shape it reads, which is a cross-repo seam that moves without warning. No workflow change: python-checks already discovers tools/tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap was not where it looked
The premise going in was that
validate_corpus.py,check_decode_errors_corpus.pyandcheck_metrics_baseline.pyprint SKIP and exit 0 without a corpus, the trapCONTRIBUTING.mdwarns about. Measured: all three exit 2 with a named reason. They already follow the doctrine. (The warning does apply tocheck_export_baseline.pyandcheck_corpus_baseline.py, which skip when their single pinned replay is absent.)The real gap is that the layer CI and developers actually trust —
main()reading a verdict and choosing an exit code — had no tests at all. The helpers underneath (problems(),dead_counters(),reconcile(),invariants(),compare()) are well covered. The wiring between helper and exit code was not, and this repo has been burned there twice: a counter that vanished while the run printed WARNING and exited 0, and a run that decoded nothing while printing OK.What this adds
22 tests across the three
tools/tests/files, following the existing convention for faking an executable. Each mutation reproduces a historical failure:check_metrics_baselinerounds=0andscore={}is pinned into baseline.json, exit 0 — the exact shape of the 13.02RoundResultsregressiondead_countersincheck_decode_errors_corpus"Decode errors: 0 ... over 0 decoded rows", exit 0 — the bug its own docstring citesproblems()invalidate_corpus"OK: 0/1 replays validated", exit 0check_metrics_baseline'sextract()also had no coverage of the nested valplay metrics shape it reads — a cross-repo seam that moves without warning. Five tests pin it.Verified against the real 215-replay corpus and a locally built
vrfkitbefore writing any of this, so the scripts' behaviour is measured rather than assumed.Also: a pre-existing CI failure
check_docs.py --fastwas already failing onmain— three lines claim 117 ascii-checked Rust files wherecheck_ascii.pyreports 118. The docs step runs before the tools test step inpython-checks, so without this fix the new tests would never have run in CI.Not built, and why
checksum_exportfixture works because it is a structured JSON manifest, not raw replay bytes. These scripts feed real.vrfbytes to the real binary.crates/vrf-container/tests/corpus.rsalready has a hand-built fixture and scopes itself to container smoke-testing — it never touches the replication payload inside a content block, and hand-authoring that is a project of its own.validate_corpus.py's claim is also a distribution claim across the corpus, which one hand-made replay cannot support.workflow_dispatchjob with an out-of-band corpus. Cross-repo checkout needs a PAT, LFS bandwidth bills the other account, and above all a manual job gates nothing on an ordinary PR.Verification
No workflow change:
python-checksalready discoverstools/tests.Handoff
metrics_builds.jsonshows drift on build 13.01 (kast_rounds117→116,trade_kills35→33), consistent with valplay's recent self-damage attribution fix.CONTRIBUTING.mdrequires explaining each changed line before--update, and that is a domain judgement — left alone.