PR #466 follow-ups: a stale test count, a meta-guard blind spot, and a duplicated NF_GH_BIN rule nf-bko9 ← Beads

open priority 2 task unassigned

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

Suggestions from ninefold-reviewer's review of #466. All three are below the gate — #466 reached result=CONVERGED with none of them outstanding as blockers. Filed rather than cycled back into the PR.

1. --selftest's test count is decoration that keeps going stale

tools/reviewer/dispatch.py:927 — the docstring says test_dispatch.py "now drives all 63 cases". The suite is 66 at 03ef0a0; the commit that corrected 51 → 63 added three tests in the same change.

Third value for the same number in one PR. The fix is probably to stop naming a count at all (the sentence's point is "it delegates to the real suite", which the count adds nothing to) rather than to update it a fourth time.

2. NoRealSubprocessTest does not see tests that reach main() through a helper

tools/tests/test_dispatch.py:784 splits the module source on def test_ and skips any block containing neither D.main( nor _run_main_from. DeviationGuardTest's two new tests call self._drive(...), so both are skipped:

``` CHECKED by the meta-guard: dry_run_never_spawns, main_runs_from_the_repo_root…, self_sync_does_NOT_reset…, an_empty_REPO…, the_flag_runs_the_suite…, it_really_touches_NO_NETWORK, the_tick_token_IS_memoised… NOT checked: test_dry_run_does_NOT_self_sync test_a_real_tick_DOES_self_sync ```

No live hazard_drive fakes D.run itself, so every test using it is safe by construction — and _drive's own source *is* scanned, but only by the accident that it lexically follows test_the_tick_token_IS_memoised_for_label_writes and therefore lands inside that test's block. Reorder the classes and the coverage silently moves.

The guard's stated property is "a unit test must not be able to destroy the working tree" (#472 F5). Suggestion: recognise helper call sites explicitly — treat self._drive( as a main() driver and require the helper to be in the faking set — so the guarantee doesn't depend on file ordering.

3. $NF_GH_BIN resolution now exists in two places

Correct in both, and #466 I2 is genuinely closed by the second one. But this is the same shape as the four-copy verdict schema #466 exists to de-duplicate: one rule, two producers, no test tying them together. If a third override lands (a --repo shim, a recorder that also needs GH_HOST), the drift is already set up.

Suggestion: export the resolution from nflib.gh — a gh_argv() or similar — and have dispatch.py call it.

Dependencies

None.

Comments

No comments.

Add a comment