Skip to content

Commit

Permalink
Merge pull request #11138 from bluetech/pkg-mod
Browse files Browse the repository at this point in the history
python: change `Package` to no longer be a `Module`/`File`
  • Loading branch information
bluetech authored Jul 28, 2023
2 parents c754da1 + a21fb87 commit 485c555
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 140 deletions.
11 changes: 11 additions & 0 deletions changelog/11137.breaking.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
:class:`pytest.Package` is no longer a :class:`pytest.Module` or :class:`pytest.File`.

The ``Package`` collector node designates a Python package, that is, a directory with an `__init__.py` file.
Previously ``Package`` was a subtype of ``pytest.Module`` (which represents a single Python module),
the module being the `__init__.py` file.
This has been deemed a design mistake (see :issue:`11137` and :issue:`7777` for details).

The ``path`` property of ``Package`` nodes now points to the package directory instead of the ``__init__.py`` file.

Note that a ``Module`` node for ``__init__.py`` (which is not a ``Package``) may still exist,
if it is picked up during collection (e.g. if you configured :confval:`python_files` to include ``__init__.py`` files).
16 changes: 16 additions & 0 deletions doc/en/deprecations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,22 @@ an appropriate period of deprecation has passed.
Some breaking changes which could not be deprecated are also listed.


:class:`pytest.Package` is no longer a :class:`pytest.Module` or :class:`pytest.File`
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. versionchanged:: 8.0

The ``Package`` collector node designates a Python package, that is, a directory with an `__init__.py` file.
Previously ``Package`` was a subtype of ``pytest.Module`` (which represents a single Python module),
the module being the `__init__.py` file.
This has been deemed a design mistake (see :issue:`11137` and :issue:`7777` for details).

The ``path`` property of ``Package`` nodes now points to the package directory instead of the ``__init__.py`` file.

Note that a ``Module`` node for ``__init__.py`` (which is not a ``Package``) may still exist,
if it is picked up during collection (e.g. if you configured :confval:`python_files` to include ``__init__.py`` files).


Collecting ``__init__.py`` files no longer collects package
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
11 changes: 2 additions & 9 deletions src/_pytest/cacheprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,7 @@ def pytest_make_collect_report(

# Use stable sort to priorize last failed.
def sort_key(node: Union[nodes.Item, nodes.Collector]) -> bool:
# Package.path is the __init__.py file, we need the directory.
if isinstance(node, Package):
path = node.path.parent
else:
path = node.path
return path in lf_paths
return node.path in lf_paths

res.result = sorted(
res.result,
Expand Down Expand Up @@ -277,9 +272,7 @@ def __init__(self, lfplugin: "LFPlugin") -> None:
def pytest_make_collect_report(
self, collector: nodes.Collector
) -> Optional[CollectReport]:
# Packages are Files, but we only want to skip test-bearing Files,
# so don't filter Packages.
if isinstance(collector, File) and not isinstance(collector, Package):
if isinstance(collector, File):
if collector.path not in self.lfplugin._last_failed_paths:
self.lfplugin._skipped_files += 1

Expand Down
5 changes: 2 additions & 3 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,8 @@ def get_scope_package(
from _pytest.python import Package

current: Optional[Union[nodes.Item, nodes.Collector]] = node
fixture_package_name = "{}/{}".format(fixturedef.baseid, "__init__.py")
while current and (
not isinstance(current, Package) or fixture_package_name != current.nodeid
not isinstance(current, Package) or current.nodeid != fixturedef.baseid
):
current = current.parent # type: ignore[assignment]
if current is None:
Expand Down Expand Up @@ -263,7 +262,7 @@ def get_parametrized_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[_K
if scope is Scope.Session:
key: _Key = (argname, param_index)
elif scope is Scope.Package:
key = (argname, param_index, item.path.parent)
key = (argname, param_index, item.path)
elif scope is Scope.Module:
key = (argname, param_index, item.path)
elif scope is Scope.Class:
Expand Down
110 changes: 51 additions & 59 deletions src/_pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
from typing import Optional
from typing import overload
from typing import Sequence
from typing import Set
from typing import Tuple
from typing import Type
from typing import TYPE_CHECKING
from typing import Union

import _pytest._code
Expand All @@ -43,6 +43,10 @@
from _pytest.runner import SetupState


if TYPE_CHECKING:
from _pytest.python import Package


def pytest_addoption(parser: Parser) -> None:
parser.addini(
"norecursedirs",
Expand Down Expand Up @@ -572,6 +576,17 @@ def _recurse(self, direntry: "os.DirEntry[str]") -> bool:
return False
return True

def _collectpackage(self, fspath: Path) -> Optional["Package"]:
from _pytest.python import Package

ihook = self.gethookproxy(fspath)
if not self.isinitpath(fspath):
if ihook.pytest_ignore_collect(collection_path=fspath, config=self.config):
return None

pkg: Package = Package.from_parent(self, path=fspath)
return pkg

def _collectfile(
self, fspath: Path, handle_dupes: bool = True
) -> Sequence[nodes.Collector]:
Expand Down Expand Up @@ -680,8 +695,6 @@ def perform_collect( # noqa: F811
return items

def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]:
from _pytest.python import Package

# Keep track of any collected nodes in here, so we don't duplicate fixtures.
node_cache1: Dict[Path, Sequence[nodes.Collector]] = {}
node_cache2: Dict[Tuple[Type[nodes.Collector], Path], nodes.Collector] = {}
Expand All @@ -691,63 +704,57 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]:
matchnodes_cache: Dict[Tuple[Type[nodes.Collector], str], CollectReport] = {}

# Directories of pkgs with dunder-init files.
pkg_roots: Dict[Path, Package] = {}
pkg_roots: Dict[Path, "Package"] = {}

pm = self.config.pluginmanager

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

# Start with a Session root, and delve to argpath item (dir or file)
# and stack all Packages found on the way.
# No point in finding packages when collecting doctests.
if not self.config.getoption("doctestmodules", False):
pm = self.config.pluginmanager
for parent in (argpath, *argpath.parents):
if not pm._is_in_confcutdir(argpath):
break

if parent.is_dir():
pkginit = parent / "__init__.py"
if pkginit.is_file() and pkginit not in node_cache1:
col = self._collectfile(pkginit, handle_dupes=False)
if col:
if isinstance(col[0], Package):
pkg_roots[parent] = col[0]
node_cache1[col[0].path] = [col[0]]
for parent in (argpath, *argpath.parents):
if not pm._is_in_confcutdir(argpath):
break

if parent.is_dir():
pkginit = parent / "__init__.py"
if pkginit.is_file() and parent not in node_cache1:
pkg = self._collectpackage(parent)
if pkg is not None:
pkg_roots[parent] = pkg
node_cache1[pkg.path] = [pkg]

# If it's a directory argument, recurse and look for any Subpackages.
# Let the Package collector deal with subnodes, don't collect here.
if argpath.is_dir():
assert not names, f"invalid arg {(argpath, names)!r}"

seen_dirs: Set[Path] = set()
for direntry in visit(argpath, self._recurse):
if not direntry.is_file():
continue
if argpath in pkg_roots:
yield pkg_roots[argpath]

for direntry in visit(argpath, self._recurse):
path = Path(direntry.path)
dirpath = path.parent

if dirpath not in seen_dirs:
# Collect packages first.
seen_dirs.add(dirpath)
pkginit = dirpath / "__init__.py"
if pkginit.exists():
for x in self._collectfile(pkginit):
if direntry.is_dir() and self._recurse(direntry):
pkginit = path / "__init__.py"
if pkginit.is_file():
pkg = self._collectpackage(path)
if pkg is not None:
yield pkg
pkg_roots[path] = pkg

elif direntry.is_file():
if path.parent in pkg_roots:
# Package handles this file.
continue
for x in self._collectfile(path):
key2 = (type(x), x.path)
if key2 in node_cache2:
yield node_cache2[key2]
else:
node_cache2[key2] = x
yield x
if isinstance(x, Package):
pkg_roots[dirpath] = x
if dirpath in pkg_roots:
# Do not collect packages here.
continue

for x in self._collectfile(path):
key2 = (type(x), x.path)
if key2 in node_cache2:
yield node_cache2[key2]
else:
node_cache2[key2] = x
yield x
else:
assert argpath.is_file()

Expand Down Expand Up @@ -806,21 +813,6 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]:
self._notfound.append((report_arg, col))
continue

# If __init__.py was the only file requested, then the matched
# node will be the corresponding Package (by default), and the
# first yielded item will be the __init__ Module itself, so
# just use that. If this special case isn't taken, then all the
# files in the package will be yielded.
if argpath.name == "__init__.py" and isinstance(matching[0], Package):
try:
yield next(iter(matching[0].collect()))
except StopIteration:
# The package collects nothing with only an __init__.py
# file in it, which gets ignored by the default
# "python_files" option.
pass
continue

yield from matching

self.trace.root.indent -= 1
Expand Down
Loading

0 comments on commit 485c555

Please sign in to comment.