From 1df6d28080343749114eda89aa06c7fda6fe1d7f Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 13 Sep 2018 18:50:05 -0300 Subject: [PATCH 1/4] Fix assertion rewriter crash if cwd changes mid-testing Unfortunately we need to get a `py.path.local` object to perform the fnmatch operation, it is different from the standard `fnmatch` module because it implements its own custom logic. So we need to use `py.path` to perform the fnmatch for backward compatibility reasons. Ideally we should be able to use a "pure path" in `pathlib` terms (a path not bound to the file system), but we don't have those in pylib. Fix #3973 --- changelog/3973.bugfix.rst | 1 + src/_pytest/assertion/rewrite.py | 6 +++++- testing/test_assertrewrite.py | 24 ++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 changelog/3973.bugfix.rst diff --git a/changelog/3973.bugfix.rst b/changelog/3973.bugfix.rst new file mode 100644 index 000000000..29c70b840 --- /dev/null +++ b/changelog/3973.bugfix.rst @@ -0,0 +1 @@ +Fix crash of the assertion rewriter if a test changed the current working directory without restoring it afterwards. diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 738d63396..2b3a0ab38 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -199,7 +199,11 @@ class AssertionRewritingHook(object): # For matching the name it must be as if it was a filename. parts[-1] = parts[-1] + ".py" - fn_pypath = py.path.local(os.path.sep.join(parts)) + try: + fn_pypath = py.path.local(os.path.sep.join(parts)) + except EnvironmentError: + return False + for pat in self.fnpats: # if the pattern contains subdirectories ("tests/**.py" for example) we can't bail out based # on the name alone because we need to match against the full path diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index aaf3e4785..394d30a05 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1232,3 +1232,27 @@ class TestEarlyRewriteBailout(object): hook.fnpats[:] = ["tests/**.py"] assert hook.find_module("file") is not None assert self.find_module_calls == ["file"] + + @pytest.mark.skipif( + sys.platform.startswith("win32"), reason="cannot remove cwd on Windows" + ) + def test_cwd_changed(self, testdir): + testdir.makepyfile( + **{ + "test_bar.py": """ + import os + import shutil + import tempfile + + d = tempfile.mkdtemp() + os.chdir(d) + shutil.rmtree(d) + """, + "test_foo.py": """ + def test(): + pass + """, + } + ) + result = testdir.runpytest() + result.stdout.fnmatch_lines("* 1 passed in *") From 37d24692664b0dc5166f8cfb5bbb407c211c97c1 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 17 Sep 2018 20:01:01 -0300 Subject: [PATCH 2/4] Use a PurePath instance to do matching against patterns in assertion rewrite This way we don't need to have real file system path, which prevents the original #3973 bug. --- src/_pytest/assertion/rewrite.py | 19 ++------- src/_pytest/compat.py | 14 +++++-- src/_pytest/paths.py | 39 +++++++++++++++++- testing/test_paths.py | 69 ++++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 20 deletions(-) create mode 100644 testing/test_paths.py diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 2b3a0ab38..3539bd55d 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -16,7 +16,8 @@ import atomicwrites import py from _pytest.assertion import util - +from _pytest.compat import PurePath, spec_from_file_location +from _pytest.paths import fnmatch_ex # pytest caches rewritten pycs in __pycache__. if hasattr(imp, "get_tag"): @@ -45,14 +46,6 @@ else: return ast.Call(a, b, c, None, None) -if sys.version_info >= (3, 4): - from importlib.util import spec_from_file_location -else: - - def spec_from_file_location(*_, **__): - return None - - class AssertionRewritingHook(object): """PEP302 Import hook which rewrites asserts.""" @@ -198,18 +191,14 @@ class AssertionRewritingHook(object): return False # For matching the name it must be as if it was a filename. - parts[-1] = parts[-1] + ".py" - try: - fn_pypath = py.path.local(os.path.sep.join(parts)) - except EnvironmentError: - return False + path = PurePath(os.path.sep.join(parts) + ".py") for pat in self.fnpats: # if the pattern contains subdirectories ("tests/**.py" for example) we can't bail out based # on the name alone because we need to match against the full path if os.path.dirname(pat): return False - if fn_pypath.fnmatch(pat): + if fnmatch_ex(pat, path): return False if self._is_marked_for_rewrite(name, state): diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index ea369ccf2..02cad24cc 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -23,7 +23,7 @@ except ImportError: # pragma: no cover # Only available in Python 3.4+ or as a backport enum = None -__all__ = ["Path"] +__all__ = ["Path", "PurePath"] _PY3 = sys.version_info > (3, 0) _PY2 = not _PY3 @@ -42,9 +42,9 @@ PY36 = sys.version_info[:2] >= (3, 6) MODULE_NOT_FOUND_ERROR = "ModuleNotFoundError" if PY36 else "ImportError" if PY36: - from pathlib import Path + from pathlib import Path, PurePath else: - from pathlib2 import Path + from pathlib2 import Path, PurePath if _PY3: @@ -56,6 +56,14 @@ else: from collections import Mapping, Sequence # noqa +if sys.version_info >= (3, 4): + from importlib.util import spec_from_file_location +else: + + def spec_from_file_location(*_, **__): + return None + + def _format_args(func): return str(signature(func)) diff --git a/src/_pytest/paths.py b/src/_pytest/paths.py index 7c0dc1ec1..52b2392d0 100644 --- a/src/_pytest/paths.py +++ b/src/_pytest/paths.py @@ -1,5 +1,11 @@ -from .compat import Path -from os.path import expanduser, expandvars, isabs +from os.path import expanduser, expandvars, isabs, sep +from posixpath import sep as posix_sep +import fnmatch +import sys + +import six + +from .compat import Path, PurePath def resolve_from_str(input, root): @@ -11,3 +17,32 @@ def resolve_from_str(input, root): return Path(input) else: return root.joinpath(input) + + +def fnmatch_ex(pattern, path): + """FNMatcher port from py.path.common which works with PurePath() instances. + + The difference between this algorithm and PurePath.match() is that the latter matches "**" glob expressions + for each part of the path, while this algorithm uses the whole path instead. + + For example: + "tests/foo/bar/doc/test_foo.py" matches pattern "tests/**/doc/test*.py" with this algorithm, but not with + PurePath.match(). + + This algorithm was ported to keep backward-compatibility with existing settings which assume paths match according + this logic. + """ + path = PurePath(path) + iswin32 = sys.platform.startswith("win") + + if iswin32 and sep not in pattern and posix_sep in pattern: + # Running on Windows, the pattern has no Windows path separators, + # and the pattern has one or more Posix path separators. Replace + # the Posix path separators with the Windows path separator. + pattern = pattern.replace(posix_sep, sep) + + if sep not in pattern: + name = path.name + else: + name = six.text_type(path) + return fnmatch.fnmatch(name, pattern) diff --git a/testing/test_paths.py b/testing/test_paths.py new file mode 100644 index 000000000..2bb1335fb --- /dev/null +++ b/testing/test_paths.py @@ -0,0 +1,69 @@ +import sys + +import py + +import pytest + +from _pytest.paths import fnmatch_ex + + +class TestPort: + """Test that our port of py.common.FNMatcher (fnmatch_ex) produces the same results as the + original py.path.local.fnmatch method. + """ + + @pytest.fixture(params=["pathlib", "py.path"]) + def match(self, request): + if request.param == "py.path": + + def match_(pattern, path): + return py.path.local(path).fnmatch(pattern) + + else: + assert request.param == "pathlib" + + def match_(pattern, path): + return fnmatch_ex(pattern, path) + + return match_ + + if sys.platform == "win32": + drv1 = "c:" + drv2 = "d:" + else: + drv1 = "/c" + drv2 = "/d" + + @pytest.mark.parametrize( + "pattern, path", + [ + ("*.py", "foo.py"), + ("*.py", "bar/foo.py"), + ("test_*.py", "foo/test_foo.py"), + ("tests/*.py", "tests/foo.py"), + (drv1 + "/*.py", drv1 + "/foo.py"), + (drv1 + "/foo/*.py", drv1 + "/foo/foo.py"), + ("tests/**/test*.py", "tests/foo/test_foo.py"), + ("tests/**/doc/test*.py", "tests/foo/bar/doc/test_foo.py"), + ("tests/**/doc/**/test*.py", "tests/foo/doc/bar/test_foo.py"), + ], + ) + def test_matching(self, match, pattern, path): + assert match(pattern, path) + + @pytest.mark.parametrize( + "pattern, path", + [ + ("*.py", "foo.pyc"), + ("*.py", "foo/foo.pyc"), + ("tests/*.py", "foo/foo.py"), + (drv1 + "/*.py", drv2 + "/foo.py"), + (drv1 + "/foo/*.py", drv2 + "/foo/foo.py"), + ("tests/**/test*.py", "tests/foo.py"), + ("tests/**/test*.py", "foo/test_foo.py"), + ("tests/**/doc/test*.py", "tests/foo/bar/doc/foo.py"), + ("tests/**/doc/test*.py", "tests/foo/bar/test_foo.py"), + ], + ) + def test_not_matching(self, match, pattern, path): + assert not match(pattern, path) From 1e2e65f0fa159f3a0e89fd548a64ce0b67f5d97e Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 19 Sep 2018 08:20:23 -0300 Subject: [PATCH 3/4] Add references to the relevant Python issues --- src/_pytest/paths.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/_pytest/paths.py b/src/_pytest/paths.py index 52b2392d0..e5177e231 100644 --- a/src/_pytest/paths.py +++ b/src/_pytest/paths.py @@ -31,6 +31,10 @@ def fnmatch_ex(pattern, path): This algorithm was ported to keep backward-compatibility with existing settings which assume paths match according this logic. + + References: + * https://bugs.python.org/issue29249 + * https://bugs.python.org/issue34731 """ path = PurePath(path) iswin32 = sys.platform.startswith("win") From 7f48f552c1f1e0e9a379fde629d9e6e3c9b89a23 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 19 Sep 2018 10:18:05 -0300 Subject: [PATCH 4/4] Fix linting --- src/_pytest/paths.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/paths.py b/src/_pytest/paths.py index e5177e231..031ea6b26 100644 --- a/src/_pytest/paths.py +++ b/src/_pytest/paths.py @@ -31,7 +31,7 @@ def fnmatch_ex(pattern, path): This algorithm was ported to keep backward-compatibility with existing settings which assume paths match according this logic. - + References: * https://bugs.python.org/issue29249 * https://bugs.python.org/issue34731