diff --git a/changelog/5965.breaking.rst b/changelog/5965.breaking.rst new file mode 100644 index 00000000000..3ecb9486aed --- /dev/null +++ b/changelog/5965.breaking.rst @@ -0,0 +1,9 @@ +symlinks are no longer resolved during collection and matching `conftest.py` files with test file paths. + +Resolving symlinks for the current directory and during collection was introduced as a bugfix in 3.9.0, but it actually is a new feature which had unfortunate consequences in Windows and surprising results in other platforms. + +The team decided to step back on resolving symlinks at all, planning to review this in the future with a more solid solution (see discussion in +`#6523 `__ for details). + +This might break test suites which made use of this feature; the fix is to create a symlink +for the entire test tree, and not only to partial files/tress as it was possible previously. diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 98ba878b3f0..04104128456 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -123,9 +123,9 @@ def _py36_windowsconsoleio_workaround(stream: TextIO) -> None: return buffered = hasattr(stream.buffer, "raw") - raw_stdout = stream.buffer.raw if buffered else stream.buffer + raw_stdout = stream.buffer.raw if buffered else stream.buffer # type: ignore[attr-defined] - if not isinstance(raw_stdout, io._WindowsConsoleIO): + if not isinstance(raw_stdout, io._WindowsConsoleIO): # type: ignore[attr-defined] return def _reopen_stdio(f, mode): @@ -135,7 +135,7 @@ def _reopen_stdio(f, mode): buffering = -1 return io.TextIOWrapper( - open(os.dup(f.fileno()), mode, buffering), + open(os.dup(f.fileno()), mode, buffering), # type: ignore[arg-type] f.encoding, f.errors, f.newlines, diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 27083900dfa..d469674c5fd 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -233,7 +233,7 @@ def get_config(args=None, plugins=None): config = Config( pluginmanager, invocation_params=Config.InvocationParams( - args=args or (), plugins=plugins, dir=Path().resolve() + args=args or (), plugins=plugins, dir=Path.cwd() ), ) @@ -478,7 +478,7 @@ def _getconftestmodules(self, path): # and allow users to opt into looking into the rootdir parent # directories instead of requiring to specify confcutdir clist = [] - for parent in directory.realpath().parts(): + for parent in directory.parts(): if self._confcutdir and self._confcutdir.relto(parent): continue conftestpath = parent.join("conftest.py") @@ -799,7 +799,7 @@ def __init__( if invocation_params is None: invocation_params = self.InvocationParams( - args=(), plugins=None, dir=Path().resolve() + args=(), plugins=None, dir=Path.cwd() ) self.option = argparse.Namespace() diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index fa7e3e1df16..05f0ecb6a47 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1496,7 +1496,7 @@ def getfixtureinfo( def pytest_plugin_registered(self, plugin: _PluggyPlugin) -> None: nodeid = None try: - p = py.path.local(plugin.__file__).realpath() # type: ignore[attr-defined] # noqa: F821 + p = py.path.local(plugin.__file__) # type: ignore[attr-defined] # noqa: F821 except AttributeError: pass else: diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 1c1cda18bdf..84ee008812f 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -665,7 +665,6 @@ def _parsearg(self, arg: str) -> Tuple[py.path.local, List[str]]: "file or package not found: " + arg + " (missing __init__.py?)" ) raise UsageError("file not found: " + arg) - fspath = fspath.realpath() return (fspath, parts) def matchnodes( diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 6878965e0c5..69f490a1d48 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -18,6 +18,7 @@ from typing import TypeVar from typing import Union +from _pytest.outcomes import skip from _pytest.warning_types import PytestWarning if sys.version_info[:2] >= (3, 6): @@ -397,3 +398,11 @@ def fnmatch_ex(pattern: str, path) -> bool: def parts(s: str) -> Set[str]: parts = s.split(sep) return {sep.join(parts[: i + 1]) or sep for i in range(len(parts))} + + +def symlink_or_skip(src, dst, **kwargs): + """Makes a symlink or skips the test in case symlinks are not supported.""" + try: + os.symlink(str(src), str(dst), **kwargs) + except OSError as e: + skip("symlinks not supported: {}".format(e)) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index e2df92d8091..7dfd588a02b 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -1,6 +1,5 @@ import os import sys -import textwrap import types import attr @@ -9,6 +8,7 @@ import pytest from _pytest.compat import importlib_metadata from _pytest.config import ExitCode +from _pytest.pathlib import symlink_or_skip from _pytest.pytester import Testdir @@ -266,29 +266,6 @@ def test_conftest_printing_shows_if_error(self, testdir): assert result.ret != 0 assert "should be seen" in result.stdout.str() - @pytest.mark.skipif( - not hasattr(py.path.local, "mksymlinkto"), - reason="symlink not available on this platform", - ) - def test_chdir(self, testdir): - testdir.tmpdir.join("py").mksymlinkto(py._pydir) - p = testdir.tmpdir.join("main.py") - p.write( - textwrap.dedent( - """\ - import sys, os - sys.path.insert(0, '') - import py - print(py.__file__) - print(py.__path__) - os.chdir(os.path.dirname(os.getcwd())) - print(py.log) - """ - ) - ) - result = testdir.runpython(p) - assert not result.ret - def test_issue109_sibling_conftests_not_loaded(self, testdir): sub1 = testdir.mkdir("sub1") sub2 = testdir.mkdir("sub2") @@ -762,19 +739,9 @@ def test(): def test_cmdline_python_package_symlink(self, testdir, monkeypatch): """ - test --pyargs option with packages with path containing symlink can - have conftest.py in their package (#2985) + --pyargs with packages with path containing symlink can have conftest.py in + their package (#2985) """ - # dummy check that we can actually create symlinks: on Windows `os.symlink` is available, - # but normal users require special admin privileges to create symlinks. - if sys.platform == "win32": - try: - os.symlink( - str(testdir.tmpdir.ensure("tmpfile")), - str(testdir.tmpdir.join("tmpfile2")), - ) - except OSError as e: - pytest.skip(str(e.args[0])) monkeypatch.delenv("PYTHONDONTWRITEBYTECODE", raising=False) dirname = "lib" @@ -790,13 +757,13 @@ def test_cmdline_python_package_symlink(self, testdir, monkeypatch): "import pytest\n@pytest.fixture\ndef a_fixture():pass" ) - d_local = testdir.mkdir("local") - symlink_location = os.path.join(str(d_local), "lib") - os.symlink(str(d), symlink_location, target_is_directory=True) + d_local = testdir.mkdir("symlink_root") + symlink_location = d_local / "lib" + symlink_or_skip(d, symlink_location, target_is_directory=True) # The structure of the test directory is now: # . - # ├── local + # ├── symlink_root # │ └── lib -> ../lib # └── lib # └── foo @@ -807,32 +774,23 @@ def test_cmdline_python_package_symlink(self, testdir, monkeypatch): # └── test_bar.py # NOTE: the different/reversed ordering is intentional here. - search_path = ["lib", os.path.join("local", "lib")] + search_path = ["lib", os.path.join("symlink_root", "lib")] monkeypatch.setenv("PYTHONPATH", prepend_pythonpath(*search_path)) for p in search_path: monkeypatch.syspath_prepend(p) # module picked up in symlink-ed directory: - # It picks up local/lib/foo/bar (symlink) via sys.path. + # It picks up symlink_root/lib/foo/bar (symlink) via sys.path. result = testdir.runpytest("--pyargs", "-v", "foo.bar") testdir.chdir() assert result.ret == 0 - if hasattr(py.path.local, "mksymlinkto"): - result.stdout.fnmatch_lines( - [ - "lib/foo/bar/test_bar.py::test_bar PASSED*", - "lib/foo/bar/test_bar.py::test_other PASSED*", - "*2 passed*", - ] - ) - else: - result.stdout.fnmatch_lines( - [ - "*lib/foo/bar/test_bar.py::test_bar PASSED*", - "*lib/foo/bar/test_bar.py::test_other PASSED*", - "*2 passed*", - ] - ) + result.stdout.fnmatch_lines( + [ + "symlink_root/lib/foo/bar/test_bar.py::test_bar PASSED*", + "symlink_root/lib/foo/bar/test_bar.py::test_other PASSED*", + "*2 passed*", + ] + ) def test_cmdline_python_package_not_exists(self, testdir): result = testdir.runpytest("--pyargs", "tpkgwhatv") diff --git a/testing/test_collection.py b/testing/test_collection.py index 8e5d5aaccb0..6644881ea44 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -3,12 +3,11 @@ import sys import textwrap -import py - import pytest from _pytest.config import ExitCode from _pytest.main import _in_venv from _pytest.main import Session +from _pytest.pathlib import symlink_or_skip from _pytest.pytester import Testdir @@ -1164,29 +1163,21 @@ def test_collect_pyargs_with_testpaths(testdir, monkeypatch): result.stdout.fnmatch_lines(["*1 passed in*"]) -@pytest.mark.skipif( - not hasattr(py.path.local, "mksymlinkto"), - reason="symlink not available on this platform", -) def test_collect_symlink_file_arg(testdir): - """Test that collecting a direct symlink, where the target does not match python_files works (#4325).""" + """Collect a direct symlink works even if it does not match python_files (#4325).""" real = testdir.makepyfile( real=""" def test_nodeid(request): - assert request.node.nodeid == "real.py::test_nodeid" + assert request.node.nodeid == "symlink.py::test_nodeid" """ ) symlink = testdir.tmpdir.join("symlink.py") - symlink.mksymlinkto(real) + symlink_or_skip(real, symlink) result = testdir.runpytest("-v", symlink) - result.stdout.fnmatch_lines(["real.py::test_nodeid PASSED*", "*1 passed in*"]) + result.stdout.fnmatch_lines(["symlink.py::test_nodeid PASSED*", "*1 passed in*"]) assert result.ret == 0 -@pytest.mark.skipif( - not hasattr(py.path.local, "mksymlinkto"), - reason="symlink not available on this platform", -) def test_collect_symlink_out_of_tree(testdir): """Test collection of symlink via out-of-tree rootdir.""" sub = testdir.tmpdir.join("sub") @@ -1204,7 +1195,7 @@ def test_nodeid(request): out_of_tree = testdir.tmpdir.join("out_of_tree").ensure(dir=True) symlink_to_sub = out_of_tree.join("symlink_to_sub") - symlink_to_sub.mksymlinkto(sub) + symlink_or_skip(sub, symlink_to_sub) sub.chdir() result = testdir.runpytest("-vs", "--rootdir=%s" % sub, symlink_to_sub) result.stdout.fnmatch_lines( @@ -1270,22 +1261,19 @@ def test_collect_pkg_init_only(testdir): result.stdout.fnmatch_lines(["sub/__init__.py::test_init PASSED*", "*1 passed in*"]) -@pytest.mark.skipif( - not hasattr(py.path.local, "mksymlinkto"), - reason="symlink not available on this platform", -) @pytest.mark.parametrize("use_pkg", (True, False)) def test_collect_sub_with_symlinks(use_pkg, testdir): + """Collection works with symlinked files and broken symlinks""" sub = testdir.mkdir("sub") if use_pkg: sub.ensure("__init__.py") - sub.ensure("test_file.py").write("def test_file(): pass") + sub.join("test_file.py").write("def test_file(): pass") # Create a broken symlink. - sub.join("test_broken.py").mksymlinkto("test_doesnotexist.py") + symlink_or_skip("test_doesnotexist.py", sub.join("test_broken.py")) # Symlink that gets collected. - sub.join("test_symlink.py").mksymlinkto("test_file.py") + symlink_or_skip("test_file.py", sub.join("test_symlink.py")) result = testdir.runpytest("-v", str(sub)) result.stdout.fnmatch_lines( diff --git a/testing/test_conftest.py b/testing/test_conftest.py index a07af60f6f9..0df303bc7cb 100644 --- a/testing/test_conftest.py +++ b/testing/test_conftest.py @@ -7,6 +7,7 @@ from _pytest.config import ExitCode from _pytest.config import PytestPluginManager from _pytest.pathlib import Path +from _pytest.pathlib import symlink_or_skip def ConftestWithSetinitial(path): @@ -190,16 +191,25 @@ def pytest_addoption(parser): result.stdout.no_fnmatch_line("*warning: could not load initial*") -@pytest.mark.skipif( - not hasattr(py.path.local, "mksymlinkto"), - reason="symlink not available on this platform", -) def test_conftest_symlink(testdir): - """Ensure that conftest.py is used for resolved symlinks.""" + """ + conftest.py discovery follows normal path resolution and does not resolve symlinks. + """ + # Structure: + # /real + # /real/conftest.py + # /real/app + # /real/app/tests + # /real/app/tests/test_foo.py + + # Links: + # /symlinktests -> /real/app/tests (running at symlinktests should fail) + # /symlink -> /real (running at /symlink should work) + real = testdir.tmpdir.mkdir("real") realtests = real.mkdir("app").mkdir("tests") - testdir.tmpdir.join("symlinktests").mksymlinkto(realtests) - testdir.tmpdir.join("symlink").mksymlinkto(real) + symlink_or_skip(realtests, testdir.tmpdir.join("symlinktests")) + symlink_or_skip(real, testdir.tmpdir.join("symlink")) testdir.makepyfile( **{ "real/app/tests/test_foo.py": "def test1(fixture): pass", @@ -216,38 +226,20 @@ def fixture(): ), } ) + + # Should fail because conftest cannot be found from the link structure. result = testdir.runpytest("-vs", "symlinktests") - result.stdout.fnmatch_lines( - [ - "*conftest_loaded*", - "real/app/tests/test_foo.py::test1 fixture_used", - "PASSED", - ] - ) - assert result.ret == ExitCode.OK + result.stdout.fnmatch_lines(["*fixture 'fixture' not found*"]) + assert result.ret == ExitCode.TESTS_FAILED # Should not cause "ValueError: Plugin already registered" (#4174). result = testdir.runpytest("-vs", "symlink") assert result.ret == ExitCode.OK - realtests.ensure("__init__.py") - result = testdir.runpytest("-vs", "symlinktests/test_foo.py::test1") - result.stdout.fnmatch_lines( - [ - "*conftest_loaded*", - "real/app/tests/test_foo.py::test1 fixture_used", - "PASSED", - ] - ) - assert result.ret == ExitCode.OK - -@pytest.mark.skipif( - not hasattr(py.path.local, "mksymlinkto"), - reason="symlink not available on this platform", -) def test_conftest_symlink_files(testdir): - """Check conftest.py loading when running in directory with symlinks.""" + """Symlinked conftest.py are found when pytest is executed in a directory with symlinked + files.""" real = testdir.tmpdir.mkdir("real") source = { "app/test_foo.py": "def test1(fixture): pass", @@ -271,7 +263,7 @@ def fixture(): build = testdir.tmpdir.mkdir("build") build.mkdir("app") for f in source: - build.join(f).mksymlinkto(real.join(f)) + symlink_or_skip(real.join(f), build.join(f)) build.chdir() result = testdir.runpytest("-vs", "app/test_foo.py") result.stdout.fnmatch_lines(["*conftest_loaded*", "PASSED"]) diff --git a/testing/test_link_resolve.py b/testing/test_link_resolve.py new file mode 100644 index 00000000000..3e9199dff56 --- /dev/null +++ b/testing/test_link_resolve.py @@ -0,0 +1,85 @@ +import os.path +import subprocess +import sys +import textwrap +from contextlib import contextmanager +from string import ascii_lowercase + +import py.path + +from _pytest import pytester + + +@contextmanager +def subst_path_windows(filename): + for c in ascii_lowercase[7:]: # Create a subst drive from H-Z. + c += ":" + if not os.path.exists(c): + drive = c + break + else: + raise AssertionError("Unable to find suitable drive letter for subst.") + + directory = filename.dirpath() + basename = filename.basename + + args = ["subst", drive, str(directory)] + subprocess.check_call(args) + assert os.path.exists(drive) + try: + filename = py.path.local(drive) / basename + yield filename + finally: + args = ["subst", "/D", drive] + subprocess.check_call(args) + + +@contextmanager +def subst_path_linux(filename): + directory = filename.dirpath() + basename = filename.basename + + target = directory / ".." / "sub2" + os.symlink(str(directory), str(target), target_is_directory=True) + try: + filename = target / basename + yield filename + finally: + # We don't need to unlink (it's all in the tempdir). + pass + + +def test_link_resolve(testdir: pytester.Testdir) -> None: + """ + See: https://github.com/pytest-dev/pytest/issues/5965 + """ + sub1 = testdir.mkpydir("sub1") + p = sub1.join("test_foo.py") + p.write( + textwrap.dedent( + """ + import pytest + def test_foo(): + raise AssertionError() + """ + ) + ) + + subst = subst_path_linux + if sys.platform == "win32": + subst = subst_path_windows + + with subst(p) as subst_p: + result = testdir.runpytest(str(subst_p), "-v") + # i.e.: Make sure that the error is reported as a relative path, not as a + # resolved path. + # See: https://github.com/pytest-dev/pytest/issues/5965 + stdout = result.stdout.str() + assert "sub1/test_foo.py" not in stdout + + # i.e.: Expect drive on windows because we just have drive:filename, whereas + # we expect a relative path on Linux. + expect = ( + "*{}*".format(subst_p) if sys.platform == "win32" else "*sub2/test_foo.py*" + ) + result.stdout.fnmatch_lines([expect])