diff --git a/pytest.ini b/pytest.ini index 29ff73b..50e7c1f 100644 --- a/pytest.ini +++ b/pytest.ini @@ -50,7 +50,9 @@ testpaths = tools/test_package_verify.py tools/test_pytest_boundary.py tools/test_reserved_test_host.py + tools/test_rig_skip_contract.py tools/test_runner_coverage.py + tools/test_skip_policy.py tools/test_uci_data_acc.py norecursedirs = .git libs ip65 ip65-build build dist tests tools/uci diff --git a/tests/rig_phase1_dhcp.py b/tests/rig_phase1_dhcp.py index 76ef0ea..75e9f03 100644 --- a/tests/rig_phase1_dhcp.py +++ b/tests/rig_phase1_dhcp.py @@ -9,10 +9,16 @@ Run: PYTHONPATH=tools python3 tests/rig_phase1_dhcp.py -Exit codes: +Exit codes (tools/_skip_policy.py, issue #178): 0 -- PASS - 0 -- SKIP (clearly printed) - 1 -- FAIL + 1 -- FAIL (a check ran and failed) + 0 -- NOT APPLICABLE: this rig is Linux-only, and on any other host + tests/rig_vice_https_macos.py owns the coverage. A named verdict, + never a bare skip. + 2 -- COULD NOT RUN (a prerequisite is missing ON LINUX, or the build is + broken -- nothing was verified). Set C64_ALLOW_SKIP=1 to accept a + prerequisite-missing run as exit 0; a FAILED BUILD is never opted + out of. """ from __future__ import annotations @@ -27,6 +33,9 @@ if _TOOLS not in sys.path: sys.path.insert(0, _TOOLS) +# needs _TOOLS on sys.path, hence the placement below the block above +from _skip_policy import cannot_run, not_applicable # noqa: E402 + PRG_PATH = os.path.join(_REPO_ROOT, "build", "c64-https.prg") # Exact literal from src/boot.asm (menu_msg @ line 424-426). @@ -38,9 +47,42 @@ DHCP_TIMEOUT = 90.0 -def _skip(reason: str) -> int: - print(f"SKIP: {reason}") - return 0 +_CERTIFIES = "ip65 net_dhcp_acquire end-to-end in VICE" + + +def _cannot_run(reason: str, *, opt_out: bool = True) -> int: + """An involuntary skip is a FAILURE -- nothing was verified (issue #178). + + `opt_out=False` for a broken build: a failed `make` is never laundered + into a pass, not even by C64_ALLOW_SKIP. + """ + return cannot_run( + reason, + executed=0, + total=1, + certifies=_CERTIFIES, + opt_out_env="C64_ALLOW_SKIP" if opt_out else None, + ) + + +_COUNTERPART = ( + "tests/rig_vice_https_macos.py owns this coverage on macOS -- its handshake begins with the same ip65 DHCP acquisition" +) + + +def _wrong_platform() -> int: + """A VOLUNTARY skip: this host can never run this rig (issue #178). + + The load-bearing question is what the remedy is. "install iproute2" is + an involuntary skip and stays exit 2 -- but on a non-Linux host there is + no remedy at all, and another rig owns the coverage, so nothing is lost + and exit 2 would be a red nobody can ever clear. + """ + return not_applicable( + f"this rig is Linux-only (br-c64 bridge + netfilter + /proc/net/udp); " + f"this host is {sys.platform} -- {_COUNTERPART}", + certifies=_CERTIFIES, + ) def _ensure_built() -> bool: @@ -59,6 +101,7 @@ def main() -> int: from https_e2e import ( BridgeEnv, check_prerequisites, + platform_supported, launch_vice_on_bridge, shutdown_vice, press_key, @@ -66,12 +109,20 @@ def main() -> int: get_screen_text, ) + # Platform FIRST: check_prerequisites() cannot answer this, because the + # same string ("ip not on PATH") means "installable" on Linux and "wrong + # OS" everywhere else (issue #178). + if not platform_supported(): + return _wrong_platform() + missing = check_prerequisites() if missing: - return _skip("missing prerequisites: " + "; ".join(missing)) + return _cannot_run("missing prerequisites: " + "; ".join(missing)) if not _ensure_built(): - return _skip("c64-https.prg could not be built") + return _cannot_run("c64-https.prg could not be built -- `make` " + "failed; this is a broken build, not a " + "missing prerequisite", opt_out=False) # ---- Run the test ------------------------------------------------------ handle = None diff --git a/tests/rig_phase2_http.py b/tests/rig_phase2_http.py index 765aae5..a62ffc2 100644 --- a/tests/rig_phase2_http.py +++ b/tests/rig_phase2_http.py @@ -9,10 +9,21 @@ Run: sudo PYTHONPATH=tools python3 tests/rig_phase2_http.py -Exit codes: +Exit codes (tools/_skip_policy.py, issue #178): 0 -- PASS - 0 -- SKIP (clearly printed) - 1 -- FAIL + 1 -- FAIL (a check ran and failed) + 0 -- NOT APPLICABLE: this rig is Linux-only. A named verdict, never a + bare skip -- and, uniquely among the four bridge rigs, a verdict + that DOES cost coverage: tests/rig_vice_https_macos.py is the + macOS counterpart for the emulated-RR-Net path, but it drives it + over TLS, so plaintext HTTP specifically has no macOS rig. Exit 0 + anyway because there is no remedy on this platform (see + _wrong_platform), not because nothing is lost. Say so out loud + rather than let the exit code imply otherwise. + 2 -- COULD NOT RUN (a prerequisite is missing ON LINUX, or the build is + broken -- nothing was verified). Set C64_ALLOW_SKIP=1 to accept a + prerequisite-missing run as exit 0; a FAILED BUILD is never opted + out of. """ from __future__ import annotations @@ -27,6 +38,9 @@ if _TOOLS not in sys.path: sys.path.insert(0, _TOOLS) +# needs _TOOLS on sys.path, hence the placement below the block above +from _skip_policy import cannot_run, not_applicable # noqa: E402 + PRG_PATH = os.path.join(_REPO_ROOT, "build", "c64-https.prg") # Screen needles (from src/boot.asm string labels). @@ -40,9 +54,48 @@ HTTP_TIMEOUT = 120.0 -def _skip(reason: str) -> int: - print(f"SKIP: {reason}") - return 0 +_CERTIFIES = "the plaintext HTTP path over emulated RR-Net" + + +def _cannot_run(reason: str, *, opt_out: bool = True) -> int: + """An involuntary skip is a FAILURE -- nothing was verified (issue #178). + + `opt_out=False` for a broken build: a failed `make` is never laundered + into a pass, not even by C64_ALLOW_SKIP. + """ + return cannot_run( + reason, + executed=0, + total=1, + certifies=_CERTIFIES, + opt_out_env="C64_ALLOW_SKIP" if opt_out else None, + ) + + +_COUNTERPART = ( + "tests/rig_vice_https_macos.py is the macOS counterpart for the emulated-RR-Net path; note it drives it over TLS, so plaintext HTTP specifically has no macOS rig" +) + + +def _wrong_platform() -> int: + """A VOLUNTARY skip: this host can never run this rig (issue #178). + + The load-bearing question is what the remedy is. "install iproute2" is + an involuntary skip and stays exit 2 -- but on a non-Linux host there is + no remedy at all, so exit 2 would be a red nobody can ever clear. + + Note what this rig does NOT get to say, and what its three siblings do: + that another rig owns the coverage. For DHCP and for HTTPS the macOS + rig genuinely re-runs the same path; for PLAINTEXT HTTP over emulated + RR-Net it does not, so this verdict really does lose coverage on macOS. + That is disclosed here and in _COUNTERPART rather than smoothed over -- + a voluntary skip may be quiet, but it may not misdescribe what it costs. + """ + return not_applicable( + f"this rig is Linux-only (br-c64 bridge + netfilter + /proc/net/udp); " + f"this host is {sys.platform} -- {_COUNTERPART}", + certifies=_CERTIFIES, + ) def _ensure_built() -> bool: @@ -97,6 +150,7 @@ def main() -> int: from https_e2e import ( BridgeEnv, check_prerequisites, + platform_supported, launch_vice_on_bridge, shutdown_vice, press_key, @@ -106,12 +160,20 @@ def main() -> int: stop_http_listener, ) + # Platform FIRST: check_prerequisites() cannot answer this, because the + # same string ("ip not on PATH") means "installable" on Linux and "wrong + # OS" everywhere else (issue #178). + if not platform_supported(): + return _wrong_platform() + missing = check_prerequisites() if missing: - return _skip("missing prerequisites: " + "; ".join(missing)) + return _cannot_run("missing prerequisites: " + "; ".join(missing)) if not _ensure_built(): - return _skip("c64-https.prg could not be built") + return _cannot_run("c64-https.prg could not be built -- `make` " + "failed; this is a broken build, not a " + "missing prerequisite", opt_out=False) handle = None listener = None diff --git a/tests/rig_phase3_https.py b/tests/rig_phase3_https.py index a404689..abdfc5f 100644 --- a/tests/rig_phase3_https.py +++ b/tests/rig_phase3_https.py @@ -14,10 +14,16 @@ Run: sudo PYTHONPATH=tools python3 tests/rig_phase3_https.py -Exit codes: +Exit codes (tools/_skip_policy.py, issue #178): 0 -- PASS - 0 -- SKIP (clearly printed) - 1 -- FAIL + 1 -- FAIL (a check ran and failed) + 0 -- NOT APPLICABLE: this rig is Linux-only, and on any other host + tests/rig_vice_https_macos.py owns the coverage. A named verdict, + never a bare skip. + 2 -- COULD NOT RUN (a prerequisite is missing ON LINUX, or the build is + broken -- nothing was verified). Set C64_ALLOW_SKIP=1 to accept a + prerequisite-missing run as exit 0; a FAILED BUILD is never opted + out of. """ from __future__ import annotations @@ -32,6 +38,9 @@ if _TOOLS not in sys.path: sys.path.insert(0, _TOOLS) +# needs _TOOLS on sys.path, hence the placement below the block above +from _skip_policy import cannot_run, not_applicable # noqa: E402 + PRG_PATH = os.path.join(_REPO_ROOT, "build", "c64-https.prg") # Screen needles (from src/boot.asm string labels). @@ -82,9 +91,42 @@ HTTPS_TIMEOUT = 1800.0 -def _skip(reason: str) -> int: - print(f"SKIP: {reason}") - return 0 +_CERTIFIES = "the TLS 1.3 handshake + GET over emulated RR-Net" + + +def _cannot_run(reason: str, *, opt_out: bool = True) -> int: + """An involuntary skip is a FAILURE -- nothing was verified (issue #178). + + `opt_out=False` for a broken build: a failed `make` is never laundered + into a pass, not even by C64_ALLOW_SKIP. + """ + return cannot_run( + reason, + executed=0, + total=1, + certifies=_CERTIFIES, + opt_out_env="C64_ALLOW_SKIP" if opt_out else None, + ) + + +_COUNTERPART = ( + "tests/rig_vice_https_macos.py owns this coverage on macOS" +) + + +def _wrong_platform() -> int: + """A VOLUNTARY skip: this host can never run this rig (issue #178). + + The load-bearing question is what the remedy is. "install iproute2" is + an involuntary skip and stays exit 2 -- but on a non-Linux host there is + no remedy at all, and another rig owns the coverage, so nothing is lost + and exit 2 would be a red nobody can ever clear. + """ + return not_applicable( + f"this rig is Linux-only (br-c64 bridge + netfilter + /proc/net/udp); " + f"this host is {sys.platform} -- {_COUNTERPART}", + certifies=_CERTIFIES, + ) def _ensure_built() -> bool: @@ -458,6 +500,7 @@ def main() -> int: from https_e2e import ( BridgeEnv, check_prerequisites, + platform_supported, launch_vice_on_bridge, shutdown_vice, press_key, @@ -467,12 +510,20 @@ def main() -> int: stop_https_listener, ) + # Platform FIRST: check_prerequisites() cannot answer this, because the + # same string ("ip not on PATH") means "installable" on Linux and "wrong + # OS" everywhere else (issue #178). + if not platform_supported(): + return _wrong_platform() + missing = check_prerequisites() if missing: - return _skip("missing prerequisites: " + "; ".join(missing)) + return _cannot_run("missing prerequisites: " + "; ".join(missing)) if not _ensure_built(): - return _skip("c64-https.prg could not be built") + return _cannot_run("c64-https.prg could not be built -- `make` " + "failed; this is a broken build, not a " + "missing prerequisite", opt_out=False) handle = None listener = None diff --git a/tests/rig_phase3_https_1mhz.py b/tests/rig_phase3_https_1mhz.py index bbaa47e..beb7318 100644 --- a/tests/rig_phase3_https_1mhz.py +++ b/tests/rig_phase3_https_1mhz.py @@ -20,11 +20,18 @@ Run (after the UCI test has fully stopped): sudo env VICE_HTTPS_OK_TO_RUN=1 PYTHONPATH=tools python3 tests/rig_phase3_https_1mhz.py -Exit codes: +Exit codes (tools/_skip_policy.py, issue #178): 0 -- PASS - 0 -- SKIP (clearly printed) - 1 -- FAIL - 2 -- pre-flight gate refused (U64E test probably still running) + 0 -- NOT APPLICABLE, as a named verdict rather than a bare skip: this + rig is Linux-only, and on any other host + tests/rig_vice_https_macos.py owns the coverage. + 1 -- FAIL (a check ran and failed) + 2 -- COULD NOT RUN (a prerequisite is missing ON LINUX, the build is + broken, or the rig is interlocked -- VICE_HTTPS_OK_TO_RUN unset, + or port 443 held by another run. Nothing was verified). Set + C64_ALLOW_SKIP=1 to accept a prerequisite-missing run as exit 0; + a FAILED BUILD and CONTENTION are never opted out of, and the + pre-flight gate is contention. """ from __future__ import annotations @@ -40,6 +47,9 @@ if _TOOLS not in sys.path: sys.path.insert(0, _TOOLS) +# needs _TOOLS on sys.path, hence the placement below the block above +from _skip_policy import cannot_run, not_applicable # noqa: E402 + PRG_PATH = os.path.join(_REPO_ROOT, "build", "c64-https.prg") # Screen needles (from src/boot.asm string labels). @@ -98,9 +108,42 @@ SCREEN_SNAP_KEEP = 30 -def _skip(reason: str) -> int: - print(f"SKIP: {reason}") - return 0 +_CERTIFIES = "the TLS 1.3 handshake + GET at honest 1 MHz over emulated RR-Net" + + +def _cannot_run(reason: str, *, opt_out: bool = True) -> int: + """An involuntary skip is a FAILURE -- nothing was verified (issue #178). + + `opt_out=False` for a broken build: a failed `make` is never laundered + into a pass, not even by C64_ALLOW_SKIP. + """ + return cannot_run( + reason, + executed=0, + total=1, + certifies=_CERTIFIES, + opt_out_env="C64_ALLOW_SKIP" if opt_out else None, + ) + + +_COUNTERPART = ( + "tests/rig_vice_https_macos.py owns this coverage on macOS (E2E_NO_WARP=1 for the 1 MHz variant)" +) + + +def _wrong_platform() -> int: + """A VOLUNTARY skip: this host can never run this rig (issue #178). + + The load-bearing question is what the remedy is. "install iproute2" is + an involuntary skip and stays exit 2 -- but on a non-Linux host there is + no remedy at all, and another rig owns the coverage, so nothing is lost + and exit 2 would be a red nobody can ever clear. + """ + return not_applicable( + f"this rig is Linux-only (br-c64 bridge + netfilter + /proc/net/udp); " + f"this host is {sys.platform} -- {_COUNTERPART}", + certifies=_CERTIFIES, + ) def _ensure_built() -> bool: @@ -540,48 +583,84 @@ def _emit(line: str) -> None: def main() -> int: + # This import is inert (module-level constants and defs only), so it can + # precede the pre-flight gate, whose real constraint is that it run + # before BridgeEnv mutates host netfilter via sudo. + from https_e2e import ( + BridgeEnv, + check_prerequisites, + platform_supported, + launch_vice_on_bridge, + shutdown_vice, + press_key, + wait_for_screen_text, + get_screen_text, + start_https_listener, + stop_https_listener, + ) + + # Platform FIRST: check_prerequisites() cannot answer this, because the + # same string ("ip not on PATH") means "installable" on Linux and "wrong + # OS" everywhere else (issue #178). Ahead of the gates below so that a + # host which can never run this rig is never told to opt in, and never + # trips the port check. + if not platform_supported(): + return _wrong_platform() + # --- Pre-flight gate (must run BEFORE BridgeEnv, which mutates host # netfilter via sudo). Refuses if the UCI HTTPS listener is still up. + # + # This flag is a CONTENTION INTERLOCK, not a taste setting, so an unset + # flag stays exit 2 exactly as it was before #178 -- the verdict is now + # a named block instead of a bare print, and that is the whole change. + # + # An earlier draft called it "the operator declining an opt-in rig" and + # routed it to not_applicable() (exit 0). That was wrong in this repo's + # own terms. UNSET IS THE DEFAULT STATE: it cannot distinguish "I + # considered this and declined" from "I forgot", so exit 0 hands a green + # run to someone who verified nothing -- the class #178 exists to close, + # committed by #178's own implementation. What the flag actually + # asserts is "the UCI HTTPS listener has fully stopped", which is a + # contention claim, and contention is the one category this policy gives + # no opt-out at all -- see the port-443 check immediately below, which is + # the same interlock measured directly rather than asserted. if os.environ.get("VICE_HTTPS_OK_TO_RUN") != "1": - print( - "ABORT: VICE_HTTPS_OK_TO_RUN is not set.\n" - " This test is gated to prevent collision with the UCI HTTPS\n" - " listener (which binds 443/4433 on the LAN interface). The\n" - " U64E test is likely still running. Wait for it to finish,\n" - " then re-run with:\n" + return _cannot_run( + "VICE_HTTPS_OK_TO_RUN is not set -- this rig is interlocked because\n" + " it collides with the UCI HTTPS listener (which binds 443/4433\n" + " on the LAN interface); the U64E test may still be running.\n" + " To run it, wait for that to finish, then:\n" " sudo env VICE_HTTPS_OK_TO_RUN=1 PYTHONPATH=tools \\\n" - " python3 tests/rig_phase3_https_1mhz.py" + " python3 tests/rig_phase3_https_1mhz.py", + opt_out=False, ) - return 2 + + # A held port is CONTENTION, the third category: the rig exists, someone + # else has it. Exit 2 like any could-not-run, but deliberately NOT + # opt-out-able -- a lane that silences contention goes green exactly when + # it collides, and unlike a missing tool it clears on its own. Same + # decision as tests/rig_vice_https_macos.py's feth0 check. for port in (443,): s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) try: s.bind(("0.0.0.0", port)) except OSError as e: - # something else is holding it — refuse - print(f"refusing: port {port} is in use (UCI test?): {e}") - return 2 + return _cannot_run( + f"port {port} is held by another process (the UCI test?): {e}" + f"\n fix: wait for the other run to finish", + opt_out=False, + ) finally: s.close() - from https_e2e import ( - BridgeEnv, - check_prerequisites, - launch_vice_on_bridge, - shutdown_vice, - press_key, - wait_for_screen_text, - get_screen_text, - start_https_listener, - stop_https_listener, - ) - missing = check_prerequisites() if missing: - return _skip("missing prerequisites: " + "; ".join(missing)) + return _cannot_run("missing prerequisites: " + "; ".join(missing)) if not _ensure_built(): - return _skip("c64-https.prg could not be built") + return _cannot_run("c64-https.prg could not be built -- `make` " + "failed; this is a broken build, not a " + "missing prerequisite", opt_out=False) handle = None listener = None diff --git a/tests/rig_vice_https_macos.py b/tests/rig_vice_https_macos.py index 52e547f..660853c 100644 --- a/tests/rig_vice_https_macos.py +++ b/tests/rig_vice_https_macos.py @@ -34,7 +34,13 @@ and the C64 dies at TCP CONNECT with nothing on the wire. The listener self-probe below detects this and says so. -Exit codes: 0 PASS / 0 SKIP (printed) / 1 FAIL. +Exit codes (tools/_skip_policy.py, issue #178): + 0 PASS, or NOT APPLICABLE on a non-macOS host (tests/rig_phase3_https.py + owns that coverage on Linux) -- a named verdict, never a bare skip. + 1 FAIL (a check ran and failed) + 2 COULD NOT RUN -- the rig is not up, or it is contended, so nothing was + verified. C64_ALLOW_SKIP=1 accepts a rig-NOT-READY run as exit 0; it + does NOT cover contention, and does not cover a failed build. """ from __future__ import annotations @@ -51,6 +57,10 @@ if p not in sys.path: sys.path.insert(0, p) +# needs _TOOLS on sys.path, hence the placement below the block above +from _skip_policy import cannot_run, not_applicable # noqa: E402 + +_CERTIFIES = "the TLS 1.3 handshake + GET over emulated RR-Net on macOS" from c64_test_harness.backends.vice_binary import BinaryViceTransport # noqa: E402 from c64_test_harness import Labels, read_bytes # noqa: E402 from https_e2e import ( # noqa: E402 @@ -88,12 +98,39 @@ ) -def _rig_check() -> list[str]: - """Return a list of missing-prerequisite messages (empty = rig OK).""" - problems = [] - if sys.platform != "darwin": - problems.append("not macOS (use tests/rig_phase3_https.py on Linux)") - return problems +def _platform_supported() -> bool: + """True if this host can run the feth/pcap rig at all. + + Module-level and parameterless on purpose: a test can replace THIS to + exercise both branches, instead of patching `sys.platform` globally. + """ + return sys.platform == "darwin" + + +def _rig_check() -> "tuple[list[str], list[str]]": + """Return (problems, contention) for a macOS host. Both empty = rig OK. + + Two lists because the remedies differ in kind, and issue #178's rule is + that the remedy decides the verdict: + + problems -- something is not installed or not set up. Remedy: run + `sudo bash tools/rig-up-macos.sh`, build the VICE. An + involuntary skip, exit 2, opt-out-able by a lane that + knowingly has no rig. + contention -- the rig exists but another process holds it. Remedy: + wait, or kill YOUR stale instance. Also exit 2, but + deliberately NOT opt-out-able: a lane that silences + contention goes green every time it collides, which is + the exact vacuous-pass this policy exists to stop, and + unlike a missing tool it is transient, so silencing it + hides a condition that would have cleared on its own. + + The platform question is asked by the CALLER, before this runs -- a + non-Darwin host is a voluntary skip owned by tests/rig_phase3_https.py, + not a problem with this rig. + """ + problems: list[str] = [] + contention: list[str] = [] if not os.path.exists(VICE_BIN): problems.append( f"{VICE_BIN} missing — build it per c64-test-harness#144 " @@ -113,13 +150,31 @@ def _rig_check() -> list[str]: # eats/garbles ARP and TCP meant for this run. Other x64sc processes # NOT on feth0 (other projects' fleets on this shared bench) are # fine — never kill those. - r = subprocess.run(["pgrep", "-fl", "ethernetioif feth0"], - capture_output=True, text=True) - if r.stdout.strip(): - problems.append( + # Match the BINARY first, then that process's argv. `pgrep -fl + # "ethernetioif feth0"` matched the full command line of EVERY process + # against an unanchored pattern, so anything merely containing that text + # matched too: a grep for it, an editor opened on this file, a driver + # script passing it as an argument. A false positive is unrecoverable + # here, because contention is deliberately not opt-out-able -- so the + # detector has to be narrow. `pgrep -x x64sc` matches only processes + # whose executable name is exactly x64sc; the argv check then confirms it + # is the one on feth0. + attached: list[str] = [] + r = subprocess.run(["pgrep", "-x", "x64sc"], capture_output=True, text=True) + for pid in r.stdout.split(): + if not pid.isdigit(): + continue + ps = subprocess.run(["ps", "-o", "command=", "-p", pid], + capture_output=True, text=True) + cmd = " ".join(ps.stdout.split()) + if "ethernetioif" in cmd and "feth0" in cmd: + attached.append(f"pid {pid}: {cmd}") + if attached: + contention.append( "another VICE is already attached to feth0 (duplicate-MAC " - f"conflict):\n {r.stdout.strip()}\n kill YOUR stale " - "instance (do not touch other projects' x64sc processes)") + "conflict):\n " + "\n ".join(attached) + + "\n kill YOUR stale instance (do not touch other projects' " + "x64sc processes)") try: pid = int(open("/tmp/c64-rig-dnsmasq.pid").read().strip()) os.kill(pid, 0) @@ -127,7 +182,7 @@ def _rig_check() -> list[str]: pass # EPERM = process exists (it's root-owned) — rig is up except (OSError, ValueError): problems.append("rig dnsmasq not running (pid file stale/absent)") - return problems + return problems, contention def _build_prg() -> None: @@ -141,7 +196,18 @@ def _build_prg() -> None: if r.returncode != 0: print(r.stdout[-2000:]) print(r.stderr[-2000:]) - raise SystemExit("build failed") + # A failed build is exit 2 (could not run), not exit 1 (a check ran + # and failed) -- the same verdict the other four rigs give, and never + # opted out of. SystemExit("build failed") used to exit 1 here, which + # made a broken build indistinguishable from a real handshake failure. + raise SystemExit(cannot_run( + "the PRG could not be built -- `make` failed; this is a broken " + "build, not a missing prerequisite", + executed=0, + total=1, + certifies=_CERTIFIES, + opt_out_env=None, + )) def _launch_vice() -> subprocess.Popen: @@ -223,13 +289,51 @@ def _sigterm(_sig, _frame): signal.signal(signal.SIGTERM, _sigterm) - problems = _rig_check() + # Platform FIRST, and it is a VOLUNTARY skip: a Linux host can never run + # the feth/pcap rig, and tests/rig_phase3_https.py owns that coverage + # there. Exit 2 would be a red nobody on that platform could clear + # (issue #178). + if not _platform_supported(): + return not_applicable( + f"this rig is macOS-only (feth pair + /dev/bpf pcap); this host " + f"is {sys.platform} -- tests/rig_phase3_https.py owns this " + f"coverage on Linux", + certifies=_CERTIFIES, + ) + + problems, contention = _rig_check() + + # PROBLEMS FIRST. Contention on a rig that does not exist is a + # meaningless statement: with no VICE binary, no feth1 and no dnsmasq, + # "another VICE holds feth0" is the wrong headline and demotes the real + # cause to a parenthetical. Worse, contention is deliberately not + # opt-out-able, so evaluating it first left a CI lane that legitimately + # has no rig -- and correctly set C64_ALLOW_SKIP=1 -- red with no + # recourse. An involuntary skip is a FAILURE, but this one is the kind + # an operator can opt out of (issue #178). if problems: - print("SKIP: rig not ready:") - for p in problems: - print(f" - {p}") - print(" fix: (re)run `sudo bash tools/rig-up-macos.sh`") - return 0 + return cannot_run( + "rig not ready:\n - " + "\n - ".join(problems) + + "\n fix: (re)run `sudo bash tools/rig-up-macos.sh`", + executed=0, + total=1, + certifies=_CERTIFIES, + opt_out_env="C64_ALLOW_SKIP", + ) + + # Only once the rig is otherwise complete does contention mean anything. + # Its own category: the rig is here, someone else has it. Exit 2 like any + # could-not-run, but with NO opt-out -- see _rig_check(). + if contention: + return cannot_run( + "rig is contended:\n - " + "\n - ".join(contention) + + "\n fix: wait for the other run, or kill YOUR stale " + "instance", + executed=0, + total=1, + certifies=_CERTIFIES, + opt_out_env=None, + ) if os.environ.get("C64_SKIP_BUILD") != "1": _build_prg() diff --git a/tools/_skip_policy.py b/tools/_skip_policy.py new file mode 100644 index 0000000..147af92 --- /dev/null +++ b/tools/_skip_policy.py @@ -0,0 +1,323 @@ +#!/usr/bin/env python3 +"""_skip_policy.py -- the project's involuntary-skip rule, in one import. + + An involuntary skip is a failure; a voluntary skip is allowed but must + never be silent. + +This repo has closed that class three times (#158 audit commit `7497e48`, +re-found as #165, fixed again in PR #172) because the correct shape was a +convention rather than a callable. This module is the callable. + +Two lanes, and the whole point is that they are different calls: + + * ``cannot_run()`` -- INVOLUNTARY. A prerequisite the caller asked for is + missing: no toolchain, no PRG, a failed ``make``, no hardware, no + fixture. Nothing was verified, so the run must not read as success. + Returns ``EXIT_CANNOT_RUN`` (2) -- deliberately distinct from 1 ("a + check ran and failed") so a caller can tell "broken" from "unproven". + + * ``not_applicable()`` -- VOLUNTARY. The caller chose a configuration + this suite does not cover (the other backend, a diagnostic mode). There + is genuinely nothing to verify, so exit 0 is honest -- but it is printed + as a named verdict with its scope spelled out, never as a bare line that + a reader mistakes for a pass. + +Exit-code contract for every caller wired to this module: + + 0 PASS, or a declared NOT APPLICABLE, or an acknowledged opt-out + 1 a check ran and FAILED + 2 COULD NOT RUN -- prerequisites missing, nothing was verified + +The opt-out +----------- +``cannot_run(..., opt_out_env="C64_ALLOW_SKIP")`` returns 0 when that +variable is set to exactly ``"1"`` -- not merely set, so ``=0`` and +``=false`` do NOT open the hatch. It still prints the full block: the +opt-out suppresses the exit code, never the warning. It exists so a CI lane +that legitimately has no hardware can stay green *by saying so in its own +configuration*, which is a decision someone made on purpose and can be +grepped for -- unlike a bare ``return 0`` buried in a prerequisite branch. + +The pytest channel +------------------ +Under pytest the skip *reason* is the only channel that survives: ``-ra`` +(pinned in ``pytest.ini`` ``addopts``) prints that string and nothing else, +and module stdout is swallowed. So ``reason_text()`` folds the vacuity +warning INTO the reason string rather than printing it alongside, and +``require()`` hands that same string to the failure/skip it raises. Without +that, an exit-0 opt-out reads as a bare ``3 skipped`` and the warning is +gone. + +``require()`` decides that by asking whether pytest is DRIVING +(``sys.modules``), never whether it is importable: outside pytest it raises +``VoluntarySkip``, which a standalone runner can actually catch. + +Usage (script lane):: + + from _skip_policy import cannot_run, not_applicable + + if missing: + return cannot_run( + "missing prerequisites: " + "; ".join(missing), + executed=0, total=TOTAL_CHECKS, + certifies="the ip65 DHCP path in VICE", + opt_out_env="C64_ALLOW_SKIP", + ) + + if backend != "uci": + return not_applicable( + f"dual-overlay smoke test is UCI-only (backend={backend})", + certifies="the UCI dual-overlay swap dispatcher", + ) + +Usage (pytest lane):: + + from _skip_policy import require + + def test_something(): + require(shutil.which("ca65") is not None, + "ca65 not on PATH", + executed=0, total=1, + certifies="the build-flag stamp", + opt_out_env="C64_ALLOW_SKIP") + ... +""" + +from __future__ import annotations + +import os +import sys +from typing import Optional, TextIO + +__all__ = [ + "EXIT_PASS", + "EXIT_FAIL", + "EXIT_CANNOT_RUN", + "SkipPolicyError", + "VoluntarySkip", + "reason_text", + "cannot_run", + "not_applicable", + "require", +] + +EXIT_PASS = 0 +EXIT_FAIL = 1 +EXIT_CANNOT_RUN = 2 + + +class SkipPolicyError(AssertionError): + """Raised by require() when an involuntary skip must be a failure. + + Subclasses AssertionError so pytest renders it as a plain failure and so + a caller that only catches AssertionError still catches it. + """ + + +class VoluntarySkip(Exception): + """Raised by require() when the opt-out is honoured and pytest is NOT driving. + + Deliberately an ``Exception`` and not a ``pytest.outcomes.Skipped``. + ``Skipped`` derives from ``BaseException``, so a standalone runner's + ``except Exception`` cannot catch it: the process dies mid-suite, prints + no summary, runs none of the remaining tests, and exits 1 -- which under + this module's own contract means "a check ran and failed". Raising a + plain Exception instead keeps the escape hatch catchable by the lane that + is actually running. + + A caller that does not catch it still does not run the test body, so the + fail-closed property is unchanged; only the reporting differs. + + CONSUMERS: being an ``Exception`` makes this SWALLOWABLE BY DESIGN, so a + broad ``except Exception`` will catch it and label it whatever that + handler labels things. Name it explicitly, BEFORE the bare + ``except Exception``, as both runners in this repo do. The direction of + the hazard is the safe one -- a swallowed voluntary skip reports as a + FAILURE, never as a pass -- which is why it is worth one line of warning + rather than a BaseException, the choice that caused the bug this class + exists to fix. + """ + + +def _opted_out(opt_out_env: Optional[str]) -> bool: + """True only if `opt_out_env` names a variable whose value is exactly "1". + + Bare truthiness would make ``C64_ALLOW_SKIP=0`` and + ``C64_ALLOW_SKIP=false`` ENABLE the opt-out -- someone setting 0 to shut + the escape hatch would silently disable the whole policy. Every other + gate in this repo (``C64_SKIP_BUILD``, ``C64_SKIP_TEMP_GC``, + ``C64_NET_TESTS_OPTIONAL``, ``VICE_HTTPS_OK_TO_RUN``) compares to the + literal "1"; this one does too. Surrounding whitespace is tolerated + because a shell export can carry it; nothing else is. + """ + if not opt_out_env: + return False + return os.environ.get(opt_out_env, "").strip() == "1" + + +def _coverage_clause(executed: Optional[int], total: Optional[int]) -> str: + if executed is None or total is None: + return "no checks executed" + return f"{executed} of {total} checks executed" + + +def reason_text( + reason: str, + *, + executed: Optional[int] = 0, + total: Optional[int] = None, + certifies: Optional[str] = None, + opt_out_env: Optional[str] = None, +) -> str: + """Build the one-string reason that carries its own vacuity warning. + + This is the string that must reach pytest's ``-ra`` line, where it is the + only surviving channel. It always states the coverage, and it always + states what the run therefore certifies nothing about. + """ + parts = [f"COULD NOT RUN: {reason}", _coverage_clause(executed, total)] + subject = certifies or "the behaviour under test" + parts.append(f"this run certifies NOTHING about {subject}") + if opt_out_env: + parts.append(f"set {opt_out_env}=1 to accept an unverified run") + return " -- ".join(parts) + + +def _print_block( + heading: str, + reason: str, + lines: "list[str]", + out: Optional[TextIO] = None, +) -> None: + stream = out if out is not None else sys.stdout + bar = "=" * 60 + print(bar, file=stream) + print(f"{heading}: {reason}", file=stream) + for line in lines: + print(f" {line}", file=stream) + print(bar, file=stream) + try: + stream.flush() + except Exception: # noqa: BLE001 - a closed/odd stream must not mask the verdict + pass + + +def cannot_run( + reason: str, + *, + executed: Optional[int] = 0, + total: Optional[int] = None, + certifies: Optional[str] = None, + opt_out_env: Optional[str] = None, + out: Optional[TextIO] = None, +) -> int: + """An INVOLUNTARY skip. Print the standard block, return 2 (or 0 if opted out). + + Returns ``EXIT_CANNOT_RUN`` so the caller can ``return cannot_run(...)`` + straight out of ``main()``. Returns ``EXIT_PASS`` only when + ``opt_out_env`` names a variable set to exactly "1" -- and even then the + block is printed in full. + """ + subject = certifies or "the behaviour under test" + opted_out = _opted_out(opt_out_env) + lines = [ + _coverage_clause(executed, total), + f"this run certifies NOTHING about {subject}", + ] + if opted_out: + lines.append( + f"{opt_out_env}=1 is set -- exiting 0 by explicit opt-out, " + "NOT because anything passed" + ) + _print_block("COULD NOT RUN (opt-out honoured)", reason, lines, out) + return EXIT_PASS + if opt_out_env: + lines.append( + f"set {opt_out_env}=1 to accept an unverified run (exit 0 instead of " + f"{EXIT_CANNOT_RUN})" + ) + lines.append(f"exit {EXIT_CANNOT_RUN} = could not run (1 would mean a check failed)") + _print_block("COULD NOT RUN", reason, lines, out) + return EXIT_CANNOT_RUN + + +def not_applicable( + reason: str, + *, + certifies: Optional[str] = None, + out: Optional[TextIO] = None, +) -> int: + """A VOLUNTARY skip. Print a named verdict, return 0. + + Use this ONLY when the caller's own configuration puts the subject out of + scope -- the other backend, a diagnostic mode -- so that there is nothing + to verify and exit 0 is the honest answer. If a prerequisite is missing, + that is ``cannot_run()``, not this. + """ + subject = certifies or "the behaviour under test" + lines = [ + "0 of 0 checks executed -- there is nothing here to verify in this " + "configuration", + f"this run certifies NOTHING about {subject}", + "exit 0 = not applicable (a prerequisite that is merely MISSING is " + f"exit {EXIT_CANNOT_RUN}, not this)", + ] + _print_block("NOT APPLICABLE", reason, lines, out) + return EXIT_PASS + + +def require( + condition: object, + reason: str, + *, + executed: Optional[int] = 0, + total: Optional[int] = None, + certifies: Optional[str] = None, + opt_out_env: Optional[str] = None, +) -> None: + """pytest-side ``cannot_run``: raise unless the prerequisite holds. + + On a false ``condition`` this raises :class:`SkipPolicyError` carrying the + full ``reason_text()`` -- pytest records a FAILURE, and the reason string + is self-contained because module stdout does not survive. + + If ``opt_out_env`` is set to exactly "1" the escape hatch opens, and how + it is reported depends on who is DRIVING, not on what is installed: + + * pytest driving -- ``pytest.skip()`` with the same self-contained + string, so the ``-ra`` summary still carries the vacuity warning + rather than a bare "skipped". + * anything else -- :class:`VoluntarySkip`, which a plain + ``except Exception`` can catch. See that class for why handing a + ``Skipped`` to a non-pytest runner is a bug, not a shortcut. + + Either way the test body does not run. + """ + if condition: + return + text = reason_text( + reason, + executed=executed, + total=total, + certifies=certifies, + opt_out_env=opt_out_env, + ) + if _opted_out(opt_out_env): + # Hand the skip to pytest ONLY when pytest is actually DRIVING this + # run -- `sys.modules`, not `import`. `import pytest` succeeds + # whenever pytest is merely INSTALLED, which on a developer machine + # is always, and `pytest.skip()` raises `pytest.outcomes.Skipped`, + # a BaseException. A standalone runner's `except Exception` cannot + # catch that, so the whole suite died mid-run at exit 1 with no + # summary and the remaining tests never executed. + # + # tools/test_uci_data_acc.py's hand-rolled copy of this policy + # already had this right, with the same `sys.modules.get("pytest")` + # and the same reason: "the standalone runner has its own reporting + # and must not see Skipped". This helper exists to supersede that + # copy, so it has to be at least as correct as the thing it replaces. + pytest = sys.modules.get("pytest") + if pytest is None: + raise VoluntarySkip(f"{text} [{opt_out_env}=1 set]") + pytest.skip(f"{text} [{opt_out_env}=1 set]") + raise SkipPolicyError(text) diff --git a/tools/https_e2e/__init__.py b/tools/https_e2e/__init__.py index 59f9a8c..27ab880 100644 --- a/tools/https_e2e/__init__.py +++ b/tools/https_e2e/__init__.py @@ -7,12 +7,13 @@ launch_vice_on_bridge, shutdown_vice, press_key, wait_for_screen_text, check_prerequisites, + platform_supported, ) Internals live in underscored helpers in each submodule. """ -from .env import BridgeEnv, check_prerequisites +from .env import BridgeEnv, check_prerequisites, platform_supported from .vice_on_bridge import launch_vice_on_bridge, shutdown_vice from .c64_menu import press_key, wait_for_screen_text, get_screen_text from .http_listener import start_http_listener, stop_http_listener @@ -21,6 +22,7 @@ __all__ = [ "BridgeEnv", "check_prerequisites", + "platform_supported", "launch_vice_on_bridge", "shutdown_vice", "press_key", diff --git a/tools/https_e2e/env.py b/tools/https_e2e/env.py index f94aa79..aa28da2 100644 --- a/tools/https_e2e/env.py +++ b/tools/https_e2e/env.py @@ -12,6 +12,7 @@ import shutil import socket import subprocess +import sys import time from contextlib import contextmanager @@ -25,8 +26,32 @@ TAP1 = "tap-c64-1" +def platform_supported() -> bool: + """True if this host can run the br-c64 bridge rig at all. + + Asked BEFORE check_prerequisites(), and deliberately NOT folded into + it: that function returns a flat list of strings, and "ip not on PATH" + means "apt install iproute2" on Linux but "this OS has no iproute2 and + never will" on Darwin. No partition of the list can tell those apart, + because the platform question has to be answered first. + + The rig needs Linux netfilter (the setup script's iptables rules), the + iproute2 `ip` command, and /proc/net/udp -- which _port_open_udp() + below reads directly. A wrong platform is a VOLUNTARY skip owned by + tests/rig_vice_https_macos.py; a missing tool ON Linux is installable + and stays an involuntary one. See issue #178. + """ + return sys.platform.startswith("linux") + + def check_prerequisites() -> list[str]: - """Return a list of missing prereqs. Empty list means all OK.""" + """Return a list of missing prereqs. Empty list means all OK. + + Contract unchanged (issue #178 added platform_supported() beside it + rather than partitioning this list): the five tool checks and the sudo + probe below all describe things that are INSTALLABLE on the rig's + target platform. Callers ask platform_supported() first. + """ missing: list[str] = [] for tool in ("x64sc", "dnsmasq", "sudo", "ip", "iptables"): if shutil.which(tool) is None: diff --git a/tools/test_ecdsa_p384_kat.py b/tools/test_ecdsa_p384_kat.py index 413c1fd..ce8b3c2 100644 --- a/tools/test_ecdsa_p384_kat.py +++ b/tools/test_ecdsa_p384_kat.py @@ -48,8 +48,20 @@ verdict, so it always exits non-zero and can never report OVERALL: PASS — read the per-step output, not the tally. +Exit codes (tools/_skip_policy.py, issue #178): + 0 PASS + 1 FAIL (a vector ran and failed) + 2 COULD NOT RUN -- --u64 was requested with no U64_HOST, so nothing ran. + NOT opt-out-able: honouring C64_ALLOW_SKIP here would fire before the + VICE lane and silence the emulator half too, which needs no hardware. + The remedy is free: set U64_HOST, or drop --u64 and get the VICE lane + on its own. Not passing --u64 at all is a VOLUNTARY skip, exits 0, + and is how you say "this lane has no hardware". + Environment: C64_SKIP_BUILD=1 Reuse existing build artifacts (skip make). + C64_ALLOW_SKIP=1 Honoured by the rig lane; NOT by the --u64 + gate above (see its comment in main()). U64_HOST= Ultimate 64 host (default 192.168.1.81). P384_KAT_VICE_TIMEOUT_S Per-VERIFY-step timeout under VICE (default 1800 s = 30 min). @@ -78,6 +90,12 @@ import time from pathlib import Path +_HERE = os.path.dirname(os.path.abspath(__file__)) +if _HERE not in sys.path: + sys.path.insert(0, _HERE) + +from _skip_policy import cannot_run # noqa: E402 + PROJECT_ROOT = Path(__file__).resolve().parents[1] PRG_PATH = PROJECT_ROOT / "build" / "c64-https.prg" LABELS_PATH = PROJECT_ROOT / "build" / "labels.txt" @@ -788,6 +806,36 @@ def main() -> int: print(f"=== test_ecdsa_p384_kat.py (P-384 dual-overlay KAT) ===") os.chdir(str(PROJECT_ROOT)) + # --u64 is the caller asking for hardware. Without U64_HOST the hardware + # lane cannot run at all, and the tally below would otherwise add 0/0 to + # the VICE result and print OVERALL: PASS (issue #178). Refuse up front, + # before the multi-hour VICE lane, rather than at the verdict. + # + # NO OPT-OUT, and this gate is the reason the rule needs stating: the + # opt-out here would be scoped WRONG. C64_ALLOW_SKIP answers "this lane + # has no hardware", but firing before the VICE lane means honouring it + # also silences the EMULATOR half -- which needs no hardware and could + # have run. Measured against master: `--u64` with U64_HOST unset ran the + # whole VICE lane and returned 1 on a failing vector; with the opt-out + # honoured here that became exit 0, so an operator who set + # C64_ALLOW_SKIP=1 for the honest reason (no U64E on this lane) would + # never learn the emulator-only half had regressed. + # + # The remedy costs nothing and needs no environment variable: set + # U64_HOST, or drop --u64 and get the VICE lane on its own. "This lane + # has no hardware" is spelled by NOT PASSING --u64; asking for hardware + # and not supplying it is a malformed invocation, not a coverage gap to + # be acknowledged. + if run_u64 and not os.environ.get("U64_HOST"): + return cannot_run( + "--u64 requested but U64_HOST is not set in the environment" + " -- drop --u64 to run the VICE lane alone", + executed=0, + total=1, + certifies="the P-384 verify path on real hardware", + opt_out_env=None, + ) + # Check the upstream test vector file exists. if not NIST_VECTORS_PATH.exists(): print(f"FATAL: vector file not found: {NIST_VECTORS_PATH}") @@ -838,7 +886,21 @@ def main() -> int: if run_u64: print(f"\n=== U64 backend (real hardware) ===") if not os.environ.get("U64_HOST"): - print(" SKIP: --u64 requested but U64_HOST not set in env") + # Unreachable: the early gate in main() already refused. Kept as + # a backstop, and it must NOT leave a 0/0 lane -- that is exactly + # what the tally used to read as OVERALL: PASS (issue #178). + # Same no-opt-out rule as that gate, and for the same reason: + # one file must not answer the same question two ways, or a + # later reader "fixes" the inconsistency in whichever direction + # they happen to notice first. + return cannot_run( + "--u64 requested but U64_HOST is not set in the environment" + " -- drop --u64 to run the VICE lane alone", + executed=v_pass + v_fail, + total=v_pass + v_fail + len(vectors), + certifies="the P-384 verify path on real hardware", + opt_out_env=None, + ) else: try: u_pass, u_fail, u_details = _run_backend( @@ -861,6 +923,16 @@ def main() -> int: print(f"{'=' * 60}") total_fail = v_fail + u_fail + # NOTE: there is deliberately no `total_run == 0` vacuity guard here. + # One was written and removed: _build_vector_list() never returns an + # empty list (1 vector for the smoke default, 4 for --full) and + # _run_backend() puts every vector into `passed` or `failed`, including + # under --sha-only, whose short-circuit in _run_one_vector() returns an + # "error" dict and is therefore counted as a failure. So the guard could + # not fire under any flag combination. A check that matches nothing is + # the same vacuous-green shape issue #178 exists to close, so it does not + # belong in #178's own implementation. If a vector FILTER is ever added, + # add the guard back beside it -- where it can actually fire. overall = "PASS" if total_fail == 0 else "FAIL" print(f"OVERALL: {overall}") if sha_only: diff --git a/tools/test_p384_symbols.py b/tools/test_p384_symbols.py index e0bb038..8058726 100755 --- a/tools/test_p384_symbols.py +++ b/tools/test_p384_symbols.py @@ -44,7 +44,11 @@ only assert the state byte updates. 9. JSR crypto_swap_none. Confirm current_overlay == 0. -Exits 0 on PASS, 1 on FAIL or environmental error. +Exit codes (tools/_skip_policy.py, issue #178): + 0 PASS, or a declared NOT APPLICABLE (BACKEND != uci) + 1 FAIL (a check ran and failed) + 2 COULD NOT RUN -- a required build artifact is missing, so nothing was + verified. VICE harness gotcha: the PRG's boot path executes nistcurves P-256 fp_mul, which fetches 8x8 multiply rows from REU banks 0/1. Without @@ -57,8 +61,8 @@ BACKEND=uci /Users/someone/.local/share/c64-test-harness/venv/bin/python3 \ tools/test_p384_symbols.py [--verbose] -Under BACKEND=ip65 the script exits 0 with a skip message -- the -embedded-blobs path is UCI-only (ip65 has no main-RAM headroom for the +Under BACKEND=ip65 the script prints a NOT APPLICABLE verdict and exits 0 +(a voluntary skip, issue #178) -- the embedded-blobs path is UCI-only (ip65 has no main-RAM headroom for the extra 15 KB; see cfg/c64-https-ip65.cfg's Phase 3 comment block). """ @@ -66,6 +70,12 @@ import subprocess import sys +_HERE = os.path.dirname(os.path.abspath(__file__)) +if _HERE not in sys.path: + sys.path.insert(0, _HERE) + +from _skip_policy import cannot_run, not_applicable # noqa: E402 + PROJECT_ROOT = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..") PRG_PATH = os.path.join(PROJECT_ROOT, "build", "c64-https.prg") LABELS_PATH = os.path.join(PROJECT_ROOT, "build", "labels.txt") @@ -125,10 +135,16 @@ def main() -> int: # Phase 3 dual-overlay embed is UCI-only -- ip65 has no main-RAM # headroom for the extra 15 KB after the existing layout. Under # ip65 the OVERLAY_BLOB_* segments are empty and the boot DMA is - # a no-op, so there is nothing meaningful to test. Skip cleanly. + # a no-op, so there is nothing meaningful to test. if backend != "uci": - print(f" SKIP: dual-overlay smoke test is UCI-only (backend={backend})") - return 0 + # VOLUNTARY skip (issue #178): the caller chose this backend and the + # segments under test are empty in it, so there is nothing to verify + # and exit 0 is honest -- but it is announced as a named verdict, not + # a bare `return 0` that reads as a pass. + return not_applicable( + f"dual-overlay smoke test is UCI-only (backend={backend})", + certifies="the UCI dual-overlay swap dispatcher", + ) if os.environ.get("C64_SKIP_BUILD") != "1": subprocess.run(["make", "clean", f"BACKEND={backend}"], @@ -145,8 +161,12 @@ def main() -> int: # Sanity-check on-disk artifacts. for path in (PRG_PATH, LABELS_PATH, SHA_BIN_PATH, CURVE_BIN_PATH): if not os.path.exists(path): - print(f"FATAL: required artifact missing: {path}") - return 1 + return cannot_run( + f"required artifact missing: {path}", + executed=0, + total=7, + certifies="the UCI dual-overlay swap dispatcher", + ) sha_image = open(SHA_BIN_PATH, "rb").read() curve_image = open(CURVE_BIN_PATH, "rb").read() diff --git a/tools/test_rig_skip_contract.py b/tools/test_rig_skip_contract.py new file mode 100644 index 0000000..cf51c68 --- /dev/null +++ b/tools/test_rig_skip_contract.py @@ -0,0 +1,534 @@ +#!/usr/bin/env python3 +"""Pin the skip contract AT THE CALL SITES, not just in the helper (#178). + +``tools/_skip_policy.py`` has its own tests. This file tests the thing those +cannot: that the eleven wired call sites still *use* it, and still ask their +questions in the right order. + +The gap this closes was the sharpest point of the #178 review. The four +bridge rigs must ask ``platform_supported()`` BEFORE ``check_prerequisites()``: +on macOS the prerequisite list contains "ip not on PATH", which is not +installable there, so asking it first turns four Linux-only rigs into a +permanent red on this project's primary platform -- and the only remedy on +offer is the global opt-out, i.e. the policy gets switched off everywhere. +Swap those two lines back in a future edit and nothing else in the tree +notices. That is #178's own thesis -- "a helper on its own is just another +convention to miss" -- turned on #178's own implementation. + +Pure logic: no VICE, no hardware, no build, no network. Every rig entry point +called here is called with a tripwire in place of the next step, so a test +that fails an ordering assertion fails loudly rather than starting a `make`. + +Two deliberate limitations, stated rather than hidden: + + * ``tests/rig_vice_https_macos.py`` is NOT imported. It imports + ``c64_test_harness`` from a hard-coded sibling checkout at module level, + which is a separate repo (``pip install -e ../c64-test-harness``), so + importing it here would make this module error on any machine without + that checkout. Its two ordering contracts are pinned by source + inspection instead -- weaker evidence, and labelled as such. + * ``platform_supported()`` is a pure function of ``sys.platform``, so it is + tested by patching exactly that one input. That is the whole of its + behaviour; there is nothing else to observe. + * ``tools/https_e2e`` reaches the same sibling checkout transitively (its + ``__init__`` re-exports from ``.vice_on_bridge``), so it is imported + lazily inside the tests that need it, through ``require()``. Without the + checkout those six tests FAIL by name; the four source-inspection tests + below still run. A module-level import would instead have been a pytest + COLLECTION ERROR on a fresh clone -- and ``pytest.ini`` says in as many + words that a collection error must never be mistaken for a passing run. + +KNOWN GAP, follow-up owed +------------------------- +The four ``test_macos_rig_*`` cases assert on SOURCE TEXT, so behavioural +mutants of ``tests/rig_vice_https_macos.py`` survive them -- reordering its +gates at runtime, or changing a verdict without changing the anchored text, +would not be caught. Closing that needs the rig restructured to be +importable without the sibling harness, which is not a drive-by. So: the +four bridge rigs are pinned BEHAVIOURALLY here; the macOS rig is pinned by +SHAPE only. +""" + +import ast +import io +import os +import subprocess +import sys +from contextlib import redirect_stdout + +_HERE = os.path.dirname(os.path.abspath(__file__)) +_REPO = os.path.dirname(_HERE) +_TESTS = os.path.join(_REPO, "tests") +for _p in (_HERE, _TESTS): + if _p not in sys.path: + sys.path.insert(0, _p) + +from _skip_policy import ( # noqa: E402 + EXIT_CANNOT_RUN, + EXIT_FAIL, + EXIT_PASS, + SkipPolicyError, + VoluntarySkip, + require, +) + +# tools/https_e2e is NOT imported at module level. Its __init__ re-exports +# from .vice_on_bridge, which imports c64_test_harness from a sibling +# checkout (`pip install -e ../c64-test-harness`) -- a separate repo. This +# module is in pytest.ini testpaths, so a module-level import would make bare +# `pytest` on a fresh clone die with a COLLECTION ERROR, and pytest.ini's own +# comment says a collection error must never be mistaken for a passing run. +# +# It is also the exact hazard the docstring above cites as the reason not to +# import the macOS rig -- which the module-level import quietly voided. +# +# So it is loaded on first use and routed through require(): a missing +# sibling checkout is INVOLUNTARY, so the six tests that need it FAIL (exit 2 +# semantics), loudly and by name, while the four source-inspection tests below +# still run and still pin what they pin. +_HTTPS_E2E = None +_HTTPS_E2E_ERROR = None + +# The tests that cannot run without it, named so the failure says how much +# coverage is lost rather than just "import failed". +_NEEDS_HTTPS_E2E = 6 + + +def _https_e2e(): + """Return the https_e2e package, or fail this test loudly (never collect-fail).""" + global _HTTPS_E2E, _HTTPS_E2E_ERROR + if _HTTPS_E2E is None and _HTTPS_E2E_ERROR is None: + try: + import https_e2e as mod + except Exception as exc: # noqa: BLE001 - a broken sibling can raise anything + _HTTPS_E2E_ERROR = exc + else: + _HTTPS_E2E = mod + require( + _HTTPS_E2E is not None, + f"tools/https_e2e could not be imported ({_HTTPS_E2E_ERROR!r}) -- it " + "re-exports from .vice_on_bridge, which needs c64_test_harness from a " + "sibling checkout: pip install -e ../c64-test-harness", + executed=0, + total=_NEEDS_HTTPS_E2E, + certifies="the rigs' platform-before-prerequisites gate order", + opt_out_env="C64_ALLOW_SKIP", + ) + return _HTTPS_E2E + +# The four Linux bridge rigs. rig_phase3_https_1mhz is included even though +# it has two extra gates in front (an opt-in flag and a port check), because +# it is the one whose gate order was got wrong once already. +BRIDGE_RIGS = ( + "rig_phase1_dhcp", + "rig_phase2_http", + "rig_phase3_https", + "rig_phase3_https_1mhz", +) + +MACOS_RIG = os.path.join(_TESTS, "rig_vice_https_macos.py") + +P384_KAT = os.path.join(_HERE, "test_ecdsa_p384_kat.py") + +# The one bridge rig whose voluntary skip really does cost coverage: the +# macOS rig drives the emulated-RR-Net path over TLS, so plaintext HTTP has +# no counterpart there. Named here so the claim is asserted, not folklore. +PLAINTEXT_ONLY_RIG = os.path.join(_TESTS, "rig_phase2_http.py") + + +class Tripwire(AssertionError): + """Raised when a rig reaches a step an ordering test must never reach.""" + + +def _tripwire(name): + def _fire(*_a, **_k): + raise Tripwire(f"reached {name}() -- a gate let the run through") + return _fire + + +class _FakeSocket: + """Stands in for socket.socket in the 1 MHz rig's port-443 check. + + The bind succeeds, so the port check is not an environmental variable in + an ordering test. Whether 443 is free on the machine running the test + says nothing about gate order. + """ + + def __init__(self, *_a, **_k): + pass + + def bind(self, _addr): + return None + + def close(self): + return None + + +class _Patched: + """Restore module attributes and environment after a with-block.""" + + def __init__(self, mod, **kw): + self.mod = mod + self.kw = kw + self.saved = {} + self.env_saved = {} + + def env(self, **kw): + for k, v in kw.items(): + self.env_saved[k] = os.environ.get(k) + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + return self + + def __enter__(self): + for name, value in self.kw.items(): + self.saved[name] = getattr(self.mod, name) + setattr(self.mod, name, value) + return self + + def __exit__(self, *exc): + for name, value in self.saved.items(): + setattr(self.mod, name, value) + for k, v in self.env_saved.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + return False + + +def _import_rig(name): + import importlib + return importlib.import_module(name) + + +def _run_main(mod, extra_tripwires=()): + """Call mod.main() with output captured; returns (rc, output).""" + saved = {} + for attr in extra_tripwires: + saved[attr] = getattr(mod, attr) + setattr(mod, attr, _tripwire(attr)) + buf = io.StringIO() + try: + with redirect_stdout(buf): + rc = mod.main() + finally: + for attr, value in saved.items(): + setattr(mod, attr, value) + return rc, buf.getvalue() + + +# --------------------------------------------------------------------------- +# The platform predicate. +# --------------------------------------------------------------------------- + +def test_platform_supported_is_true_only_on_linux(): + _env = _https_e2e().env + saved = _env.sys.platform + try: + for value in ("linux", "linux2"): + _env.sys.platform = value + assert _env.platform_supported() is True, value + for value in ("darwin", "win32", "freebsd13", "cygwin"): + _env.sys.platform = value + assert _env.platform_supported() is False, value + finally: + _env.sys.platform = saved + + +def test_platform_supported_is_exported_from_the_package(): + # The rigs import it from the package, not from .env. + mod = _https_e2e() + assert mod.platform_supported is mod.env.platform_supported + + +# --------------------------------------------------------------------------- +# The ordering contract, behaviourally, on all four bridge rigs. +# --------------------------------------------------------------------------- + +def test_bridge_rigs_ask_platform_before_prerequisites(): + """Wrong platform -> exit 0, and check_prerequisites() is never called. + + The tripwire is the whole point: if a future edit moves the prerequisite + check back in front of the platform check, this raises instead of + quietly returning 2. + """ + pkg = _https_e2e() + for name in BRIDGE_RIGS: + mod = _import_rig(name) + with _Patched(pkg, platform_supported=lambda: False, + check_prerequisites=_tripwire("check_prerequisites")): + rc, out = _run_main(mod) + assert rc == 0, f"{name}: wrong platform must be exit 0, got {rc}\n{out}" + assert "NOT APPLICABLE" in out, f"{name}: no named verdict\n{out}" + + +def test_bridge_rigs_still_fail_when_the_platform_is_right(): + """Right platform + a missing prerequisite -> exit 2. + + The other half of the contract. A fix for the macOS over-fail that also + silenced the real coverage hole would pass the test above and fail this + one. + """ + pkg = _https_e2e() + for name in BRIDGE_RIGS: + mod = _import_rig(name) + saved_socket = getattr(mod, "socket", None) + if saved_socket is not None: + # Only rig_phase3_https_1mhz binds a port before the prerequisite + # check; neutralise it so gate ORDER is what this test measures. + mod.socket = type("m", (), {"socket": _FakeSocket, + "AF_INET": 0, "SOCK_STREAM": 0}) + try: + with _Patched(pkg, platform_supported=lambda: True, + check_prerequisites=lambda: ["a tool is missing"]).env( + VICE_HTTPS_OK_TO_RUN="1", C64_ALLOW_SKIP=None): + rc, out = _run_main(mod, extra_tripwires=("_ensure_built",)) + finally: + if saved_socket is not None: + mod.socket = saved_socket + assert rc == 2, f"{name}: missing prereq on-platform must be 2, got {rc}\n{out}" + assert "COULD NOT RUN" in out, f"{name}: no named verdict\n{out}" + + +def test_bridge_rigs_route_a_broken_build_to_two_without_an_opt_out(): + """A failed `make` is exit 2 even with C64_ALLOW_SKIP=1.""" + pkg = _https_e2e() + for name in BRIDGE_RIGS: + mod = _import_rig(name) + saved_socket = getattr(mod, "socket", None) + if saved_socket is not None: + mod.socket = type("m", (), {"socket": _FakeSocket, + "AF_INET": 0, "SOCK_STREAM": 0}) + saved_built = mod._ensure_built + mod._ensure_built = lambda: False # the build failed + try: + with _Patched(pkg, platform_supported=lambda: True, + check_prerequisites=lambda: []).env( + VICE_HTTPS_OK_TO_RUN="1", C64_ALLOW_SKIP="1"): + rc, out = _run_main(mod) + finally: + mod._ensure_built = saved_built + if saved_socket is not None: + mod.socket = saved_socket + assert rc == 2, f"{name}: broken build must be 2 even opted out, got {rc}\n{out}" + + +def test_the_interlock_flag_unset_is_contention_and_stays_exit_two(): + """VICE_HTTPS_OK_TO_RUN unset must NOT be laundered into a pass. + + This is the one gate where #178's own first draft got the taxonomy + backwards: it read an unset flag as "the operator declined an opt-in + rig" and returned 0, silently loosening the exit 2 this rig had on + master. Unset is the DEFAULT state, so it cannot tell a considered + decline from a forgotten flag -- and the flag asserts "the UCI HTTPS + listener has stopped", which is a contention claim. Contention is the + category with no opt-out at all. + + Pinned behaviourally, and pinned as NOT opt-out-able, so neither half + can be quietly reverted. + """ + mod = _import_rig("rig_phase3_https_1mhz") + with _Patched(_https_e2e(), platform_supported=lambda: True, + check_prerequisites=_tripwire("check_prerequisites")).env( + VICE_HTTPS_OK_TO_RUN=None): + rc, out = _run_main(mod) + assert rc == 2, f"unset interlock must be exit 2, got {rc}\n{out}" + assert "COULD NOT RUN" in out, out + assert "NOT APPLICABLE" not in out, out + assert "VICE_HTTPS_OK_TO_RUN" in out, out + + # ...and no environment variable may rescue it, unlike the + # missing-prerequisite lane three lines further down the rig. + with _Patched(_https_e2e(), platform_supported=lambda: True, + check_prerequisites=_tripwire("check_prerequisites")).env( + VICE_HTTPS_OK_TO_RUN=None, C64_ALLOW_SKIP="1"): + rc, out = _run_main(mod) + assert rc == 2, f"contention must not be opt-out-able, got {rc}\n{out}" + + +# --------------------------------------------------------------------------- +# tests/rig_vice_https_macos.py -- source inspection only (see module docstring). +# --------------------------------------------------------------------------- + +def test_the_u64_gate_is_not_opt_out_able_because_it_precedes_the_vice_lane(): + """--u64 with no U64_HOST must be exit 2 even under C64_ALLOW_SKIP=1. + + This gate is the one place in this PR where honouring the opt-out would + make an exit code WEAKER than master's, and the reason is scope rather + than arithmetic. On master, `--u64` with U64_HOST unset still ran the + entire VICE lane and returned `0 if total_fail == 0 else 1`, so a failing + emulator vector reported 1. The gate added here fires BEFORE + _build_prg() and before any VICE work, so an honoured opt-out returns 0 + with nothing run at all. + + C64_ALLOW_SKIP answers "this lane has no hardware" -- which is exactly + the operator who would set it here -- yet it would silence the EMULATOR + half, which needs no hardware. The remedy costs nothing and needs no + variable: set U64_HOST, or drop --u64. Pinned as a subprocess because + the exit CODE is the whole contract. + + Cheap by construction: the gate returns before the vector file check, + before _build_prg(), and before VICE, and the module imports nothing but + stdlib and _skip_policy -- so this runs in well under a second and + touches no hardware. + + THIS SPAWN IS THE SECOND LINK IN A TWO-DEEP CHAIN, and the depth is + bounded by inspection, not by a guard: + + pytest -> test_rig_skip_contract.py -> test_ecdsa_p384_kat.py + + tools/test_skip_policy.py spawns THIS module (its section 4 drives + _standalone() as a subprocess), _standalone() runs every test_*, so this + case runs nested and really does spawn the KAT. The KAT is a leaf only + because its gate returns before _build_prg(), which would otherwise run + `make clean` and two `make` invocations inside the 120 s timeout below. + Read the chain comment in tools/test_skip_policy.py before adding a third + link, or before moving that gate. + """ + env = dict(os.environ) + env.pop("U64_HOST", None) + env["C64_ALLOW_SKIP"] = "1" + proc = subprocess.run([sys.executable, P384_KAT, "--u64"], + capture_output=True, text=True, cwd=_REPO, env=env, + timeout=120) + out = proc.stdout + proc.stderr + assert proc.returncode == 2, ( + f"--u64 with no U64_HOST must be exit 2 even opted out, got " + f"{proc.returncode}\n{out}") + assert "COULD NOT RUN" in out, out + # ...and it must not advertise a hatch it does not honour. + assert "opt-out honoured" not in out, out + + +def test_phase2_does_not_claim_a_counterpart_it_does_not_have(): + """rig_phase2_http.py must not tell an operator its coverage is covered. + + The file used to contradict itself inside 40 lines: its module docstring + said "tests/rig_vice_https_macos.py owns the coverage", while its own + _COUNTERPART string -- the one that actually reaches the operator at + runtime -- said that rig drives TLS, "so plaintext HTTP specifically has + no macOS rig". The docstring is what a reader hits first. + + This matters past tidiness: the voluntary-skip verdict is justified by + "another rig owns the coverage, nothing is lost", and for this one rig + that is false. Exit 0 is still right (there is no remedy on macOS) but + the reason has to be the true one, or the exit code quietly overstates + what was verified -- the same defect class this module exists to catch. + """ + with open(PLAINTEXT_ONLY_RIG, "r", encoding="utf-8") as fh: + src = fh.read() + + # The runtime string is the anchor: assert it says what we think, so + # this test cannot pass because the string was silently reworded. + assert "no macOS rig" in src, ( + "rig_phase2_http.py must state that plaintext HTTP has no macOS " + "counterpart; the anchor string is gone") + + doc = ast.get_docstring(ast.parse(src)) or "" + assert doc, "rig_phase2_http.py lost its module docstring" + assert "owns the coverage" not in doc, ( + "rig_phase2_http.py's docstring claims another rig owns its " + "coverage, which its own _COUNTERPART denies:\n" + doc) + + +def _macos_source(): + with open(MACOS_RIG, "r", encoding="utf-8") as fh: + return fh.read() + + +def _index_of(src, needle, label): + i = src.find(needle) + # A matcher that matches nothing is the same vacuous shape this whole + # file is about, one level up: fail loudly rather than pass by absence. + assert i >= 0, f"anchor not found in rig_vice_https_macos.py: {label} ({needle!r})" + return i + + +def test_macos_rig_reports_problems_before_contention(): + """Contention on a rig that does not exist is the wrong headline. + + It is also not opt-out-able, so evaluating it first left a no-rig CI lane + red with no recourse even when it correctly set C64_ALLOW_SKIP=1. + """ + src = _macos_source() + check = _index_of(src, "problems, contention = _rig_check()", "the _rig_check call") + problems = _index_of(src[check:], "if problems:", "the problems branch") + contention = _index_of(src[check:], "if contention:", "the contention branch") + assert problems < contention, ( + "rig_vice_https_macos.py must test `problems` before `contention`") + + +def test_macos_rig_asks_platform_before_checking_the_rig(): + src = _macos_source() + main_at = _index_of(src, "\ndef main() -> int:", "main()") + body = src[main_at:] + platform = _index_of(body, "_platform_supported()", "the platform gate") + check = _index_of(body, "_rig_check()", "the _rig_check call") + assert platform < check, ( + "rig_vice_https_macos.py must ask the platform question before " + "inspecting the rig") + + +def test_macos_rig_contention_detector_is_not_a_bare_substring_match(): + """`pgrep -fl "ethernetioif feth0"` matched any process whose command + line merely contained that text -- a grep, an editor, a driver script. + A false positive is unrecoverable because contention has no opt-out. + """ + src = _macos_source() + assert '"-fl", "ethernetioif feth0"' not in src, ( + "the unanchored pgrep -fl substring match is back") + _index_of(src, '"pgrep", "-x", "x64sc"', "the binary-name match") + + +def test_macos_rig_build_failure_exits_two(): + src = _macos_source() + assert 'raise SystemExit("build failed")' not in src, ( + 'SystemExit("build failed") exits 1, which makes a broken build ' + "indistinguishable from a real handshake failure") + _index_of(src, "raise SystemExit(cannot_run(", "the build-failure verdict") + + +def _standalone() -> int: + """Run every test_* in this module without pytest. + + Honours the 0/1/2 contract of the module under test rather than + collapsing "could not run" onto 1. The six https_e2e tests raise + SkipPolicyError when the sibling harness checkout is absent -- that is a + coverage hole (exit 2), not a failed check (exit 1). + """ + tests = [(n, o) for n, o in sorted(globals().items()) + if n.startswith("test_") and callable(o)] + assert tests, "FATAL: no tests found -- a matcher that matches nothing" + failed = cannot = skipped = 0 + for name, fn in tests: + try: + fn() + print(f" PASS {name}") + except VoluntarySkip as exc: + skipped += 1 + print(f" SKIP {name}: {exc}") + except SkipPolicyError as exc: + cannot += 1 + print(f" CANNOT RUN {name}: {exc}") + except Exception as exc: # noqa: BLE001 + failed += 1 + print(f" FAIL {name}: {exc!r}") + passed = len(tests) - failed - cannot - skipped + tail = "" + if cannot: + tail += f", {cannot} COULD NOT RUN" + if skipped: + tail += f", {skipped} skipped by explicit opt-out" + print(f"\n{passed}/{len(tests)} passed{tail}") + if failed: + return EXIT_FAIL + if cannot: + return EXIT_CANNOT_RUN + return EXIT_PASS + + +if __name__ == "__main__": + sys.exit(_standalone()) diff --git a/tools/test_skip_policy.py b/tools/test_skip_policy.py new file mode 100644 index 0000000..08e1f89 --- /dev/null +++ b/tools/test_skip_policy.py @@ -0,0 +1,460 @@ +#!/usr/bin/env python3 +"""Tests for tools/_skip_policy.py -- the involuntary-skip rule itself (#178). + +Pure logic: no VICE, no hardware, no build. Runs under pytest from the repo +root (it is pinned in ``pytest.ini`` ``testpaths``) and standalone:: + + python3 tools/test_skip_policy.py + +The two things worth guarding here, both of which have a history: + + 1. **The opt-out must require the literal "1".** Bare truthiness makes + ``C64_ALLOW_SKIP=0`` *enable* the escape hatch, so someone setting 0 to + shut it off silently disables the whole policy instead. Every other + gate in this repo compares to "1"; the cases below pin that. + + 2. **The two lanes must not collapse into each other.** An involuntary + skip returns 2 and a voluntary one returns 0; a change that makes + everything fail is as wrong as the vacuous green it replaced. +""" + +import io +import os +import subprocess +import sys +import textwrap + +_HERE = os.path.dirname(os.path.abspath(__file__)) +if _HERE not in sys.path: + sys.path.insert(0, _HERE) + +from _skip_policy import ( # noqa: E402 + EXIT_CANNOT_RUN, + EXIT_FAIL, + EXIT_PASS, + SkipPolicyError, + VoluntarySkip, + cannot_run, + not_applicable, + reason_text, + require, +) + +ENV = "C64_TEST_SKIP_POLICY_OPT_OUT" + + +class _Env: + """Set (or unset) ENV for the duration of a with-block.""" + + def __init__(self, value): + self.value = value + self.saved = None + + def __enter__(self): + self.saved = os.environ.get(ENV) + if self.value is None: + os.environ.pop(ENV, None) + else: + os.environ[ENV] = self.value + return self + + def __exit__(self, *exc): + if self.saved is None: + os.environ.pop(ENV, None) + else: + os.environ[ENV] = self.saved + return False + + +def _call(value, **kw): + """cannot_run() with ENV set to `value`; returns (exit code, output).""" + buf = io.StringIO() + with _Env(value): + rc = cannot_run( + "a prerequisite is missing", + executed=0, + total=3, + certifies="the thing under test", + opt_out_env=ENV, + out=buf, + **kw, + ) + return rc, buf.getvalue() + + +# --------------------------------------------------------------------------- +# 1. The opt-out is exactly "1", and nothing else. +# --------------------------------------------------------------------------- + +def test_opt_out_unset_is_a_failure(): + rc, out = _call(None) + assert rc == EXIT_CANNOT_RUN, rc + assert "COULD NOT RUN" in out + assert "opt-out honoured" not in out + + +def test_opt_out_one_is_honoured(): + rc, out = _call("1") + assert rc == EXIT_PASS, rc + assert "opt-out honoured" in out + # The warning survives the opt-out: silencing the exit code must not + # silence the vacuity notice. + assert "certifies NOTHING" in out + assert "NOT because anything passed" in out + + +def test_opt_out_zero_does_not_open_the_hatch(): + # The regression this file exists for: bare truthiness made "0" enable + # the opt-out, so setting 0 to CLOSE it disabled the policy instead. + rc, _ = _call("0") + assert rc == EXIT_CANNOT_RUN, f"C64_ALLOW_SKIP=0 must not opt out (got {rc})" + + +def test_opt_out_false_does_not_open_the_hatch(): + rc, _ = _call("false") + assert rc == EXIT_CANNOT_RUN, f"=false must not opt out (got {rc})" + + +def test_opt_out_empty_does_not_open_the_hatch(): + rc, _ = _call("") + assert rc == EXIT_CANNOT_RUN, f"empty must not opt out (got {rc})" + + +def test_opt_out_tolerates_surrounding_whitespace(): + # A shell export can carry a stray space or a trailing newline (`export + # C64_ALLOW_SKIP=$(...)` is the usual source of the latter); both are + # still an explicit 1, and the newline is the case .strip() exists for. + for value in (" 1 ", "1\n", "\t1", " 1\n"): + rc, _ = _call(value) + assert rc == EXIT_PASS, (repr(value), rc) + + +def test_opt_out_yes_and_true_are_not_one(): + for value in ("yes", "true", "TRUE", "on", "2", "11"): + rc, _ = _call(value) + assert rc == EXIT_CANNOT_RUN, f"={value} must not opt out (got {rc})" + + +def test_no_opt_out_env_means_no_escape_hatch_at_all(): + # The build-failure and contention sites pass opt_out_env=None. Nothing + # in the environment may rescue them. + buf = io.StringIO() + with _Env("1"): + rc = cannot_run("the build is broken", executed=0, total=1, + certifies="anything", opt_out_env=None, out=buf) + assert rc == EXIT_CANNOT_RUN, rc + # Plain containment. This used to .replace("opt-out honoured", "") first, + # which deleted the exact phrase the assertion was looking for -- so it + # could not fail for the defect it targets. + out = buf.getvalue() + assert "opt-out" not in out, out + assert "COULD NOT RUN" in out + + +# --------------------------------------------------------------------------- +# 2. The two lanes stay distinct. +# --------------------------------------------------------------------------- + +def test_not_applicable_is_a_pass_with_a_named_verdict(): + buf = io.StringIO() + rc = not_applicable("UCI-only (backend=ip65)", + certifies="the dual-overlay dispatcher", out=buf) + assert rc == EXIT_PASS, rc + out = buf.getvalue() + assert "NOT APPLICABLE" in out + assert "certifies NOTHING" in out # quiet, but never silent + assert "COULD NOT RUN" not in out + + +def test_not_applicable_has_no_environment_switch(): + # A voluntary skip is decided by the code's own conditions, so no env + # var may turn it into a failure or vice versa. + for value in (None, "0", "1"): + buf = io.StringIO() + with _Env(value): + rc = not_applicable("wrong backend", certifies="x", out=buf) + assert rc == EXIT_PASS, (value, rc) + + +def test_the_two_exit_codes_are_distinct(): + # 2 must not collapse onto 1: "could not run" and "a check failed" are + # different states and callers are documented to tell them apart. + assert EXIT_CANNOT_RUN == 2 + assert EXIT_PASS == 0 + + +# --------------------------------------------------------------------------- +# 3. The reason string carries its own warning (the pytest -ra channel). +# --------------------------------------------------------------------------- + +def test_reason_text_is_self_contained(): + text = reason_text("ca65 not on PATH", executed=0, total=7, + certifies="the build-flag stamp", opt_out_env=ENV) + # Under pytest -ra this string is the ONLY channel that survives, so + # every part of the warning has to be inside it. + assert "ca65 not on PATH" in text + assert "0 of 7 checks executed" in text + assert "certifies NOTHING about the build-flag stamp" in text + assert ENV in text + assert "\n" not in text, "must stay one line for the -ra summary" + + +def test_require_raises_when_the_prerequisite_is_missing(): + with _Env(None): + try: + require(False, "ca65 not on PATH", executed=0, total=7, + certifies="the build-flag stamp", opt_out_env=ENV) + except SkipPolicyError as exc: + assert "certifies NOTHING" in str(exc) + else: + raise AssertionError("require() did not raise") + + +def test_require_is_silent_when_the_prerequisite_holds(): + with _Env(None): + require(True, "not reached", executed=1, total=1) + + +def test_require_still_raises_when_the_opt_out_is_zero(): + """=0 must FAIL, and must not be able to pass by SKIPPING either. + + The obvious spelling of this test -- `except SkipPolicyError: pass` -- + is vacuous against the exact defect it guards. If _opted_out() ever + reverts to bare truthiness, `bool("0")` is true, require() takes the + opt-out branch, calls pytest.skip(), and pytest records this case as + SKIPPED at exit 0. The guard converts itself into a green skip under + precisely the mutation it exists to catch -- the vacuous-skip shape + #178 exists to kill, inside #178's own implementation. + Measured: under that mutant this case was `1 skipped`, not a failure. + + So the catch is BaseException-wide. pytest.outcomes.Skipped derives + from BaseException, not Exception, which is what let it slip past. + """ + with _Env("0"): + try: + require(False, "ca65 not on PATH", executed=0, total=7, + certifies="the build-flag stamp", opt_out_env=ENV) + except SkipPolicyError: + pass + except BaseException as exc: # noqa: BLE001 - a Skipped here IS the bug + raise AssertionError( + f"=0 must fail, not skip or otherwise escape: {exc!r}") from None + else: + raise AssertionError("=0 must not suppress the failure") + + +def test_require_does_not_hand_a_skip_to_a_non_pytest_runner(): + """The opt-out must stay catchable by whoever is actually driving. + + require() asks sys.modules, not `import pytest`: pytest being INSTALLED + says nothing about pytest DRIVING, and pytest.skip() raises a + BaseException that a standalone runner's `except Exception` cannot + catch. Before this was fixed, `C64_ALLOW_SKIP=1 python3 + tools/test_rig_skip_contract.py` died mid-suite at exit 1 with no + summary and four runnable tests never run -- an opt-out that reports + as "a check ran and failed". + + Simulated by hiding pytest from sys.modules for the duration; the real + lane is any run of these modules as a script. + """ + saved = sys.modules.pop("pytest", None) + try: + with _Env("1"): + try: + require(False, "ca65 not on PATH", executed=0, total=7, + certifies="the build-flag stamp", opt_out_env=ENV) + except VoluntarySkip as exc: + assert isinstance(exc, Exception), "must be catchable as Exception" + assert "certifies NOTHING" in str(exc), str(exc) + except BaseException as exc: # noqa: BLE001 + raise AssertionError( + f"opt-out outside pytest must raise VoluntarySkip, got " + f"{exc!r}") from None + else: + raise AssertionError( + "opt-out must not RETURN outside pytest -- the caller would " + "run the body as if the prerequisite held") + finally: + if saved is not None: + sys.modules["pytest"] = saved + + +def test_require_hands_a_skip_to_pytest_when_pytest_is_driving(): + """The other half: with pytest driving, it is still a pytest skip. + + Pinned so the sys.modules fix cannot be "simplified" into never + skipping at all, which would make the documented escape hatch a lie. + """ + was_loaded = "pytest" in sys.modules + import pytest # noqa: PLC0415 - this test is about pytest specifically + + try: + with _Env("1"): + try: + require(False, "ca65 not on PATH", executed=0, total=7, + certifies="the build-flag stamp", opt_out_env=ENV) + except VoluntarySkip as exc: + raise AssertionError( + f"pytest is driving; the opt-out must be a pytest skip: " + f"{exc!r}") from None + except BaseException as exc: # noqa: BLE001 + assert type(exc).__name__ == "Skipped", repr(exc) + assert isinstance(exc, pytest.skip.Exception), repr(exc) + else: + raise AssertionError("the opt-out must not RETURN under pytest") + finally: + # Do not leave pytest in sys.modules for a standalone run that did + # not have it there: require() reads sys.modules, so this test would + # otherwise change how every later test in the file behaves. + if not was_loaded: + sys.modules.pop("pytest", None) + + +# --------------------------------------------------------------------------- +# 4. The standalone runner honours the 0/1/2 contract. +# +# Driven as a SUBPROCESS, because the exit code is the whole contract and +# pytest never collects _standalone() at all. The target is +# tools/test_rig_skip_contract.py rather than this module: it is the one with +# a genuine could-not-run lane (six tests need the sibling c64_test_harness +# checkout). +# +# SPAWN DEPTH IS 2, AND BOUNDED BY INSPECTION ONLY -- there is no guard in the +# code, so read this before adding a third link. The full chain is: +# +# pytest +# -> test_rig_skip_contract.py (this file spawns it, below) +# -> test_ecdsa_p384_kat.py (its test_the_u64_gate_* spawns THAT, +# and _standalone() runs every test_*, +# so the link really does fire here) +# +# It terminates, and it is cheap, for the SAME reason -- and that reason is a +# cross-dependency worth naming, because it is not local to either file. +# +# The KAT is not a leaf in general: _build_prg() runs `make clean` and two +# `make BACKEND=uci` invocations (test_ecdsa_p384_kat.py:646-665). What makes +# it a leaf HERE is that its --u64-with-no-U64_HOST gate returns BEFORE +# _build_prg(), so nothing further is spawned and the whole nested run +# measures ~0.07 s. Timeouts nest correctly -- 180 s on the outer call here, +# 120 s on the inner one there. +# +# So THE COST BOUND OF THIS TEST RESTS ON WHERE THAT GATE SITS. Moving it +# below _build_prg() -- the alternative fix considered and rejected when the +# gate was made non-opt-out-able -- would start a full `make clean && make` +# inside that 120 s inner timeout. Anyone reconsidering the gate's placement +# is also deciding this, and there is nothing at the gate that would say so; +# hence the note here and the pointer at the spawn site itself. +# +# An earlier version of this comment claimed the target "has no spawner of its +# own". That was true when it was written and false in the same commit, which +# is the whole reason the chain is spelled out rather than asserted: a third +# link would have to be added deliberately, in sight of this. +# --------------------------------------------------------------------------- + +_RIG_CONTRACT = os.path.join(_HERE, "test_rig_skip_contract.py") + +# Run a script with c64_test_harness made unimportable, so the six +# https_e2e-dependent tests take the require() path. A meta_path finder, +# not a PYTHONPATH trick: the sibling is pip-installed, so there is no path +# entry to remove. +_BLOCK_AND_RUN = textwrap.dedent(""" + import runpy, sys + class _Block: + def find_spec(self, name, path=None, target=None): + if name == "c64_test_harness" or name.startswith("c64_test_harness."): + raise ImportError("blocked by tools/test_skip_policy.py") + return None + sys.meta_path.insert(0, _Block()) + sys.argv = [sys.argv[1]] + runpy.run_path(sys.argv[0], run_name="__main__") +""") + + +def _run_blocked(script, **env_over): + env = dict(os.environ) + for k, v in env_over.items(): + if v is None: + env.pop(k, None) + else: + env[k] = v + return subprocess.run([sys.executable, "-c", _BLOCK_AND_RUN, script], + capture_output=True, text=True, timeout=180, + cwd=os.path.dirname(_HERE), env=env) + + +def test_standalone_runner_returns_two_for_could_not_run(): + """"Could not run" must not be reported as 1, which means "a check failed". + + Nothing collects _standalone(), so this is the only thing standing + between the 0/1/2 split and a future edit collapsing it back to + `return 1 if failed else 0` -- in a PR whose thesis is that a helper + with no call-site test is just another convention to miss. + """ + proc = _run_blocked(_RIG_CONTRACT, C64_ALLOW_SKIP=None) + out = proc.stdout + proc.stderr + assert proc.returncode == EXIT_CANNOT_RUN, ( + f"a missing prerequisite must be {EXIT_CANNOT_RUN}, not " + f"{proc.returncode}\n{out}") + assert "CANNOT RUN" in out, out + assert "COULD NOT RUN" in out, out # the vacuity string survives + assert "passed" in out, "the summary line must still be printed\n" + out + + +def test_standalone_runner_returns_zero_when_the_opt_out_is_honoured(): + """The opt-out lane, which is where defect D1 crashed the whole runner. + + Before the sys.modules fix this exact invocation died with an uncaught + pytest Skipped: exit 1, no summary, and the four tests that did not need + the sibling checkout never ran. + """ + proc = _run_blocked(_RIG_CONTRACT, C64_ALLOW_SKIP="1") + out = proc.stdout + proc.stderr + assert proc.returncode == EXIT_PASS, ( + f"an honoured opt-out must be {EXIT_PASS}, got {proc.returncode}\n{out}") + assert "Traceback" not in out, "the opt-out must not raise\n" + out + assert "skipped by explicit opt-out" in out, out + # The tests that did NOT need the sibling checkout must still have run. + assert "PASS test_macos_rig" in out, out + + +def _standalone() -> int: + """Run every test_* in this module without pytest. + + Honours the module's own 0/1/2 contract rather than collapsing "could + not run" onto 1: a SkipPolicyError is exactly the involuntary skip this + policy calls a 2, and reporting it as 1 ("a check ran and failed") is + the confusion the exit codes exist to prevent. + """ + tests = [(n, o) for n, o in sorted(globals().items()) + if n.startswith("test_") and callable(o)] + assert tests, "FATAL: no tests found -- a matcher that matches nothing" + failed = cannot = skipped = 0 + for name, fn in tests: + try: + fn() + print(f" PASS {name}") + except VoluntarySkip as exc: + skipped += 1 + print(f" SKIP {name}: {exc}") + except SkipPolicyError as exc: + cannot += 1 + print(f" CANNOT RUN {name}: {exc}") + except Exception as exc: # noqa: BLE001 + failed += 1 + print(f" FAIL {name}: {exc!r}") + passed = len(tests) - failed - cannot - skipped + tail = "" + if cannot: + tail += f", {cannot} COULD NOT RUN" + if skipped: + tail += f", {skipped} skipped by explicit opt-out" + print(f"\n{passed}/{len(tests)} passed{tail}") + if failed: + return EXIT_FAIL + if cannot: + return EXIT_CANNOT_RUN + return EXIT_PASS + + +if __name__ == "__main__": + sys.exit(_standalone())