Merge pull request #3010 from cryvate/fix-issue-2985

Improve handling of pyargs

Author: RonnyPfannschmidt

AUTHORS

@@ -74,6 +74,7 @@ Grig Gheorghiu
 Grigorii Eremeev (budulianin)
 Guido Wesdorp
 Harald Armin Massa
+Henk-Jaap Wagenaar
 Hugo van Kemenade
 Hui Wang (coldnight)
 Ian Bicking

_pytest/main.py

@@ -1,8 +1,10 @@
 """ core implementation of testing process: init, session, runtest loop. """
 from __future__ import absolute_import, division, print_function
 
+import contextlib
 import functools
 import os
+import pkgutil
 import six
 import sys
 
@@ -206,6 +208,46 @@ def pytest_ignore_collect(path, config):
     return False
 
 
+@contextlib.contextmanager
+def _patched_find_module():
+    """Patch bug in pkgutil.ImpImporter.find_module
+
+    When using pkgutil.find_loader on python<3.4 it removes symlinks
+    from the path due to a call to os.path.realpath. This is not consistent
+    with actually doing the import (in these versions, pkgutil and __import__
+    did not share the same underlying code). This can break conftest
+    discovery for pytest where symlinks are involved.
+
+    The only supported python<3.4 by pytest is python 2.7.
+    """
+    if six.PY2:  # python 3.4+ uses importlib instead
+        def find_module_patched(self, fullname, path=None):
+            # Note: we ignore 'path' argument since it is only used via meta_path
+            subname = fullname.split(".")[-1]
+            if subname != fullname and self.path is None:
+                return None
+            if self.path is None:
+                path = None
+            else:
+                # original: path = [os.path.realpath(self.path)]
+                path = [self.path]
+            try:
+                file, filename, etc = pkgutil.imp.find_module(subname,
+                                                              path)
+            except ImportError:
+                return None
+            return pkgutil.ImpLoader(fullname, file, filename, etc)
+
+        old_find_module = pkgutil.ImpImporter.find_module
+        pkgutil.ImpImporter.find_module = find_module_patched
+        try:
+            yield
+        finally:
+            pkgutil.ImpImporter.find_module = old_find_module
+    else:
+        yield
+
+
 class FSHookProxy:
     def __init__(self, fspath, pm, remove_mods):
         self.fspath = fspath
@@ -728,17 +770,19 @@ def _tryconvertpyarg(self, x):
         """Convert a dotted module name to path.
 
         """
-        import pkgutil
+
         try:
-            loader = pkgutil.find_loader(x)
+            with _patched_find_module():
+                loader = pkgutil.find_loader(x)
         except ImportError:
             return x
         if loader is None:
             return x
         # This method is sometimes invoked when AssertionRewritingHook, which
         # does not define a get_filename method, is already in place:
         try:
-            path = loader.get_filename(x)
+            with _patched_find_module():
+                path = loader.get_filename(x)
         except AttributeError:
             # Retrieve path from AssertionRewritingHook:
             path = loader.modules[x][0].co_filename

changelog/2985.bugfix

@@ -0,0 +1 @@
+Fix conversion of pyargs to filename to not convert symlinks and not use deprecated features on Python 3.

testing/acceptance_test.py

@@ -3,6 +3,8 @@
 import os
 import sys
 
+import six
+
 import _pytest._code
 import py
 import pytest
@@ -645,6 +647,69 @@ def join_pythonpath(*dirs):
             "*1 passed*"
         ])
 
+    @pytest.mark.skipif(not hasattr(os, "symlink"), reason="requires symlinks")
+    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)
+        """
+        monkeypatch.delenv('PYTHONDONTWRITEBYTECODE', raising=False)
+
+        search_path = ["lib", os.path.join("local", "lib")]
+
+        dirname = "lib"
+        d = testdir.mkdir(dirname)
+        foo = d.mkdir("foo")
+        foo.ensure("__init__.py")
+        lib = foo.mkdir('bar')
+        lib.ensure("__init__.py")
+        lib.join("test_bar.py"). \
+            write("def test_bar(): pass\n"
+                  "def test_other(a_fixture):pass")
+        lib.join("conftest.py"). \
+            write("import pytest\n"
+                  "@pytest.fixture\n"
+                  "def a_fixture():pass")
+
+        d_local = testdir.mkdir("local")
+        symlink_location = os.path.join(str(d_local), "lib")
+        if six.PY2:
+            os.symlink(str(d), symlink_location)
+        else:
+            os.symlink(str(d), symlink_location, target_is_directory=True)
+
+        # The structure of the test directory is now:
+        # .
+        # ├── local
+        # │   └── lib -> ../lib
+        # └── lib
+        #     └── foo
+        #         ├── __init__.py
+        #         └── bar
+        #             ├── __init__.py
+        #             ├── conftest.py
+        #             └── test_bar.py
+
+        def join_pythonpath(*dirs):
+            cur = os.getenv('PYTHONPATH')
+            if cur:
+                dirs += (cur,)
+            return os.pathsep.join(str(p) for p in dirs)
+
+        monkeypatch.setenv('PYTHONPATH', join_pythonpath(*search_path))
+        for p in search_path:
+            monkeypatch.syspath_prepend(p)
+
+        # module picked up in symlink-ed directory:
+        result = testdir.runpytest("--pyargs", "-v", "foo.bar")
+        testdir.chdir()
+        assert result.ret == 0
+        result.stdout.fnmatch_lines([
+            "*lib/foo/bar/test_bar.py::test_bar*PASSED*",
+            "*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")
         assert result.ret