Skip to content

Commit

Permalink
Fix handling recursive symlinks
Browse files Browse the repository at this point in the history
When pytest was run on a directory containing a recursive symlink it failed
with ELOOP as the library was not able to determine the type of the
direntry:

src/_pytest/main.py:685: in collect
    if not direntry.is_file():
E   OSError: [Errno 40] Too many levels of symbolic links: '/home/florian/proj/pytest/tests/recursive'

This is fixed by handling ELOOP and other errors in the visit function in
pathlib.py, so the entries whose is_file() call raises an OSError with the
pre-defined list of error numbers will be exluded from the result.

The _ignore_errors function was copied from Lib/pathlib.py of cpython 3.9.

Fixes pytest-dev#7951
  • Loading branch information
csernazs committed Oct 31, 2020
1 parent b95991a commit 67682d6
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 1 deletion.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -314,3 +314,4 @@ Xuecong Liao
Yoav Caspi
Zac Hatfield-Dodds
Zoltán Máté
Zsolt Cserna
1 change: 1 addition & 0 deletions changelog/7951.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed handling of recursive symlinks when collecting tests.
39 changes: 38 additions & 1 deletion src/_pytest/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
import uuid
import warnings
from enum import Enum
from errno import EBADF
from errno import ELOOP
from errno import ENOENT
from errno import ENOTDIR
from functools import partial
from os.path import expanduser
from os.path import expandvars
Expand Down Expand Up @@ -37,6 +41,24 @@

_AnyPurePath = TypeVar("_AnyPurePath", bound=PurePath)

# The following function, variables and comments were
# copied from cpython 3.9 Lib/pathlib.py file.

# EBADF - guard against macOS `stat` throwing EBADF
_IGNORED_ERRORS = (ENOENT, ENOTDIR, EBADF, ELOOP)

_IGNORED_WINERRORS = (
21, # ERROR_NOT_READY - drive exists but is not accessible
1921, # ERROR_CANT_RESOLVE_FILENAME - fix for broken symlink pointing to itself
)


def _ignore_error(exception):
return (
getattr(exception, "errno", None) in _IGNORED_ERRORS
or getattr(exception, "winerror", None) in _IGNORED_WINERRORS
)


def get_lock_path(path: _AnyPurePath) -> _AnyPurePath:
return path.joinpath(".lock")
Expand Down Expand Up @@ -555,8 +577,23 @@ def visit(
Entries at each directory level are sorted.
"""
entries = sorted(os.scandir(path), key=lambda entry: entry.name)

# Skip entries with symlink loops and other brokenness, so the caller doesn't
# have to deal with it.
entries = []
for entry in os.scandir(path):
try:
entry.is_file()
except OSError as err:
if _ignore_error(err):
continue
raise
entries.append(entry)

entries.sort(key=lambda entry: entry.name)

yield from entries

for entry in entries:
if entry.is_dir(follow_symlinks=False) and recurse(entry):
yield from visit(entry.path, recurse)
Expand Down
14 changes: 14 additions & 0 deletions testing/test_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -1404,3 +1404,17 @@ def a(): return 4
result = testdir.runpytest()
# Not INTERNAL_ERROR
assert result.ret == ExitCode.INTERRUPTED


def test_does_not_crash_on_recursive_symlink(testdir: Testdir) -> None:
"""Regression test for an issue around recursive symlinks (#7951)."""
symlink_or_skip("recursive", str(testdir.tmpdir.join("recursive")))
testdir.makepyfile(
"""
def test_foo(): assert True
"""
)
result = testdir.runpytest()

assert result.ret == ExitCode.OK
assert result.parseoutcomes() == {"passed": 1}
13 changes: 13 additions & 0 deletions testing/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
from _pytest.pathlib import ImportPathMismatchError
from _pytest.pathlib import maybe_delete_a_numbered_dir
from _pytest.pathlib import resolve_package_path
from _pytest.pathlib import symlink_or_skip
from _pytest.pathlib import visit


class TestFNMatcherPort:
Expand Down Expand Up @@ -401,3 +403,14 @@ def test_commonpath() -> None:
assert commonpath(subpath, path) == path
assert commonpath(Path(str(path) + "suffix"), path) == path.parent
assert commonpath(path, path.parent.parent) == path.parent.parent


def test_visit_ignores_errors(tmpdir) -> None:
symlink_or_skip("recursive", str(tmpdir.join("recursive")))
tmpdir.join("foo").write_binary(b"")
tmpdir.join("bar").write_binary(b"")

assert [entry.name for entry in visit(tmpdir, recurse=lambda entry: False)] == [
"bar",
"foo",
]

0 comments on commit 67682d6

Please sign in to comment.