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 collection failures due to permission errors when using --pyargs #12043

Merged
merged 5 commits into from
Mar 2, 2024
Merged
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions changelog/11904.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed a regression in pytest 8.0.0 that would cause test collection to fail due to permission errors when using ``--pyargs``.

This change improves the collection tree for tests specified using ``--pyargs``, see :pull:`12043` for a comparison with pytest 8.0 and <8.
93 changes: 68 additions & 25 deletions src/_pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import fnmatch
import functools
import importlib
import importlib.util
import os
from pathlib import Path
import sys
Expand Down Expand Up @@ -563,7 +564,7 @@
self._initialpaths: FrozenSet[Path] = frozenset()
self._initialpaths_with_parents: FrozenSet[Path] = frozenset()
self._notfound: List[Tuple[str, Sequence[nodes.Collector]]] = []
self._initial_parts: List[Tuple[Path, List[str]]] = []
self._initial_parts: List[CollectionArgument] = []
self._collection_cache: Dict[nodes.Collector, CollectReport] = {}
self.items: List[nodes.Item] = []

Expand Down Expand Up @@ -769,15 +770,15 @@
initialpaths: List[Path] = []
initialpaths_with_parents: List[Path] = []
for arg in args:
fspath, parts = resolve_collection_argument(
collection_argument = resolve_collection_argument(
self.config.invocation_params.dir,
arg,
as_pypath=self.config.option.pyargs,
)
self._initial_parts.append((fspath, parts))
initialpaths.append(fspath)
initialpaths_with_parents.append(fspath)
initialpaths_with_parents.extend(fspath.parents)
self._initial_parts.append(collection_argument)
initialpaths.append(collection_argument.path)
initialpaths_with_parents.append(collection_argument.path)
initialpaths_with_parents.extend(collection_argument.path.parents)
self._initialpaths = frozenset(initialpaths)
self._initialpaths_with_parents = frozenset(initialpaths_with_parents)

Expand Down Expand Up @@ -839,29 +840,43 @@

pm = self.config.pluginmanager

for argpath, names in self._initial_parts:
self.trace("processing argument", (argpath, names))
for collection_argument in self._initial_parts:
self.trace("processing argument", collection_argument)
self.trace.root.indent += 1

argpath = collection_argument.path
names = collection_argument.parts
module_name = collection_argument.module_name

# resolve_collection_argument() ensures this.
if argpath.is_dir():
assert not names, f"invalid arg {(argpath, names)!r}"

# Match the argpath from the root, e.g.
paths = [argpath]
# Add relevant parents of the path, from the root, e.g.
# /a/b/c.py -> [/, /a, /a/b, /a/b/c.py]
paths = [*reversed(argpath.parents), argpath]
# Paths outside of the confcutdir should not be considered, unless
# it's the argpath itself.
while len(paths) > 1 and not pm._is_in_confcutdir(paths[0]):
paths = paths[1:]
if module_name is None:
# Paths outside of the confcutdir should not be considered.
for path in argpath.parents:
if not pm._is_in_confcutdir(path):
break
paths.insert(0, path)
else:
# For --pyargs arguments, only consider paths matching the module
# name. Paths beyond the package hierarchy are not included.
module_name_parts = module_name.split(".")
for i, path in enumerate(argpath.parents, 2):
if i > len(module_name_parts) or path.stem != module_name_parts[-i]:
break
paths.insert(0, path)

Check warning on line 871 in src/_pytest/main.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/main.py#L871

Added line #L871 was not covered by tests

# Start going over the parts from the root, collecting each level
# and discarding all nodes which don't match the level's part.
any_matched_in_initial_part = False
notfound_collectors = []
work: List[
Tuple[Union[nodes.Collector, nodes.Item], List[Union[Path, str]]]
] = [(self, paths + names)]
] = [(self, [*paths, *names])]
while work:
matchnode, matchparts = work.pop()

Expand Down Expand Up @@ -953,44 +968,64 @@
node.ihook.pytest_collectreport(report=rep)


def search_pypath(module_name: str) -> str:
"""Search sys.path for the given a dotted module name, and return its file system path."""
def search_pypath(module_name: str) -> Optional[str]:
"""Search sys.path for the given a dotted module name, and return its file
system path if found."""
try:
spec = importlib.util.find_spec(module_name)
# AttributeError: looks like package module, but actually filename
# ImportError: module does not exist
# ValueError: not a module name
except (AttributeError, ImportError, ValueError):
return module_name
return None

Check warning on line 980 in src/_pytest/main.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/main.py#L980

Added line #L980 was not covered by tests
if spec is None or spec.origin is None or spec.origin == "namespace":
return module_name
return None

Check warning on line 982 in src/_pytest/main.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/main.py#L982

Added line #L982 was not covered by tests
elif spec.submodule_search_locations:
return os.path.dirname(spec.origin)
else:
return spec.origin


@dataclasses.dataclass(frozen=True)
class CollectionArgument:
"""A resolved collection argument."""

path: Path
parts: Sequence[str]
module_name: Optional[str]


def resolve_collection_argument(
invocation_path: Path, arg: str, *, as_pypath: bool = False
) -> Tuple[Path, List[str]]:
) -> CollectionArgument:
"""Parse path arguments optionally containing selection parts and return (fspath, names).

Command-line arguments can point to files and/or directories, and optionally contain
parts for specific tests selection, for example:

"pkg/tests/test_foo.py::TestClass::test_foo"

This function ensures the path exists, and returns a tuple:
This function ensures the path exists, and returns a resolved `CollectionArgument`:

(Path("/full/path/to/pkg/tests/test_foo.py"), ["TestClass", "test_foo"])
CollectionArgument(
path=Path("/full/path/to/pkg/tests/test_foo.py"),
parts=["TestClass", "test_foo"],
module_name=None,
)

When as_pypath is True, expects that the command-line argument actually contains
module paths instead of file-system paths:

"pkg.tests.test_foo::TestClass::test_foo"

In which case we search sys.path for a matching module, and then return the *path* to the
found module.
found module, which may look like this:

CollectionArgument(
path=Path("/home/u/myvenv/lib/site-packages/pkg/tests/test_foo.py"),
parts=["TestClass", "test_foo"],
module_name="pkg.tests.test_foo",
)

If the path doesn't exist, raise UsageError.
If the path is a directory and selection parts are present, raise UsageError.
Expand All @@ -999,8 +1034,12 @@
strpath, *parts = base.split("::")
if parts:
parts[-1] = f"{parts[-1]}{squacket}{rest}"
module_name = None
if as_pypath:
strpath = search_pypath(strpath)
pyarg_strpath = search_pypath(strpath)
if pyarg_strpath is not None:
module_name = strpath
strpath = pyarg_strpath
fspath = invocation_path / strpath
fspath = absolutepath(fspath)
if not safe_exists(fspath):
Expand All @@ -1017,4 +1056,8 @@
else "directory argument cannot contain :: selection parts: {arg}"
)
raise UsageError(msg.format(arg=arg))
return fspath, parts
return CollectionArgument(
path=fspath,
parts=parts,
module_name=module_name,
)
45 changes: 45 additions & 0 deletions testing/test_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -1787,3 +1787,48 @@ def test_collect_short_file_windows(pytester: Pytester) -> None:
test_file.write_text("def test(): pass", encoding="UTF-8")
result = pytester.runpytest(short_path)
assert result.parseoutcomes() == {"passed": 1}


def test_pyargs_collection_tree(pytester: Pytester, monkeypatch: MonkeyPatch) -> None:
"""When using `--pyargs`, the collection tree of a pyargs collection
argument should only include parents in the import path, not up to confcutdir.

Regression test for #11904.
"""
site_packages = pytester.path / "venv/lib/site-packages"
site_packages.mkdir(parents=True)
monkeypatch.syspath_prepend(site_packages)
pytester.makepyfile(
**{
"venv/lib/site-packages/pkg/__init__.py": "",
"venv/lib/site-packages/pkg/sub/__init__.py": "",
"venv/lib/site-packages/pkg/sub/test_it.py": "def test(): pass",
}
)

result = pytester.runpytest("--pyargs", "--collect-only", "pkg.sub.test_it")
assert result.ret == ExitCode.OK
result.stdout.fnmatch_lines(
[
"<Package venv/lib/site-packages/pkg>",
" <Package sub>",
" <Module test_it.py>",
" <Function test>",
],
consecutive=True,
)

# Now with an unrelated rootdir with unrelated files.
monkeypatch.chdir(tempfile.gettempdir())

result = pytester.runpytest("--pyargs", "--collect-only", "pkg.sub.test_it")
assert result.ret == ExitCode.OK
result.stdout.fnmatch_lines(
[
"<Package *pkg>",
" <Package sub>",
" <Module test_it.py>",
" <Function test>",
],
consecutive=True,
)
83 changes: 61 additions & 22 deletions testing/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from _pytest.config import ExitCode
from _pytest.config import UsageError
from _pytest.main import CollectionArgument
from _pytest.main import resolve_collection_argument
from _pytest.main import validate_basetemp
from _pytest.pytester import Pytester
Expand Down Expand Up @@ -133,26 +134,43 @@ def invocation_path(self, pytester: Pytester) -> Path:

def test_file(self, invocation_path: Path) -> None:
"""File and parts."""
assert resolve_collection_argument(invocation_path, "src/pkg/test.py") == (
invocation_path / "src/pkg/test.py",
[],
assert resolve_collection_argument(
invocation_path, "src/pkg/test.py"
) == CollectionArgument(
path=invocation_path / "src/pkg/test.py",
parts=[],
module_name=None,
)
assert resolve_collection_argument(invocation_path, "src/pkg/test.py::") == (
invocation_path / "src/pkg/test.py",
[""],
assert resolve_collection_argument(
invocation_path, "src/pkg/test.py::"
) == CollectionArgument(
path=invocation_path / "src/pkg/test.py",
parts=[""],
module_name=None,
)
assert resolve_collection_argument(
invocation_path, "src/pkg/test.py::foo::bar"
) == (invocation_path / "src/pkg/test.py", ["foo", "bar"])
) == CollectionArgument(
path=invocation_path / "src/pkg/test.py",
parts=["foo", "bar"],
module_name=None,
)
assert resolve_collection_argument(
invocation_path, "src/pkg/test.py::foo::bar::"
) == (invocation_path / "src/pkg/test.py", ["foo", "bar", ""])
) == CollectionArgument(
path=invocation_path / "src/pkg/test.py",
parts=["foo", "bar", ""],
module_name=None,
)

def test_dir(self, invocation_path: Path) -> None:
"""Directory and parts."""
assert resolve_collection_argument(invocation_path, "src/pkg") == (
invocation_path / "src/pkg",
[],
assert resolve_collection_argument(
invocation_path, "src/pkg"
) == CollectionArgument(
path=invocation_path / "src/pkg",
parts=[],
module_name=None,
)

with pytest.raises(
Expand All @@ -169,13 +187,24 @@ def test_pypath(self, invocation_path: Path) -> None:
"""Dotted name and parts."""
assert resolve_collection_argument(
invocation_path, "pkg.test", as_pypath=True
) == (invocation_path / "src/pkg/test.py", [])
) == CollectionArgument(
path=invocation_path / "src/pkg/test.py",
parts=[],
module_name="pkg.test",
)
assert resolve_collection_argument(
invocation_path, "pkg.test::foo::bar", as_pypath=True
) == (invocation_path / "src/pkg/test.py", ["foo", "bar"])
assert resolve_collection_argument(invocation_path, "pkg", as_pypath=True) == (
invocation_path / "src/pkg",
[],
) == CollectionArgument(
path=invocation_path / "src/pkg/test.py",
parts=["foo", "bar"],
module_name="pkg.test",
)
assert resolve_collection_argument(
invocation_path, "pkg", as_pypath=True
) == CollectionArgument(
path=invocation_path / "src/pkg",
parts=[],
module_name="pkg",
)

with pytest.raises(
Expand All @@ -186,10 +215,13 @@ def test_pypath(self, invocation_path: Path) -> None:
)

def test_parametrized_name_with_colons(self, invocation_path: Path) -> None:
ret = resolve_collection_argument(
assert resolve_collection_argument(
invocation_path, "src/pkg/test.py::test[a::b]"
) == CollectionArgument(
path=invocation_path / "src/pkg/test.py",
parts=["test[a::b]"],
module_name=None,
)
assert ret == (invocation_path / "src/pkg/test.py", ["test[a::b]"])

def test_does_not_exist(self, invocation_path: Path) -> None:
"""Given a file/module that does not exist raises UsageError."""
Expand All @@ -209,17 +241,24 @@ def test_does_not_exist(self, invocation_path: Path) -> None:
def test_absolute_paths_are_resolved_correctly(self, invocation_path: Path) -> None:
"""Absolute paths resolve back to absolute paths."""
full_path = str(invocation_path / "src")
assert resolve_collection_argument(invocation_path, full_path) == (
Path(os.path.abspath("src")),
[],
assert resolve_collection_argument(
invocation_path, full_path
) == CollectionArgument(
path=Path(os.path.abspath("src")),
parts=[],
module_name=None,
)

# ensure full paths given in the command-line without the drive letter resolve
# to the full path correctly (#7628)
drive, full_path_without_drive = os.path.splitdrive(full_path)
assert resolve_collection_argument(
invocation_path, full_path_without_drive
) == (Path(os.path.abspath("src")), [])
) == CollectionArgument(
path=Path(os.path.abspath("src")),
parts=[],
module_name=None,
)


def test_module_full_path_without_drive(pytester: Pytester) -> None:
Expand Down