From 27766e7ccbb098748a9c73d7339a7cac3b5f2db6 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sun, 9 Feb 2020 16:56:19 +0100 Subject: [PATCH 1/2] FSCollector: use session._node_location_to_relpath for nodeids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is relevant for `pytest t/foo.py --rootdir=/tmp`. Before: ``` ../../../../tmp F.sxx … t/foo.py:5: ValueError … FAILED ../../../../tmp/::test_fail - ValueError ``` This removes `_check_initialpaths_for_relpath` (added via https://github.com/pytest-dev/pytest/pull/2776 to address part of the issue), but it is apparently bad trying to make them relative to any given arg, when they are meant to be relative to `rootdir` really. --- changelog/6701.bugfix.rst | 1 + src/_pytest/nodes.py | 20 ++++++++------------ testing/acceptance_test.py | 4 ++-- testing/python/fixtures.py | 4 +++- testing/test_conftest.py | 9 ++++++++- testing/test_nodes.py | 22 ---------------------- testing/test_session.py | 24 ++++++++++++++---------- 7 files changed, 36 insertions(+), 48 deletions(-) create mode 100644 changelog/6701.bugfix.rst diff --git a/changelog/6701.bugfix.rst b/changelog/6701.bugfix.rst new file mode 100644 index 00000000000..6edb893be0d --- /dev/null +++ b/changelog/6701.bugfix.rst @@ -0,0 +1 @@ +Node ids for paths outside of the rootdir are generated properly, e.g. for ``pytest testing --rootdir=/tmp -vv``. diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 133e93df7cc..77205afff39 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -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 @@ -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: @@ -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) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index cafc87f2e7d..0502343cf2e 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -730,8 +730,8 @@ 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*", + "../hello/ns_pkg/hello/test_hello.py::test_hello PASSED *", + "../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*", diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index bfbe359515c..babf3d966a1 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -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", "", @@ -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), diff --git a/testing/test_conftest.py b/testing/test_conftest.py index a07af60f6f9..09959471d0a 100644 --- a/testing/test_conftest.py +++ b/testing/test_conftest.py @@ -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 diff --git a/testing/test_nodes.py b/testing/test_nodes.py index dbb3e2e8f64..4196708c234 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -1,5 +1,3 @@ -import py - import pytest from _pytest import nodes @@ -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 diff --git a/testing/test_session.py b/testing/test_session.py index 1f17acbbd70..be6491d300e 100644 --- a/testing/test_session.py +++ b/testing/test_session.py @@ -1,5 +1,8 @@ +import os + import pytest from _pytest.config import ExitCode +from _pytest.pytester import Testdir class SessionTests: @@ -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") @@ -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): From bcb2db271b1ce1125c562ac8be697db39c887952 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 24 Feb 2020 21:13:48 +0100 Subject: [PATCH 2/2] harden test_cmdline_python_namespace_package --- testing/acceptance_test.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 0502343cf2e..800cdcb5975 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -730,6 +730,7 @@ def test_cmdline_python_namespace_package(self, testdir, monkeypatch): assert result.ret == 0 result.stdout.fnmatch_lines( [ + "rootdir: {}/world".format(testdir.tmpdir), "../hello/ns_pkg/hello/test_hello.py::test_hello PASSED *", "../hello/ns_pkg/hello/test_hello.py::test_other PASSED *", "ns_pkg/world/test_world.py::test_world*PASSED*", @@ -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):