PR #573 follow-ups: IS_WINDOWS as a seam, and three filing nits nf-m7wk ← Beads

open priority 2 task unassigned

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

Suggestions from ninefold-reviewer's review of #573. All four are sub-floor residuals from a CONVERGED verdict — none blocked the merge, none is a defect users or the gate would ever notice. Filed so the tension leaves the PR thread rather than because any of it is urgent. Unclaimed.

**1. presubmit.py:392-394 reaches for os.name == "nt" where nflib.godot now exposes IS_WINDOWS.** The consequence is testability, not correctness: the F3 test (test_presubmit.test_a_present_but_non_executable_godot_is_NOT_reported_green) has to skipTest on Windows, whereas the two test_nflib_godot tests added in the same commit *drive* IS_WINDOWS and so assert both platforms' arms from either box. That difference is exactly #565's direction 2. Pointing presubmit.doctor() at godot.IS_WINDOWS would let the Windows arm of the exec-bit check be asserted from Linux too.

**2. tools/tests/test_f5.pytest_a_stale_override_falls_THROUGH_to_discovery's docstring claims more isolation than it has.** It says *"Patched on the f5 module's own reference, so this pins f5's fall-through behaviour and not the resolver's."* f5.godot is the nflib.godot module object (from nflib import godot, repo_root), so mock.patch.object(f5.godot, "find", …) patches the resolver globally for the duration. Harmless — mock restores it — but the stated reason is wrong, and in this repo the docstring is the record. Either delete the sentence or patch godot.find and say so.

**3. tools/nflib/godot.pyIS_WINDOWS is a load-bearing test seam but is absent from __all__** (["GLOBS", "INSTALL_ROOT", "describe_search", "find"]). Two tests patch it to drive the Windows arms. Either add it, or note in the module docstring that it is public-by-use.

**4. tools/tests/test_nflib_godot.pytest_the_not_found_message_does_not_promise_a_PATH_search_on_windows sits in FindTest,** immediately above the DescribeSearchTest class it belongs to. Pure filing.

Dependencies

None.

Comments

No comments.

Add a comment