Skip to content

Codify the involuntary-skip rule: tools/_skip_policy.py plus a test_pytest_boundary guard #178

Description

@JC-000

The class, and why a third pass is needed

An involuntary skip is a failure; an explicit skip is allowed but must never be
silent.
That standard was adopted in #158 (audit commit 7497e48), which swept
six suites that exited 0 having tested nothing.

It came back one day later via #157, was re-found as #165, and was fixed
again in PR #172. Two closures, one reintroduction, and the reintroduction
happened because #157's test did not exist when #158's sweep was written.

A sweep cannot prevent the next one. This issue proposes the two things that
can — a shared helper so the correct shape is one import away, and a guard in
tools/test_pytest_boundary.py so the wrong shape cannot be merged
— and
carries the survey of the eight files that currently have it.

Survey

Line numbers verified against master 0b55c30.

1. tools/test_build_flags_stamp.py — seven sites → filed separately as #177

Seven tests, each opening with print(f"SKIP: {missing} not on PATH") and a bare
return. Guards at :139-142, :172-175, :193-196, :225-228, :246-249,
:274-277, :307-310. A bare return from a pytest test is a pass, and the
module is in pytest.ini testpaths, so a machine without cc65 reports
7 passed in 0.01s. Details, both output channels, and the #174 cross-link in
#177 — it is the sharpest instance and the one with a scheduling conflict, so
it has its own issue. Fix it with this sweep, not separately.

2. tools/test_p384_symbols.py:129-131

if backend != "uci":
    print(f"  SKIP: dual-overlay smoke test is UCI-only (backend={backend})")
    return 0

Module-level, before any work. Explicit return 0 on a condition the caller did
not choose.

3. tools/test_ecdsa_p384_kat.py:840-841, verdict at :857-872

if not os.environ.get("U64_HOST"):
    print("  SKIP: --u64 requested but U64_HOST not set in env")

u_pass and u_fail stay 0, so :860 prints U64: 0/0 passed, :863
computes total_fail = v_fail + 0, and :865 prints OVERALL: PASS with
:872 returning 0. The user explicitly asked for hardware with --u64 and got a
pass without it. Note the existing precedent one branch away: :866-871 already
prints a paragraph explaining that --sha-only "can never report OVERALL: PASS"
— the file knows this shape is a hazard and handles one instance of it.

4-7. tests/rig_phase{1_dhcp,2_http,3_https,3_https_1mhz}.py

A shared convention, four copies:

def _skip(reason: str) -> int:
    print(f"SKIP: {reason}")
    return 0
file _skip def prereq site build-failure site
rig_phase1_dhcp.py :41-43 :71 :74
rig_phase2_http.py :43 :111 :114
rig_phase3_https.py :85 :472 :475
rig_phase3_https_1mhz.py :101 :581 :584

The sharpest edge is the last column, identical in all four:

return _skip("c64-https.prg could not be built")

A failed make is laundered into exit 0. That is not a missing
prerequisite — it is the build breaking, reported as "nothing to do here". All
four codify the convention in their docstrings ("Exit codes:" at :12, :12,
:17, :23), which is why it spread.

8. tests/rig_vice_https_macos.py — same convention, different mechanism

Correction to the survey as I received it: this file has no _skip()
helper
. It has the same behaviour written inline at :228-232 (print("SKIP: rig not ready:")return 0) and codified in its docstring at :37:

Exit codes: 0 PASS / 0 SKIP (printed) / 1 FAIL.

It also does not share the build-failure defect: :234-236 calls
_build_prg() and then assert os.path.exists(PRG_PATH), so a failed build
raises rather than skipping. Include it in the sweep for the docstring
convention and the inline return 0, not for laundering a build failure.

Borderline — mention, do not prioritise

  • tools/test_http.py. The skip site itself (:597-607) is close to
    correct: it prints !! SINK VECTORS NOT RUN twice, names the command that
    covers them, and escalates to a failure under C64_EXPECT_SINK. The gap is at
    the other end — :1047 is a bare sys.exit(0 if failed == 0 else 1) with no
    total > 0 guard, so a run in which everything skipped still exits 0. Two
    sibling files got that guard; this one did not. (The site is at :597-607,
    not :593-607.)
  • tools/test_x25519.py:662-667. skipped_groups drops the fe_* groups
    under sibling_build. Lowest priority — the condition is a deliberate build
    choice, and the groups are recorded rather than vanished.

Confirmed clean — listed so nobody re-audits

The six #158 files, plus test_x509.py, test_ecdsa_kat_oracle.py,
test_tls_p384_negotiation.py, test_hs_sequence.py, test_tls_deframer.py,
test_transcript_large.py, test_finished_verify.py, test_net.py,
test_package_verify.py, test_pytest_boundary.py, test_reserved_test_host.py,
test_net_test_env.py, run_all_tests.py, all of tools/uci/, and
tools/package/*.sh.

Spot-checked here rather than taken on trust: test_x509.py:765-771 prints
GROUPS THAT COULD NOT RUN (counted as failures) and says the assertions never
executed; test_finished_verify.py:316,324, test_hs_sequence.py:373,391,
test_tls_deframer.py:761,767, test_ecdsa_kat_oracle.py:393,399,
test_transcript_large.py:62,176 and test_tls_p384_negotiation.py:368,390 all
use FATAL: for a missing PRG or missing labels. These two are the exemplars
CLAUDE.md already names; the rest follow their shape.

Proposal

Part 1 — tools/_skip_policy.py

cannot_run(reason, *, executed, total, opt_out_env)

Prints the standard block — the reason, N of M checks executed, what the run
therefore certifies nothing about, and the name of the opt-out variable — and
returns 2 ("could not run", distinct from 1 "a check failed"), or 0 when
opt_out_env is set in the environment. Plus a require() wrapper for the
pytest side that raises so pytest records a failure.

PR #172 is the worked example of the correct shape, including one subtlety worth
copying verbatim:

Under pytest the skip reason is the only channel — -ra (pinned in
pytest.ini addopts) prints that string and nothing else — so the reason
carries the vacuity warning itself rather than relying on module stdout that
pytest swallows.

Without that, an exit-0 opt-out reads as a bare 3 skipped and the warning is
gone. The helper must put the vacuity text in the reason string, not print it.

Part 2 — the guard, and this is the part that matters

A helper on its own is just another convention to miss, which is exactly how
#157 reintroduced the class one day after #158 closed it. What prevents a third
reintroduction is a test.

tools/test_pytest_boundary.py already enforces the testpaths /
norecursedirs contract in both directions by AST, and already fails when a new
pure-logic module is not listed. Extend it to assert that no
tools/test_*.py module reaches return 0, sys.exit(0) or pytest.skip from
a prerequisite branch without going through _skip_policy.

This must land alongside the eight-file sweep, not after it. A guard added to
an already-clean tree is a guard nobody has seen fail, and #161's rule applies to
it directly: would this pin change if the fact did? Land it red against the
eight files above, watch it fail, then sweep.

Two design notes for whoever writes it, both from #161's qualifications:

  • It must fail loudly when its pattern finds nothing — a matcher that
    silently matches zero call sites is the same vacuous-green shape this whole
    issue is about, one level up.
  • Mutation-test it, and do not over-read the result: it "proves an assertion
    can fail. It does not prove it fails for the right reason."

The tests/ rigs (items 4-8) are outside tools/test_*.py and so outside the
guard's natural scope. Either widen it to tests/rig_*.py or accept that the
rigs are guarded by convention only — worth deciding explicitly rather than by
omission, since four of the five are where the build-failure laundering lives.

Sequencing

#177 (test_build_flags_stamp.py) overlaps #174: its
test_backend_flip_removes_the_other_backends_prg deliberately depends on
make -n being destructive, so whoever fixes #174 rewrites a test this sweep
also touches. Coordinate those two; the other seven files are independent.

Found during a supervised agent session, 2026-08-31.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions