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

FSCollector: use session._node_location_to_relpath #6701

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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 changelog/6701.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Node ids for paths outside of the rootdir are generated properly, e.g. for ``pytest testing --rootdir=/tmp -vv``.
20 changes: 8 additions & 12 deletions src/_pytest/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,12 +422,6 @@ def _prunetraceback(self, excinfo):
excinfo.traceback = ntraceback.filter()


def _check_initialpaths_for_relpath(session, fspath):
for initial_path in session._initialpaths:
if fspath.common(initial_path) == initial_path:
return fspath.relto(initial_path)


class FSHookProxy:
def __init__(
self, fspath: py.path.local, pm: PytestPluginManager, remove_mods
Expand All @@ -444,7 +438,12 @@ def __getattr__(self, name: str):

class FSCollector(Collector):
def __init__(
self, fspath: py.path.local, parent=None, config=None, session=None, nodeid=None
self,
fspath: py.path.local,
parent=None,
config=None,
session=None,
nodeid: Optional[str] = None,
) -> None:
name = fspath.basename
if parent is not None:
Expand All @@ -457,11 +456,8 @@ def __init__(
session = session or parent.session

if nodeid is None:
nodeid = self.fspath.relto(session.config.rootdir)

if not nodeid:
nodeid = _check_initialpaths_for_relpath(session, fspath)
if nodeid and os.sep != SEP:
nodeid = session._node_location_to_relpath(self.fspath)
if os.sep != SEP:
nodeid = nodeid.replace(os.sep, SEP)

super().__init__(name, parent, config, session, nodeid=nodeid, fspath=fspath)
Expand Down
11 changes: 8 additions & 3 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,9 @@ def test_cmdline_python_namespace_package(self, testdir, monkeypatch):
assert result.ret == 0
result.stdout.fnmatch_lines(
[
"test_hello.py::test_hello*PASSED*",
"test_hello.py::test_other*PASSED*",
"rootdir: {}/world".format(testdir.tmpdir),
"../hello/ns_pkg/hello/test_hello.py::test_hello PASSED *",
Copy link
Member

Choose a reason for hiding this comment

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

this indicates a breaking change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Node ids are documented to be relative to the rootdir, so this looks like a bugfix to me.

Copy link
Member

Choose a reason for hiding this comment

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

some bugfixes are breaking changes, based on the required output change here, this is a breaking change

i wll have to crosscheck as im realatively sure that this change would break reporting continuitiy in at least one work project

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"../hello/ns_pkg/hello/test_hello.py::test_other PASSED *",
"ns_pkg/world/test_world.py::test_world*PASSED*",
"ns_pkg/world/test_world.py::test_other*PASSED*",
"*4 passed in*",
Expand All @@ -745,7 +746,11 @@ def test_cmdline_python_namespace_package(self, testdir, monkeypatch):
)
assert result.ret == 0
result.stdout.fnmatch_lines(
["*test_world.py::test_other*PASSED*", "*1 passed*"]
[
"rootdir: {}".format(testdir.tmpdir),
"world/ns_pkg/world/test_world.py::test_other PASSED *",
"=* 1 passed in *=",
]
)

def test_invoke_test_and_doctestmodules(self, testdir):
Expand Down
4 changes: 3 additions & 1 deletion testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -3690,6 +3690,7 @@ def test_foo(request):
result = testdir.runpytest()
result.stdout.fnmatch_lines(
[
"test_foos.py F *",
"The requested fixture has no parameter defined for test:",
" test_foos.py::test_foo",
"",
Expand All @@ -3707,8 +3708,9 @@ def test_foo(request):
result = testdir.runpytest("--rootdir", rootdir, tests_dir)
result.stdout.fnmatch_lines(
[
"../tests/test_foos.py F *",
"The requested fixture has no parameter defined for test:",
" test_foos.py::test_foo",
" ../tests/test_foos.py::test_foo",
"",
"Requested fixture 'fix_with_param' defined in:",
"{}:4".format(fixfile),
Expand Down
9 changes: 8 additions & 1 deletion testing/test_conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,14 @@ def fixture():
build.join(f).mksymlinkto(real.join(f))
build.chdir()
result = testdir.runpytest("-vs", "app/test_foo.py")
result.stdout.fnmatch_lines(["*conftest_loaded*", "PASSED"])
result.stdout.fnmatch_lines(
[
"conftest_loaded",
"../real/app/test_foo.py::test1 fixture_used",
"PASSED",
"*= 1 passed in *",
]
)
assert result.ret == ExitCode.OK


Expand Down
22 changes: 0 additions & 22 deletions testing/test_nodes.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import py

import pytest
from _pytest import nodes

Expand Down Expand Up @@ -38,23 +36,3 @@ def test():
)
with pytest.raises(ValueError, match=".*instance of PytestWarning.*"):
items[0].warn(UserWarning("some warning"))


def test__check_initialpaths_for_relpath():
"""Ensure that it handles dirs, and does not always use dirname."""
cwd = py.path.local()

class FakeSession:
_initialpaths = [cwd]

assert nodes._check_initialpaths_for_relpath(FakeSession, cwd) == ""

sub = cwd.join("file")

class FakeSession:
_initialpaths = [cwd]

assert nodes._check_initialpaths_for_relpath(FakeSession, sub) == "file"

outside = py.path.local("/outside")
assert nodes._check_initialpaths_for_relpath(FakeSession, outside) is None
24 changes: 14 additions & 10 deletions testing/test_session.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import os

import pytest
from _pytest.config import ExitCode
from _pytest.pytester import Testdir


class SessionTests:
Expand Down Expand Up @@ -334,7 +337,7 @@ def pytest_sessionfinish():


@pytest.mark.parametrize("path", ["root", "{relative}/root", "{environment}/root"])
def test_rootdir_option_arg(testdir, monkeypatch, path):
def test_rootdir_option_arg(testdir: Testdir, monkeypatch, path: str) -> None:
monkeypatch.setenv("PY_ROOTDIR_PATH", str(testdir.tmpdir))
path = path.format(relative=str(testdir.tmpdir), environment="$PY_ROOTDIR_PATH")

Expand All @@ -344,18 +347,19 @@ def test_rootdir_option_arg(testdir, monkeypatch, path):
"""
import os
def test_one():
assert 1
"""
assert os.getcwd() == {!r}
""".format(
os.getcwd()
)
)

expected = [
"*rootdir: {}/root".format(testdir.tmpdir),
"test_rootdir_option_arg.py *",
"*1 passed*",
]
result = testdir.runpytest("--rootdir={}".format(path))
result.stdout.fnmatch_lines(
[
"*rootdir: {}/root".format(testdir.tmpdir),
"root/test_rootdir_option_arg.py *",
"*1 passed*",
]
)
result.stdout.fnmatch_lines(expected)


def test_rootdir_wrong_option_arg(testdir):
Expand Down