test: six suites could exit 0 having tested nothing - #158
Merged
Conversation
Each of these six exits reported success for a run in which the code under test was never executed. None of them changes what the suites cover; all six change what a green run means. Applying the rule from 7497e48 — an involuntary skip is a failure, an explicit skip is allowed but must never be silent: test_x509_name.py a missing x509_verify_hostname label (i.e. any BACKEND=ip65 build) returned 0, so the whole 23-vector hostname suite — the only coverage server-name validation has — passed green having verified nothing. Now returns 2. test_x509_name.py a vector whose DER exceeds cert_buf was dropped with `continue`, leaving the denominator. Now counted as a failure and listed at the end. The vectors most likely to outgrow cert_buf are the real CA-issued leaves. test_tls_record.py printed "[?] No tests ran" and then exited 0. test_tls_handshake.py same shape. Both now exit 1 on zero checks. test_ecdsa_p384_kat.py --sha-only synthesised `valid` from the vector's own expected_valid, so every vector compared equal to its expectation and OVERALL: PASS was printed for a run that never called ecdsa_verify_384. It now returns a no-verdict result, counted as a failure; --sha-only is a diagnostic and can no longer report PASS. test_dns.py prereq checks exited 0 with a SKIP line, so a test_http_integration.py host with no TAP rig (every macOS dev machine) reported a green DNS/HTTP path. Now exit 2 ("could not run", distinct from 1 "tests failed"); C64_NET_TESTS_OPTIONAL=1 makes the skip an explicit, loud, exit-0 choice. Expect currently-green runs to turn red. That is the point: the A/B evidence for all six is in the PR body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 31, 2026
Closed
JC-000
added a commit
that referenced
this pull request
Aug 31, 2026
…enssl
`real_cert_der()` wrapped a whole subprocess in `except Exception: return
None`, so every way it could fail collapsed into one message — "openssl
unavailable" — and `build_vectors()` then dropped the six real-certificate
vectors while the suite exited 0.
Both halves are environment-gated, which is why the bug survived: with a
`tools/https_e2e/certs/server.pem` on disk and openssl installed nothing
looks wrong. Measured in a tree with no generated cert (a fresh clone, and
this worktree): the suite printed "openssl unavailable" with openssl 3.6.2
on PATH, built no `real cert /` vector at all, and exited 0. Measured with
the cert present but openssl removed from PATH: the message was right for
once, and the six vectors were dropped just the same, still exiting 0.
Two changes:
- Mint on demand. `ensure_certs("p256")` is idempotent, stdlib-only and
already the shared entry point for the listener and the other in-tree
suites, so the fixture is produced rather than missed. The vectors are
derived from `gen_certs.DEFAULT_SANS`, so they follow #164's rename to
`foo.invalid` / `www.foo.invalid` without further edits.
- Fail loudly when it still cannot be produced. `RealCertUnavailable`
names which cause fired — generation failed, the file is absent or
unreadable, the file is not PEM, the decode is not a DER certificate —
and `main()` aborts with exit 2 before launching VICE. An involuntary
skip is a failure (#158's standard); these are the only vectors that
parse a certificate a TLS listener actually produced.
`real_cert_rows()` declares those vectors as data and `main()` compares
what was BUILT against what was DECLARED, so a future refactor cannot drop
one silently. No total is asserted: the suite's vector count is
environment-dependent (23 with a network, 17 under `X509_NAME_OFFLINE=1`)
and pinning it would be the same defect one level up. The header line now
reports how many vectors came from the real certificate, which is what
distinguishes an honest 17 from the two broken ones — the real-cert six and
the live six have the same 3-accept/3-reject shape, so the totals alone
cannot tell them apart.
openssl is no longer consulted at all. A PEM is base64-armoured DER,
`gen_certs.san_dns_names` already walks one with the stdlib, and openssl is
not a documented dependency of this repo (PR #96 went the other way and
removed `cryptography` from the cert path). Reporting a missing openssl
accurately would still have let it drop six vectors; removing the
dependency means it cannot — verified by a run with openssl absent from
PATH, which mints and passes 23/23.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Step 1 of the test-suite migration: the six suites that could exit 0 having executed none of the code they test. No new coverage — this changes only what today's runs mean.
Rule applied, from audit commit
7497e48: an involuntary skip is a failure; an explicit skip is allowed but must never be silent. Accounting copied fromtest_x509.py:219-224,773,test_ecdsa_kat_oracle.py:459-462,test_finished_verify.py:322-326.Expect currently-green runs to turn red. That is the point — the A/B evidence for every one of the six is below, so newly-red runs are not a regression.
Toolchain for all runs:
BACKEND=uciPRG, sha2567461ab0eb1bc488659fecefe1f828290b7fcfaa55057fc8f90d4b5fbae39946e, built in this worktree aftermake clean. Harness python/Users/someone/Documents/c64-ChaCha20-Poly1305/.venv/bin/python3.1.
test_x509_name.py— missing label returned 0The whole 23-vector hostname suite — the only coverage
x509_verify_hostnamehas — exited green on any build not exporting the label, i.e. everyBACKEND=ip65build.How the vacuity was reproduced: not with an ip65 PRG.
.incbinresolves against the CWD and this is a worktree three levels down, so an ip65 build here would silently assemble the parent checkout's blob (CLAUDE.md, "Do not build ip65 in a nested git worktree"). Instead the exact trigger condition was reproduced directly: thex509_verify_hostnameline was removed frombuild/labels.txtof the UCI build, which is precisely the state an ip65 build presents toLabels.address().before
after
Green path unchanged — full VICE run against the unmodified UCI labels:
2.
test_x509_name.py— oversized vectors left the denominatorA vector whose DER exceeded
cert_bufwas dropped with a barecontinue: declared, no verdict, invisible inRESULTS: n/n passed. Now counted as a failure and listed under "VECTORS THAT COULD NOT RUN", same rule as the KAT oracle's "every declared vector must produce a verdict".Not triggered on today's vectors (largest live leaf is en.wikipedia.org at 1,636 B against a 2,048 B
cert_buf, see the 17/17 run above) — so this is a latent-vacuity fix, and the vectors most likely to trip it are exactly the real CA-issued leaves. Diff:3 & 4.
test_tls_record.py/test_tls_handshake.py— "No tests ran", exit 0Both printed
[?] No tests ran (routines not yet implemented?)and thensys.exit(0 if failed == 0 else 1)— zero checks, zero failures, exit 0.Reaching
total == 0needs every test group to return(0, 0), so both were driven through their own realmain()withrun_testsreturning(0, 0)and only the environment stubbed (make, VICE,wait_for_text). Identical driver before and after; the summary/exit logic exercised is the module's own.before (
test_tls_record)after
before (
test_tls_handshake)after
Non-empty runs are untouched, checked both ways through the same driver:
tools/run_all_tests.pyimportsrun_testsfrom these two modules, notmain(), so the aggregate is unaffected.5.
test_ecdsa_p384_kat.py --sha-only— fabricated the expected answer_run_one_vectorreturned"valid": vec["expected_valid"]. Every vector therefore compared equal to its own expectation,_run_backendtallied it as a pass, andOVERALL: PASSwas printed for a run in whichecdsa_verify_384was never executed.The P-384 build does not exist (CLAUDE.md, "Known issues"), so the flow was driven against a fake device that answers every DMA and every
run_subroutine(), returns the correct host-computed SHA-384 for the splice cross-check (so steps 1-8 succeed exactly as on a working overlay), and records which addresses were executed._run_one_vectorand_run_backend— the code under test — run unmodified.before
after
--sha-onlykeeps its diagnostic value (it still reports which of swap / SHA-384 / splice broke, at step granularity) but can no longer report PASS;main()says so explicitly and the usage text now documents it. The full (non---sha-only) path is untouched.6.
test_dns.py/test_http_integration.py— prereq check exited 0A host with no TAP rig — every macOS dev machine, including this one — printed one SKIP line and exited 0, reporting a green DNS path (4 assertions) and a green end-to-end HTTP path (5 assertions) that had not run.
before (real runs, no stubbing — the prerequisite genuinely is absent here)
after
Exit 2 is "could not run", deliberately distinct from 1 "tests failed" (the convention
test_x509_name.pyalready uses for its FATALs). The explicit half of the rule is the opt-in:Neither suite is dispatched by
tools/run_all_tests.py.Not fixed, and why (adjacent, out of this step's scope)
test_x509_name.py's real-certificate block printsNOTE: openssl unavailable — real-certificate vectors skippedand silently drops six vectors. On this machineopensslis on PATH: the actual cause is thattools/https_e2e/certs/server.pemdoes not exist (it is generated by the listener, and only the README is in tree). So the suite quietly runs 17 vectors, not the 23 it is documented as running, and blames the wrong prerequisite. Same class as the six above, but it is a seventh defect and this PR was scoped to six — filing separately rather than widening the diff.Verification
pytestat repo root: 31 passed (unchanged).py_compileclean on all six files.src/,Makefile,CLAUDE.mdor doc changes;tools/test_*.pyonly.🤖 Generated with Claude Code