Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid modifying path placeholders created by editable installs #1030

Merged

Conversation

jeremy-hiatt
Copy link
Contributor

The setuptools implementation of editable installs will insert a placeholder entry into sys.path as part of its magic to register its custom import mechanism.

These are not real filesystem paths and as such should not be rewritten to absolute paths.

This is a proposed fix for #1028 .

Checklist:

  • Make sure to include reasonable tests for your change if necessary

I couldn't figure out how to test this specific behavior, but happy to add one given guidance on how to modify sys.path in the test environment.

  • We use towncrier for changelog management, so please add a news file

Added.

The setuptools implementation of editable installs will insert a
placeholder entry into sys.path as part of its magic to register its
custom import mechanism.

These are not real filesystem paths and as such should not be
rewritten to absolute paths.
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, any idea how to test this sanely

@jeremy-hiatt
Copy link
Contributor Author

This looks good, any idea how to test this sanely

@RonnyPfannschmidt , I tried a bit longer in response to your comment but I still can't figure out how to set up the test so it runs with a custom sys.path element. I found the monkeypatch.syspath_prepend() method but it doesn't seem to work here. Would you like me to keep trying or are you able to merge as-is?

@jeremy-hiatt
Copy link
Contributor Author

Hi @RonnyPfannschmidt , I think I found a reasonably straightforward way to create a test, and I added this commit to the PR. If there's nothing else, I'd appreciate it if you're able to merge!

@RonnyPfannschmidt RonnyPfannschmidt merged commit 54720c1 into pytest-dev:master Mar 1, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants