Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix assertion rewriter crash if cwd changes mid-testing #3980

Merged
merged 4 commits into from
Sep 19, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/3973.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix crash of the assertion rewriter if a test changed the current working directory without restoring it afterwards.
17 changes: 5 additions & 12 deletions src/_pytest/assertion/rewrite.py
Original file line number Diff line number Diff line change
@@ -16,7 +16,8 @@
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 @@ def ast_Call(a, b, c):
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,14 +191,14 @@ def _early_rewrite_bailout(self, name, state):
return False

# 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))
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):
14 changes: 11 additions & 3 deletions src/_pytest/compat.py
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@
# 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 @@
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 @@
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))

43 changes: 41 additions & 2 deletions src/_pytest/paths.py
Original file line number Diff line number Diff line change
@@ -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,36 @@ 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.

References:
* https://bugs.python.org/issue29249
* https://bugs.python.org/issue34731
"""
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)
24 changes: 24 additions & 0 deletions testing/test_assertrewrite.py
Original file line number Diff line number Diff line change
@@ -1232,3 +1232,27 @@ def test_simple_failure():
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 *")
69 changes: 69 additions & 0 deletions testing/test_paths.py
Original file line number Diff line number Diff line change
@@ -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)