Fix issue where working dir becomes wrong on subst drive on Windows. Fixes #5965 (#6523)

Co-authored-by: Bruno Oliveira <nicoddemus@gmail.com>

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 <https://github.com/pytest-dev/pytest/pull/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.

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 _py36_windowsconsoleio_workaround(stream: TextIO) -> None:
             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,

src/_pytest/config/__init__.py

@@ -232,7 +232,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()
         ),
     )
 
@@ -477,7 +477,7 @@ class PytestPluginManager(PluginManager):
         # 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")
@@ -798,7 +798,7 @@ class Config:
 
         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()

src/_pytest/fixtures.py

@@ -1496,7 +1496,7 @@ class FixtureManager:
     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:

src/_pytest/main.py

@@ -665,7 +665,6 @@ class Session(nodes.FSCollector):
                     "file or package not found: " + arg + " (missing __init__.py?)"
                 )
             raise UsageError("file not found: " + arg)
-        fspath = fspath.realpath()
         return (fspath, parts)
 
     def matchnodes(

src/_pytest/pathlib.py

@@ -18,6 +18,7 @@ from typing import Set
 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))

testing/acceptance_test.py

@@ -1,6 +1,5 @@
 import os
 import sys
-import textwrap
 import types
 
 import attr
@@ -9,6 +8,7 @@ import py
 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 @@ class TestGeneralUsage:
         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 @@ class TestInvocationVariants:
 
     def test_cmdline_python_package_symlink(self, testdir, monkeypatch):
         """
-        test --pyargs option with packages with path containing symlink can
+        --pyargs with packages with path containing symlink can have conftest.py in
-        have conftest.py in their package (#2985)
+        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 @@ class TestInvocationVariants:
             "import pytest\n@pytest.fixture\ndef a_fixture():pass"
         )
 
-        d_local = testdir.mkdir("local")
+        d_local = testdir.mkdir("symlink_root")
-        symlink_location = os.path.join(str(d_local), "lib")
+        symlink_location = d_local / "lib"
-        os.symlink(str(d), symlink_location, target_is_directory=True)
+        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,29 +774,20 @@ class TestInvocationVariants:
         #             └── 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*",
+                "symlink_root/lib/foo/bar/test_bar.py::test_bar PASSED*",
-                    "lib/foo/bar/test_bar.py::test_other PASSED*",
+                "symlink_root/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*",
             ]
         )

testing/test_collection.py

@@ -3,12 +3,11 @@ import pprint
 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_collect_symlink_out_of_tree(testdir):
 
     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(

testing/test_conftest.py

@@ -7,6 +7,7 @@ import pytest
 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 test_conftest_confcutdir(testdir):
     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)
+    symlink_or_skip(realtests, testdir.tmpdir.join("symlinktests"))
-    testdir.tmpdir.join("symlink").mksymlinkto(real)
+    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 test_conftest_symlink(testdir):
             ),
         }
     )
+
+    # Should fail because conftest cannot be found from the link structure.
     result = testdir.runpytest("-vs", "symlinktests")
-    result.stdout.fnmatch_lines(
+    result.stdout.fnmatch_lines(["*fixture 'fixture' not found*"])
-        [
+    assert result.ret == ExitCode.TESTS_FAILED
-            "*conftest_loaded*",
-            "real/app/tests/test_foo.py::test1 fixture_used",
-            "PASSED",
-        ]
-    )
-    assert result.ret == ExitCode.OK
 
     # 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 test_conftest_symlink_files(testdir):
     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"])

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])