diff --git a/changelog/11904.bugfix.rst b/changelog/11904.bugfix.rst new file mode 100644 index 000000000..2aed9bcb0 --- /dev/null +++ b/changelog/11904.bugfix.rst @@ -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. diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 639bf26c1..b7ed72ddc 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -5,6 +5,7 @@ import dataclasses import fnmatch import functools import importlib +import importlib.util import os from pathlib import Path import sys @@ -563,7 +564,7 @@ class Session(nodes.Collector): 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] = [] @@ -769,15 +770,15 @@ class Session(nodes.Collector): 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) @@ -839,21 +840,35 @@ class Session(nodes.Collector): 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) # Start going over the parts from the root, collecting each level # and discarding all nodes which don't match the level's part. @@ -861,7 +876,7 @@ class Session(nodes.Collector): 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() @@ -953,26 +968,36 @@ class Session(nodes.Collector): 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 if spec is None or spec.origin is None or spec.origin == "namespace": - return module_name + return None 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 @@ -980,9 +1005,13 @@ def resolve_collection_argument( "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: @@ -990,7 +1019,13 @@ def resolve_collection_argument( "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. @@ -999,8 +1034,12 @@ def resolve_collection_argument( 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): @@ -1017,4 +1056,8 @@ def resolve_collection_argument( 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, + ) diff --git a/testing/test_collection.py b/testing/test_collection.py index 050740045..fbc8543e9 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -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( + [ + "", + " ", + " ", + " ", + ], + 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( + [ + "", + " ", + " ", + " ", + ], + consecutive=True, + ) diff --git a/testing/test_main.py b/testing/test_main.py index d92fa21d3..345aa1e62 100644 --- a/testing/test_main.py +++ b/testing/test_main.py @@ -8,6 +8,7 @@ from typing import Optional 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 @@ -133,26 +134,43 @@ class TestResolveCollectionArgument: 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( @@ -169,13 +187,24 @@ class TestResolveCollectionArgument: """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( @@ -186,10 +215,13 @@ class TestResolveCollectionArgument: ) 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.""" @@ -209,9 +241,12 @@ class TestResolveCollectionArgument: 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 @@ -219,7 +254,11 @@ class TestResolveCollectionArgument: 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: