[Bug Fix] Pytester.syspathinsert() has no effect when using runpytest_subprocess() . closes #10651 #12812
Conversation
for more information, see https://pre-commit.ci
…ytester-subprocess
for more information, see https://pre-commit.ci
…process' of https://github.com/Oreldm/pytest into bug-fix-10651-syspathinsert-not-passing-to-pytester-subprocess
for more information, see https://pre-commit.ci
…process' of https://github.com/Oreldm/pytest into bug-fix-10651-syspathinsert-not-passing-to-pytester-subprocess
psf-chronographer
bot
added
the
bot:chronographer:provided
for more information, see https://pre-commit.ci
…process' of https://github.com/Oreldm/pytest into bug-fix-10651-syspathinsert-not-passing-to-pytester-subprocess
for more information, see https://pre-commit.ci
…process' of https://github.com/Oreldm/pytest into bug-fix-10651-syspathinsert-not-passing-to-pytester-subprocess
for more information, see https://pre-commit.ci
…process' of https://github.com/Oreldm/pytest into bug-fix-10651-syspathinsert-not-passing-to-pytester-subprocess
for more information, see https://pre-commit.ci
…ytester-subprocess
src/_pytest/pytester.py
Outdated
| self._syspath_prepended = path_str | ||
|
|
||
| # Store the prepended path in an attribute that persists across method calls | ||
| if not hasattr(self, "_prepended_syspaths"): |
There was a problem hiding this comment.
Let's avoid hasattr hacks, just declare the required attributes on __init__. 👍
src/_pytest/pytester.py
Outdated
| self._monkeypatch.syspath_prepend(str(path)) | ||
| path_str = str(path) | ||
| self._monkeypatch.syspath_prepend(path_str) | ||
| self._syspath_prepended = path_str |
There was a problem hiding this comment.
_syspath_prepended only stores the latest path called by syspathinsert, but syspathinsert might be called multiple times, and all paths should be considered.
src/_pytest/pytester.py
Outdated
| # Store the prepended path in an attribute that persists across method calls | ||
| if not hasattr(self, "_prepended_syspaths"): | ||
| self._prepended_syspaths = [] | ||
| self._prepended_syspaths.append(path_str) |
There was a problem hiding this comment.
Let's just store Path objects here, no need to store strings -- we can convert to strings later.
src/_pytest/pytester.py
Outdated
| pythonpath = env.get("PYTHONPATH", "") | ||
|
|
||
| paths_to_add = [os.getcwd()] | ||
| if hasattr(self, "_syspath_prepended"): |
There was a problem hiding this comment.
We should add all paths from _prepended_syspaths at this point.
src/_pytest/pytester.py
Outdated
| python_executable = sys.executable | ||
| pytest_command = [python_executable, "-m", "pytest"] | ||
|
|
||
| if hasattr(self, "_syspath_prepended"): |
There was a problem hiding this comment.
Why are we setting sys.path and configuring PYTHONPATH? Isn't sufficient to only configure PYTHONPATH?
There was a problem hiding this comment.
Actually I don't think we need to modify runpytest_subprocess at all, given we are already configuring PYTHONPATH in popen?
|
Thanks @Oreldm for the contribution, please take a look at my comments. |
Co-authored-by: Bruno Oliveira <[email protected]>
for more information, see https://pre-commit.ci
…ytester-subprocess
If this change fixes an issue, please:
closes #XYZWto the PR description and/or commits (whereXYZWis the issue number). See the github docs for more information.Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:
Create a new changelog file in the
changelogfolder, with a name like<ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.Write sentences in the past or present tense, examples:
Also make sure to end the sentence with a
..Add yourself to
AUTHORSin alphabetical order.Closes #10651