Use sys.executable instead of "python" in subprocess tests#103
Merged
Conversation
62 tests spawned a subprocess with the literal executable name "python", which does not exist on modern macOS or on many Linux distributions where only python3 is installed. CI passes because `uv sync` provides a venv in which "python" resolves, so the breakage is invisible there and hits anyone running pytest directly. sys.executable is correct regardless of environment and additionally guarantees the subprocess runs under the same interpreter as the test. Locally this takes the suite from 61 failed / 531 passed to 592 passed, 0 failed. Left tests/test_s3lfs.py's method="python" alone -- that names a compression backend, not an interpreter. Fixes #86 Co-Authored-By: Claude Opus 4.8 <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 #86. Re-lands the content of #93 against
main.#93 was merged into
cleanup-structure, which was subsequently bypassed when the rest of the work was rebased ontomain. The fix never reachedmainas a result, so this is the same change applied directly.The problem
62 tests spawn a subprocess with the literal executable name
python, which does not exist on modern macOS or on many Linux distributions where onlypython3is installed.CI does not catch it:
uv syncprovides a venv in whichpythonresolves. The breakage is invisible in CI and hits anyone runningpytestdirectly.Effect
On
main, locally:61 failed / 592 passed→653 passed, 0 failed.tests/test_manifest_not_exists_coverage.pytests/test_ls_command_coverage.pytests/test_git_root_coverage.pysys.executableis correct regardless of environment and additionally guarantees the subprocess runs under the same interpreter as the test, rather than whicheverpythonhappens to be first onPATH.Notes
tests/test_s3lfs.py'smethod="python"is deliberately untouched — that names a compression backend, not an interpreter.sys.executableis longer than"python"and pushed them past black's 88-char limit. That is the whole of the non-mechanical diff.main, so it is not repeated here.Why it matters beyond convenience
A suite with 61 permanent failures cannot be used as a local signal. During this work I misread a baseline for exactly that reason — comparing against a tree built with
git archive(which omits.git) and wrongly concluding a change had fixed 28 tests. Constant background noise makes that class of error easy to make and hard to notice.🤖 Generated with Claude Code
https://claude.ai/code/session_019UrDtdFKGwQZp8oYGwL2rj