From b62fd79c0cfb6caeb6f620a033415bf0571f0f3c Mon Sep 17 00:00:00 2001 From: Henk-Jaap Wagenaar Date: Wed, 6 Dec 2017 21:55:04 +0000 Subject: [PATCH 1/4] Fix issue 2985. --- AUTHORS | 1 + _pytest/main.py | 19 +++++++++++ changelog/2985.bugfix | 1 + testing/acceptance_test.py | 65 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+) create mode 100644 changelog/2985.bugfix diff --git a/AUTHORS b/AUTHORS index 44ae6aa43ab..0492a1659b9 100644 --- a/AUTHORS +++ b/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 diff --git a/_pytest/main.py b/_pytest/main.py index 25554098dac..cb3ba4b8a8c 100644 --- a/_pytest/main.py +++ b/_pytest/main.py @@ -729,6 +729,25 @@ def _tryconvertpyarg(self, x): """ import pkgutil + + if six.PY2: # python 3.4+ uses importlib instead + def find_module_patched(self, fullname, path=None): + subname = fullname.split(".")[-1] + if subname != fullname and self.path is None: + return None + if self.path is None: + path = None + else: + path = [self.path] + try: + file, filename, etc = pkgutil.imp.find_module(subname, + path) + except ImportError: + return None + return pkgutil.ImpLoader(fullname, file, filename, etc) + + pkgutil.ImpImporter.find_module = find_module_patched + try: loader = pkgutil.find_loader(x) except ImportError: diff --git a/changelog/2985.bugfix b/changelog/2985.bugfix new file mode 100644 index 00000000000..1a268c0c5e2 --- /dev/null +++ b/changelog/2985.bugfix @@ -0,0 +1 @@ +Fix conversion of pyargs to filename to not convert symlinks and not use deprecated features on Python 3. diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index a7838545b07..fe55c5f8b95 100644 --- a/testing/acceptance_test.py +++ b/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 -> ../world + # └── lib + # └── foo + # ├── __init__.py + # └── bar + # ├── __init__.py + # ├── conftest.py + # └── test_world.py + + def join_pythonpath(*dirs): + cur = py.std.os.environ.get('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 From dc196242486dc52123ca086a4cfd9b6e46f2352a Mon Sep 17 00:00:00 2001 From: Henk-Jaap Wagenaar Date: Tue, 12 Dec 2017 08:26:40 +0000 Subject: [PATCH 2/4] Improve testing comments and code of issue 2985 (symlink in pyargs). --- testing/acceptance_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index fe55c5f8b95..9678f6914c0 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -681,17 +681,17 @@ def test_cmdline_python_package_symlink(self, testdir, monkeypatch): # The structure of the test directory is now: # . # ├── local - # │ └── lib -> ../world + # │ └── lib -> ../lib # └── lib # └── foo # ├── __init__.py # └── bar # ├── __init__.py # ├── conftest.py - # └── test_world.py + # └── test_bar.py def join_pythonpath(*dirs): - cur = py.std.os.environ.get('PYTHONPATH') + cur = os.getenv('PYTHONPATH') if cur: dirs += (cur,) return os.pathsep.join(str(p) for p in dirs) From 3ca1e4b7f056bb054b0394bcfb45dc2c78ffa364 Mon Sep 17 00:00:00 2001 From: Henk-Jaap Wagenaar Date: Tue, 12 Dec 2017 08:27:08 +0000 Subject: [PATCH 3/4] Make patch for issue in pkgutil.ImpImporter local by using contextmanager. --- _pytest/main.py | 63 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/_pytest/main.py b/_pytest/main.py index cb3ba4b8a8c..0866a838ecc 100644 --- a/_pytest/main.py +++ b/_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 @@ -728,28 +730,48 @@ def _tryconvertpyarg(self, x): """Convert a dotted module name to path. """ - import pkgutil - - if six.PY2: # python 3.4+ uses importlib instead - def find_module_patched(self, fullname, path=None): - subname = fullname.split(".")[-1] - if subname != fullname and self.path is None: - return None - if self.path is None: - path = None - else: - path = [self.path] + @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: - file, filename, etc = pkgutil.imp.find_module(subname, - path) - except ImportError: - return None - return pkgutil.ImpLoader(fullname, file, filename, etc) - - pkgutil.ImpImporter.find_module = find_module_patched + yield + finally: + pkgutil.ImpImporter.find_module = old_find_module + else: + yield try: - loader = pkgutil.find_loader(x) + with _patched_find_module(): + loader = pkgutil.find_loader(x) except ImportError: return x if loader is None: @@ -757,7 +779,8 @@ def find_module_patched(self, fullname, path=None): # 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 From 1e295535c376599511bfac4521ace951af645b52 Mon Sep 17 00:00:00 2001 From: Henk-Jaap Wagenaar Date: Tue, 12 Dec 2017 09:53:06 +0000 Subject: [PATCH 4/4] Move _patched_find_module to module namespace. --- _pytest/main.py | 78 +++++++++++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 38 deletions(-) diff --git a/_pytest/main.py b/_pytest/main.py index 0866a838ecc..142240c9965 100644 --- a/_pytest/main.py +++ b/_pytest/main.py @@ -208,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 @@ -730,44 +770,6 @@ def _tryconvertpyarg(self, x): """Convert a dotted module name to path. """ - @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 try: with _patched_find_module():