Build the venv a Python test needs instead of imitating one - #5
Merged
Conversation
The first CI run on three operating systems found four failures, all in the
Python tests and none in what they were testing.
The environment they set up was hand-built: a pyvenv.cfg beside a symlink at
/usr/bin/python3, on the argument that a real one would take a minute and prove
the same thing. It does not. Whether Python recognises that as a virtual
environment depends on what the symlink resolves to, and on macOS
/usr/bin/python3 is the Command Line Tools stub, which re-execs the real binary
- so sys.executable is the framework's path, the pyvenv.cfg beside the symlink
is never seen, and the environment's site-packages never reaches sys.path.
Pyright was handed an interpreter that answers nothing ("Unable to get Python
version from interpreter") and resolved the import to nothing. The layout was
POSIX-only besides: a venv on Windows keeps its packages in Lib\site-packages.
`python -m venv --without-pip` is right on every platform and costs a tenth of
a second, and the interpreter reports where its own packages go, so nothing is
guessed. Finding the Python to build it with goes through LanguageServers.OnPath,
which already knows that a bare command name on Windows means PATHEXT; where
there is none, the test says so and is ignored rather than throwing "no system
python3", which is what every Windows machine got.
The fourth failure was cleanup: deleting the workspace while the server is
still letting go of it fails on Windows and fails a test whose assertions all
passed. TempDirectory.Delete exists for that and is what every other test uses.
Assisted-by: Claude:claude-opus-5:Claude Code
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.
The first CI run across three operating systems (run 33433853550) is red on Windows (4 failures) and macOS (2), green on Linux. Nothing in the product regressed - the build succeeds everywhere - the tests were written against a POSIX machine.
TheProjectsOwnEnvironmentIsPreferredToWhateverIsOnPathno system python3AnImportResolvesIntoAnEnvironmentThatIsNotInTheWorktreeno system python3AProjectConfigNamingItsOwnVenvDoesNotBlindTheReviewno system python3PyrightAnswersDefinitionsAndReferencesAcrossFilesIOExceptiondeleting the temp dirThe fake environment is not an interpreter on macOS. A
pyvenv.cfgbeside a symlink at/usr/bin/python3is how a venv looks, but/usr/bin/python3there is the Command Line Tools stub: it re-execs the real binary, sosys.executablebecomes the framework path, the plantedpyvenv.cfgis never seen, and the fakesite-packagesnever reachessys.path. Pyright logsUnable to get Python version from interpreterand resolves the import to nothing. Reproduced locally and fixed there. The layout was POSIX-only anyway - a venv on Windows keeps packages inLib\site-packages.SystemPython()hardcoded/usr/bin/python3, so it threw on every Windows machine whatever Python was installed - three of the four Windows failures.The fourth was cleanup, not an assertion:
Directory.Deletewhile pyright's node process still holds the workspace root.TempDirectory.Deleteexists for exactly that and is what every other test uses.So: one
PythonVenvhelper that builds a real environment withpython -m venv --without-pip(a tenth of a second, no network) and asks the interpreter where its packages go. The Python to build it with comes fromLanguageServers.OnPath, which already knows a bare name on Windows means PATHEXT; with no Python at all the test is ignored instead of throwing. ~60 lines of duplicated fakes go.Tests only - no product code, no workflow change. All three runners already have a Python, so these run rather than skip everywhere.
Verified locally on macOS: red first (same failure as CI), then 261/261 green in Release. Windows is what this PR's own CI is for.