[red-knot] Allow setting python in mdtests#17221
Conversation
MichaReiser
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
|
|
👍 the test I have locally passes when it's rebased on this branch |
|
thanks! |
|
Oh no shoot, I messed it up locally. Actually it now fails with this when my PR branch is rebased onto this branch: |
| python_path: PythonPath::KnownSitePackages( | ||
| configuration | ||
| .python() | ||
| .into_iter() | ||
| .map(SystemPath::to_path_buf) | ||
| .collect(), | ||
| ), |
There was a problem hiding this comment.
Hmm, I'm still struggling to get my test to pass locally. I think this is incorrect because the python configuration setting is meant to point to the installation's sys.prefix value (e.g. .venv) but the KnownSitePackages variant of PythonPath is meant to hold the resolved site-packages subdirectory of sys.prefix (e.g. .venv/lib/python3.13/site-packages if it's a Python 3.13 virtual environment)
There was a problem hiding this comment.
I think this should be using PythonPath::SysPrefix instead
There was a problem hiding this comment.
I'll fix this as part of my PR
There was a problem hiding this comment.
🤦 yes, this should be SysPrefix, at least if it should mirror the CLI
…ges` test as an mdtest (#17222) ## Summary This PR does the following things: - Fixes the `python` configuration setting for mdtest (added in #17221) so that it expects a path pointing to a venv's `sys.prefix` variable rather than the a path pointing to the venv's `site-packages` subdirectory. This brings the `python` setting in mdtest in sync with our CLI `--python` flag. - Tweaks mdtest so that it automatically creates a valid `pyvenv.cfg` file for you if you don't specify one. This makes it much more ergonomic to write an mdtest with a custom `python` setting: red-knot will reject a `python` setting that points to a directory that doesn't have a `pyvenv.cfg` file in it - Tweaks mdtest so that it doesn't check a custom `pyvenv.cfg` as Python source code if you _do_ add a custom `pyvenv.cfg` file for your mock virtual environment in an mdtest. (You get a lot of diagnostics about Python syntax errors in the `pyvenv.cfg` file, otherwise!) - Rewrites the test added in #17178 as an mdtest, and deletes the original test that was added in that PR ## Test Plan I verified that the new mdtest fails if I revert the changes to `resolver.rs` that were added in #17178
2 participants
Summary
This PR extends the mdtest options to allow setting the
environment.pythonoption.Test Plan
I let @AlexWaygood write a test and he'll tell me if it works 😆