Part of #108. Found while fixing #130.
1. scripts/pre_push_check.py can never report success
Its docstring says "Run this before pushing to ensure GitHub Actions won't fail," and CLAUDE.md instructs running it "repeatedly until they ALL pass." It cannot pass, and never could:
$ flake8 clustrix/ tests/ --max-line-length=88 --extend-ignore=E203,W503,F401,E722,F541,F841,F811,E731,E501,W291,W293,F824 | wc -l
92 # on master
91 # on the #130 branch
CI does not catch this because .github/workflows/tests.yml appends --exit-zero to the same flake8 invocation, so the findings are printed and ignored. The local script omits --exit-zero, so it always returns non-zero, burns all 5 retry attempts, and prints "Maximum attempts reached."
Net effect: the one gate developers are told to trust is permanently red for reasons unrelated to whatever they just changed, which trains people to ignore it.
Decide one of:
- add the deliberate patterns (
E402, W504) to extend-ignore in both places and keep the local gate strict, or
- fix the findings, or
- make the local script mirror CI's
--exit-zero (weakest option — flake8 then gates nothing anywhere).
2. tests/integration/test_direct_gpu_detection.py has a real f-string bug
75 of the 92 findings are E402 (deliberate sys.path setup before imports). The remainder are not stylistic. Thirteen cluster in one file, and they are pointing at a genuine defect rather than a false positive.
test_direct_gpu_detection.py:42 opens an f-string whose body is a Python program meant to run on the remote host:
"-c",
f"""
import os
print(f'CUDA_VISIBLE_DEVICES_ENV: {cuda_env}')
try:
import torch
print(f'TORCH_VERSION: {torch.__version__}')
...
for i in range(torch.cuda.device_count()):
props = torch.cuda.get_device_properties(i)
print(f'GPU_{i}: {props.name} ...')
except ImportError as e:
print(f'TORCH_IMPORT_ERROR: {e}')
Only {cuda_env} is meant to be interpolated locally. Everything else — {torch.__version__}, {i}, {props.name}, {e}, {line}, {gpu_lines} — belongs to the remote program and must be written {{...}}. As written, the outer f-string evaluates them in the local scope when the string is built, so this raises NameError: name 'torch' is not defined before the subprocess is ever launched.
flake8 reports exactly these as F821 undefined name, plus one F823. They are true positives:
tests/integration/test_direct_gpu_detection.py:48:46: F821 undefined name 'torch'
tests/integration/test_direct_gpu_detection.py:55:48: F821 undefined name 'props'
tests/integration/test_direct_gpu_detection.py:57:51: F821 undefined name 'e'
tests/integration/test_direct_gpu_detection.py:69:61: F821 undefined name 'gpu_lines'
tests/integration/test_direct_gpu_detection.py:73:53: F823 local variable 'result' ...
This survived because tests/integration/ is gated by the #109 billable-resources guard and is never collected in CI, so the module is never imported.
Two more, unrelated to that file:
tests/real_world/api_validation/validate_container_registry.py:202:39: F821 undefined name 'json' # missing import
tests/real_world/cluster_validation/debug_slurm_output_location.py:119:31: W605 invalid escape sequence '\$'
Acceptance criteria
Not in scope
The pytest command in pre_push_check.py was already corrected under #130: it ran a bare pytest, which was harmless only while the project had no effective config. With testpaths live it resolved to all 1670 tests including the 224 network-bound ones in tests/real_world/, and did not terminate. It now runs pytest tests/unit/ -m "not real_world", matching CI.
Part of #108. Found while fixing #130.
1.
scripts/pre_push_check.pycan never report successIts docstring says "Run this before pushing to ensure GitHub Actions won't fail," and
CLAUDE.mdinstructs running it "repeatedly until they ALL pass." It cannot pass, and never could:CI does not catch this because
.github/workflows/tests.ymlappends--exit-zeroto the same flake8 invocation, so the findings are printed and ignored. The local script omits--exit-zero, so it always returns non-zero, burns all 5 retry attempts, and prints "Maximum attempts reached."Net effect: the one gate developers are told to trust is permanently red for reasons unrelated to whatever they just changed, which trains people to ignore it.
Decide one of:
E402,W504) toextend-ignorein both places and keep the local gate strict, or--exit-zero(weakest option — flake8 then gates nothing anywhere).2.
tests/integration/test_direct_gpu_detection.pyhas a real f-string bug75 of the 92 findings are
E402(deliberatesys.pathsetup before imports). The remainder are not stylistic. Thirteen cluster in one file, and they are pointing at a genuine defect rather than a false positive.test_direct_gpu_detection.py:42opens an f-string whose body is a Python program meant to run on the remote host:Only
{cuda_env}is meant to be interpolated locally. Everything else —{torch.__version__},{i},{props.name},{e},{line},{gpu_lines}— belongs to the remote program and must be written{{...}}. As written, the outer f-string evaluates them in the local scope when the string is built, so this raisesNameError: name 'torch' is not definedbefore the subprocess is ever launched.flake8 reports exactly these as
F821 undefined name, plus oneF823. They are true positives:This survived because
tests/integration/is gated by the #109 billable-resources guard and is never collected in CI, so the module is never imported.Two more, unrelated to that file:
Acceptance criteria
scripts/pre_push_check.pyexits 0 on a clean treetest_direct_gpu_detection.pybraces escaped; module imports and the remote program runsvalidate_container_registry.pyimportsjsonW605escape fixedE402either ignored by policy or resolvedNot in scope
The
pytestcommand inpre_push_check.pywas already corrected under #130: it ran a barepytest, which was harmless only while the project had no effective config. Withtestpathslive it resolved to all 1670 tests including the 224 network-bound ones intests/real_world/, and did not terminate. It now runspytest tests/unit/ -m "not real_world", matching CI.