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

[Bug Fix] Pytester.syspathinsert() has no effect when using runpytest_subprocess() . closes #10651 #12812

Draft
wants to merge 30 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1a772a3
progress
Oreldm Sep 13, 2024
7c775f3
Update Authors
Oreldm Sep 13, 2024
0a569ca
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 13, 2024
bc0aaa1
Merge branch 'main' into bug-fix-10651-syspathinsert-not-passing-to-p…
Oreldm Sep 13, 2024
6f95f59
Fix DOCString
Oreldm Sep 13, 2024
c78ed4a
Fix DOCString
Oreldm Sep 13, 2024
b88d0d3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 13, 2024
5bb131f
Fix DOCString
Oreldm Sep 13, 2024
f630494
Merge branch 'bug-fix-10651-syspathinsert-not-passing-to-pytester-sub…
Oreldm Sep 13, 2024
4d691ea
progress
Oreldm Sep 13, 2024
a87a190
progress
Oreldm Sep 13, 2024
6b468fe
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 13, 2024
30f2ba7
added bugfix.rst
Oreldm Sep 13, 2024
7f10717
Merge branch 'bug-fix-10651-syspathinsert-not-passing-to-pytester-sub…
Oreldm Sep 13, 2024
d31e435
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 13, 2024
ac930cd
progress
Oreldm Sep 13, 2024
994d24d
Merge branch 'bug-fix-10651-syspathinsert-not-passing-to-pytester-sub…
Oreldm Sep 13, 2024
03d80d0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 13, 2024
3d24eb3
progress
Oreldm Sep 13, 2024
f731aaf
Merge branch 'bug-fix-10651-syspathinsert-not-passing-to-pytester-sub…
Oreldm Sep 13, 2024
2690455
progress
Oreldm Sep 13, 2024
6679929
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 13, 2024
080b763
Fix SyntaxError in Pytester.runpytest_subprocess due to unescaped paths
Oreldm Sep 13, 2024
1d74520
Merge branch 'bug-fix-10651-syspathinsert-not-passing-to-pytester-sub…
Oreldm Sep 13, 2024
5719422
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 13, 2024
32fe1b9
Merge branch 'main' into bug-fix-10651-syspathinsert-not-passing-to-p…
Oreldm Sep 17, 2024
2924ad0
[PR Fix] Update testing/test_pytester.py remove timeout
Oreldm Sep 20, 2024
deee731
PR Fix - multiple env variable fix
Oreldm Sep 20, 2024
8bd1a5b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 20, 2024
3c88995
Merge branch 'main' into bug-fix-10651-syspathinsert-not-passing-to-p…
Oreldm Sep 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ Oliver Bestwalter
Omar Kohl
Omer Hadari
Ondřej Súkup
Orel Damari
Oscar Benjamin
Parth Patel
Patrick Hayes
Expand Down
1 change: 1 addition & 0 deletions changelog/10651.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed bug where the `Pytester.syspathinsert` has no effect when using subprocess.
54 changes: 47 additions & 7 deletions src/_pytest/pytester.py
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,14 @@
if path is None:
path = self.path

self._monkeypatch.syspath_prepend(str(path))
path_str = str(path)
self._monkeypatch.syspath_prepend(path_str)
self._syspath_prepended = path_str
Copy link
Member

Choose a reason for hiding this comment

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

_syspath_prepended only stores the latest path called by syspathinsert, but syspathinsert might be called multiple times, and all paths should be considered.


# Store the prepended path in an attribute that persists across method calls
if not hasattr(self, "_prepended_syspaths"):
Copy link
Member

Choose a reason for hiding this comment

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

Let's avoid hasattr hacks, just declare the required attributes on __init__. 👍

self._prepended_syspaths = []
self._prepended_syspaths.append(path_str)
Copy link
Member

Choose a reason for hiding this comment

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

Let's just store Path objects here, no need to store strings -- we can convert to strings later.


def mkdir(self, name: str | os.PathLike[str]) -> Path:
"""Create a new (sub)directory.
Expand Down Expand Up @@ -1337,10 +1344,16 @@

You probably want to use :py:meth:`run` instead.
"""
env = os.environ.copy()
env["PYTHONPATH"] = os.pathsep.join(
filter(None, [os.getcwd(), env.get("PYTHONPATH", "")])
)
env = kw.pop("env", None) or os.environ.copy()
pythonpath = env.get("PYTHONPATH", "")

Check warning on line 1348 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1347-L1348

Added lines #L1347 - L1348 were not covered by tests

paths_to_add = [os.getcwd()]

Check warning on line 1350 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1350

Added line #L1350 was not covered by tests
if hasattr(self, "_syspath_prepended"):
Copy link
Member

Choose a reason for hiding this comment

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

We should add all paths from _prepended_syspaths at this point.

paths_to_add.insert(0, self._syspath_prepended)

Check warning on line 1352 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1352

Added line #L1352 was not covered by tests

pythonpath = os.pathsep.join(filter(None, [*paths_to_add, pythonpath]))

Check warning on line 1354 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1354

Added line #L1354 was not covered by tests

env["PYTHONPATH"] = pythonpath

Check warning on line 1356 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1356

Added line #L1356 was not covered by tests
kw["env"] = env

if stdin is self.CLOSE_STDIN:
Expand All @@ -1365,6 +1378,7 @@
*cmdargs: str | os.PathLike[str],
timeout: float | None = None,
stdin: NotSetType | bytes | IO[Any] | int = CLOSE_STDIN,
env: dict[str, str] | None = None,
) -> RunResult:
"""Run a command with arguments.

Expand Down Expand Up @@ -1413,6 +1427,7 @@
stdout=f1,
stderr=f2,
close_fds=(sys.platform != "win32"),
env=env,
)
if popen.stdin is not None:
popen.stdin.close()
Expand Down Expand Up @@ -1488,8 +1503,33 @@
plugins = [x for x in self.plugins if isinstance(x, str)]
if plugins:
args = ("-p", plugins[0], *args)
args = self._getpytestargs() + args
return self.run(*args, timeout=timeout)

env = os.environ.copy()
pythonpath = env.get("PYTHONPATH", "")

Check warning on line 1508 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1507-L1508

Added lines #L1507 - L1508 were not covered by tests

if hasattr(self, "_syspath_prepended"):
pythonpath = os.pathsep.join(

Check warning on line 1511 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1511

Added line #L1511 was not covered by tests
filter(None, [self._syspath_prepended, pythonpath])
)

env["PYTHONPATH"] = pythonpath

Check warning on line 1515 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1515

Added line #L1515 was not covered by tests

python_executable = sys.executable
pytest_command = [python_executable, "-m", "pytest"]

Check warning on line 1518 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1517-L1518

Added lines #L1517 - L1518 were not covered by tests

if hasattr(self, "_syspath_prepended"):
Copy link
Member

Choose a reason for hiding this comment

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

Why are we setting sys.path and configuring PYTHONPATH? Isn't sufficient to only configure PYTHONPATH?

Copy link
Member

Choose a reason for hiding this comment

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

Actually I don't think we need to modify runpytest_subprocess at all, given we are already configuring PYTHONPATH in popen?

prepend_command = (

Check warning on line 1521 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1521

Added line #L1521 was not covered by tests
f"import sys; sys.path.insert(0, {self._syspath_prepended!r});"
)
pytest_command = [

Check warning on line 1524 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1524

Added line #L1524 was not covered by tests
python_executable,
"-c",
f"{prepend_command} import pytest; pytest.main({list(args)!r})",
]
else:
pytest_command.extend(str(arg) for arg in args)

return self.run(*pytest_command, timeout=timeout, env=env)

Check warning on line 1532 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L1532

Added line #L1532 was not covered by tests

def spawn_pytest(self, string: str, expect_timeout: float = 10.0) -> pexpect.spawn:
"""Run pytest using pexpect.
Expand Down
34 changes: 34 additions & 0 deletions testing/test_pytester.py
Original file line number Diff line number Diff line change
Expand Up @@ -835,3 +835,37 @@ def test_two():
result.assert_outcomes(passed=1, deselected=1)
# If deselected is not passed, it is not checked at all.
result.assert_outcomes(passed=1)


def test_syspathinsert__in_process__path_exists(pytester: Pytester):
some_dir = "abcd"
pytester.syspathinsert(some_dir)
pytester.makepyfile(
f"""
import sys

def test_foo():
assert "{some_dir}" in sys.path
"""
)

result = pytester.runpytest_inprocess()

result.assert_outcomes(passed=1)


def test_syspathinsert__sub_process__path_exists(pytester: Pytester):
some_dir = "abcd"
pytester.syspathinsert(some_dir)
pytester.makepyfile(
f"""
import sys

def test_foo():
assert "{some_dir}" in sys.path
"""
)

result = pytester.runpytest_subprocess(timeout=1)
Oreldm marked this conversation as resolved.
Show resolved Hide resolved

result.assert_outcomes(passed=1)
Loading