PR #520 follow-ups: probe_gate's not-finished path has no main()-level test nf-alvs ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/528 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #520. All below the gate floor — #520's four findings (F1–F4) are fixed and verified in the code. Nothing here blocks anything.

1. probe_gate's not-finished path has no main()-level test

_run_main (tools/tests/test_probe_gate.py:351) stubs G.run with code 0 and a results table, and every test in DryRunTest / the sha-skip classes reuses that one shape. So the refusal and did-not-finish branches are pinned at parse_sweep and nowhere else.

That matters because F1's damage was not in parse_sweep — it was in what main() does next: record_baseline writing an empty failing map to the shared screenshots branch. A test that drives main() with Result(code=1, out="=== build ===\n") and asserts recorded == [] is the one that would have failed on the shipped bug.

2. A test whose name overclaims what it asserts

test_the_empty_failing_set_of_a_crash_cannot_reach_the_delta (test_probe_gate.py:160) is documented as *"the consequence, asserted end-to-end"*, but its body is a single assertFalse(G.parse_sweep(crashed, code=1).finished) — a near-duplicate of the test directly above it with a shorter fixture. It touches neither compute_delta nor main().

Either drive the delta/record path (item 1) or rename it. As written the next reader takes the gap in item 1 to be closed.

3. Nothing pins the two RESULTS_MARKER constants to each other

probe_all.py:233 and tools/pr/probe_gate.py:419 both define RESULTS_MARKER = "──── probe_all results ────" independently, and every test on both sides hardcodes the literal rather than importing either constant. A divergence leaves both suites green.

Post-F1 the marker is load-bearing again on the code route, so the consequence is: every merge prints *"the sweep did not finish (no results table) — nothing compared or recorded"* and exits 0, forever. Fail-closed and noisy rather than a false all-clear, which is why it is not a gate item — but it is the #389 shape (a UI gate that quietly stops gating). One assertion that the gate's marker equals the sweep's closes it.

4. Doc close-out (piece 11)
5. A systemic per-script failure records as a regression

With F2's containment, if *every* run_one raises (unwritable temp dir, probe.py absent, a permissions change), the sweep prints ~32 FAIL lines, exits 1, and the gate records all 32 as NEW failures attributed to that merge. This is parity with probe_all.sh under set -uo pipefail, and it fails loud rather than silent — noted only because it is the inverse of F1 and the two are easy to reason about together.

Dependencies

None.

Comments

No comments.

Add a comment