Repository navigation
A missing Python is not a skip, it is UNSUP, and it blocks - #3
Merged
Merged
Conversation
WHAT CHANGED test/canonical-float-policy.test.mjs 5 x `if (!python) return` -> requireCapability WHY The file I added in 90a22a9 returned early when no python was found, which prints `ok` having asserted nothing. harness.mjs already has the mechanism for this, and its docstring says exactly what I did wrong: "It is NOT a skip. A test that returns early on a missing prerequisite prints `ok` having asserted nothing, which is the same false green ADR-0021 exists to prevent, one layer up." Worst in this file of all places: the defect under test IS a Node/Python divergence, so a green run with no python validates nothing at all while claiming the two languages agree. Mine was the only silent skip in the repository. The three older Python tests assert `python.status === 0`, so a python-less host fails them - blunt, but never a false green. WHAT I VERIFIED node research-kit/bin/selftest.mjs -> 565 passed, 0 failed node research-kit/bin/preflight.mjs -> PASS, 0 blocking, 0 warnings Reproduced the missing-python condition by pointing the probe at an interpreter that does not exist: 5 UNSUP PYTHON-NOT-FOUND, blocking=5, exit 1 (selftest.mjs:56) 1 ok the structural scan, which needs no python and correctly still runs Attempted first by stripping PATH; on Windows the child still resolved python, so that reproduction was worthless and I replaced it with one that works. CI cannot hit this path: .github/workflows/offline-suite.yml pins actions/setup-python@v5 at 3.12 and runs `python --version` before the suite. WHAT I DID NOT CHANGE The three older conformance tests. They block correctly already; converting them is a reporting improvement, not a defect fix, and belongs in its own change rather than widening this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the review finding on #2.
The defect
The test file added in
90a22a9usedif (!python) returnin 5 tests. That printsokhaving asserted nothing — a false green. It was worst in this file of all places, because the defect under test is a Node/Python divergence: a green run with no Python validates nothing while claiming the two languages agree.harness.mjsalready has the mechanism, and its docstring names the exact mistake:Mine was the only silent skip in the repository. The three older Python tests assert
python.status === 0, so a Python-less host fails them — blunt, but never a false green.The fix
5 ×
if (!python) return→requireCapability(PYTHON, 'PYTHON-NOT-FOUND', ...).Verification
Reproduced the missing-Python condition by pointing the probe at an interpreter that does not exist:
I tried stripping
PATHfirst; on Windows the child process still resolved Python, so that reproduction proved nothing and I replaced it with one that works.CI cannot hit this path anyway:
.github/workflows/offline-suite.ymlpinsactions/setup-python@v5at 3.12 and runspython --versionbefore the suite — so the second half of the review question is already guaranteed.Deliberately not changed
The three older conformance tests. They block correctly already; converting them to
requireCapabilityis a reporting improvement, not a defect fix, and belongs in its own change rather than widening this one.🤖 Generated with Claude Code