From d011e352fd4bcbdde429ffacef9a3bf952dc0d2d Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 23 Jul 2026 11:17:01 +0200 Subject: [PATCH] Add opt-in FunctionDefinition collection tree node (#3926) Introduce the ``collect_function_definition`` ini option to promote the internal ``FunctionDefinition`` from a transient parametrization helper to a real collector node in the collection tree. The option is tri-state: - ``hidden`` (default): unchanged flat layout; the definition drives parametrization and is kept out of the tree. - ``pedantic``: insert the definition node between the Module/Class and its test functions; function-level markers are scoped to the definition and each invocation owns only its callspec markers. - ``messy``: migration stopgap -- insert the node but transfer the function-level markers back onto each invocation to preserve the legacy marker layout for code not yet prepared for the new scope; emits a header warning. FunctionDefinition becomes a PyCollector (no longer a Function/Item), so it is collected rather than run. Invocation node ids stay flat and identical across all modes, so selection, caching and reporting are unaffected. obj and instance resolution and getmodpath skip an interposed definition node. Co-Authored-By: Claude Opus 4.8 --- changelog/3926.improvement.rst | 15 ++ doc/en/reference/reference.rst | 81 +++++++ src/_pytest/main.py | 11 + src/_pytest/python.py | 255 +++++++++++++++----- testing/test_collect_function_definition.py | 193 +++++++++++++++ 5 files changed, 492 insertions(+), 63 deletions(-) create mode 100644 changelog/3926.improvement.rst create mode 100644 testing/test_collect_function_definition.py diff --git a/changelog/3926.improvement.rst b/changelog/3926.improvement.rst new file mode 100644 index 00000000000..98f5503663d --- /dev/null +++ b/changelog/3926.improvement.rst @@ -0,0 +1,15 @@ +Added the :confval:`collect_function_definition` option, which can promote the +internal ``FunctionDefinition`` to a real node in the collection tree. + +When set to ``pedantic``, a ``FunctionDefinition`` collector node is inserted +between a :class:`~pytest.Module`/:class:`~pytest.Class` and its test functions, +so that the (possibly parametrized) invocations of each test are collected +underneath a shared definition node instead of as flat siblings. Function-level +markers are scoped to the definition node in this mode. + +The default ``hidden`` keeps the previous flat layout, and the transitional +``messy`` value keeps the definition node out of marker resolution for code that +is not yet prepared for the new scope. + +The node ids of the individual test invocations are unchanged in all modes, so +test selection, caching and reporting behave exactly as before. diff --git a/doc/en/reference/reference.rst b/doc/en/reference/reference.rst index 4f2e0b70850..b7235d58a99 100644 --- a/doc/en/reference/reference.rst +++ b/doc/en/reference/reference.rst @@ -1378,6 +1378,79 @@ passed multiple times. The expected format is ``name=value``. For example:: variables, that will be expanded. For more information about cache plugin please refer to :ref:`cache_provider`. +.. confval:: collect_function_definition + :type: ``string`` + :default: ``hidden`` + + .. versionadded:: 9.2 + + Controls whether a ``FunctionDefinition`` collector node is inserted into the + collection tree between a :class:`~pytest.Module`/:class:`~pytest.Class` and + its test functions, grouping the (possibly parametrized) invocations of a test + function *underneath* the definition node instead of as flat siblings. + + It accepts three values: + + ``hidden`` (default) + Keep the flat layout. The definition node is used only internally to drive + parametrization and is kept out of the collection tree. + + ``pedantic`` + Insert the definition node with correct marker scoping: function-level + markers (such as ``@pytest.mark.parametrize`` or custom marks) belong to + the definition node, and each invocation only owns its own parameter-set + markers. This is the intended target layout. + + ``messy`` + A stopgap for migration only. It inserts the definition node but transfers + the function-level markers back down onto each invocation, reproducing the + old (duplicated) marker layout so that code which is not yet prepared for + the definition scope keeps working. It emits a warning in the session + header and exists purely to buy time; fix the offending code and move to + ``pedantic``. + + .. tab:: toml + + .. code-block:: toml + + [pytest] + collect_function_definition = "pedantic" + + .. tab:: ini + + .. code-block:: ini + + [pytest] + collect_function_definition = pedantic + + For example, given a parametrized test: + + .. code-block:: python + + import pytest + + + @pytest.mark.parametrize("x", [1, 2]) + def test_it(x): ... + + the default (``hidden``) layout collects the invocations directly under the + module:: + + + + + + while ``pedantic`` and ``messy`` group them under a definition node:: + + + + + + + This is an opt-in structural change. The node ids of the individual test + invocations are unchanged (for example ``test_it.py::test_it[1]``), so test + selection, caching and reporting behave exactly as before. + .. confval:: collect_imported_tests :type: ``bool`` :default: ``true`` @@ -3677,6 +3750,14 @@ All the command-line flags can also be obtained by running ``pytest --help``:: collect_imported_tests (bool): Whether to collect tests in imported modules outside `testpaths` + collect_function_definition (string): + How the function-definition collector node + participates in the collection tree. - hidden + (default): keep the flat layout, no node in the tree + - pedantic: insert the node and scope function-level + markers to it - messy: insert the node but transfer + markers down to each invocation to preserve the + legacy marker layout (emits a warning) consider_namespace_packages (bool): Consider namespace packages when resolving module names during import diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 1b337e20c7e..364f27b7d2d 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -251,6 +251,17 @@ def pytest_addoption(parser: Parser) -> None: type="bool", default=True, ) + parser.addini( + "collect_function_definition", + "How the function-definition collector node participates in the " + "collection tree.\n" + "- hidden (default): keep the flat layout, no node in the tree\n" + "- pedantic: insert the node and scope function-level markers to it\n" + "- messy: insert the node but transfer markers down to each invocation " + "to preserve the legacy marker layout (emits a warning)", + type="string", + default="hidden", + ) parser.addini( "consider_namespace_packages", type="bool", diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 4f168012c36..7b2d26669ea 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -91,6 +91,29 @@ get_args(LongStrIdStrategy) ) +# Modes for the ``collect_function_definition`` option. +# - "hidden": legacy flat layout; the FunctionDefinition is used transiently to +# drive parametrization and kept out of ("hidden" from) the tree. +# - "pedantic": insert the FunctionDefinition node; function-level markers are +# scoped to it and each invocation only owns its callspec markers. +# - "messy": insert the node, but transfer the function-level markers down onto +# each invocation to preserve the legacy marker layout. Emits a +# warning, as this defeats the purpose of the definition scope. +FunctionDefinitionMode = Literal["hidden", "pedantic", "messy"] +_FUNCTION_DEFINITION_MODES: frozenset[FunctionDefinitionMode] = frozenset( + get_args(FunctionDefinitionMode) +) + + +def _collect_function_definition_mode(config: Config) -> FunctionDefinitionMode: + value = config.getini("collect_function_definition") + if value not in _FUNCTION_DEFINITION_MODES: + raise UsageError( + f"Unknown collect_function_definition: {value!r}. " + f"Valid values: {', '.join(sorted(_FUNCTION_DEFINITION_MODES))}" + ) + return cast(FunctionDefinitionMode, value) + def pytest_addoption(parser: Parser) -> None: parser.addini( @@ -163,6 +186,16 @@ def pytest_configure(config: Config) -> None: ) +def pytest_report_header(config: Config) -> list[str] | None: + if _collect_function_definition_mode(config) == "messy": + return [ + "warning: collect_function_definition=messy transfers markers to the " + "individual invocations to preserve the legacy layout; prefer " + "'pedantic' once your plugins handle the definition scope", + ] + return None + + def async_fail(nodeid: str) -> None: msg = ( "async def functions are not natively supported.\n" @@ -331,6 +364,11 @@ def getmodpath(self, stopatmodule: bool = True, includemodule: bool = False) -> """Return Python path relative to the containing module.""" parts = [] for node in self.iter_parents(): + # A FunctionDefinition parent contributes the same name as the + # Function collected under it, so skip it to avoid duplication + # (but keep it when it is the node itself). + if node is not self and isinstance(node, FunctionDefinition): + continue name = node.name if isinstance(node, Module): name = os.path.splitext(name)[0] @@ -467,54 +505,19 @@ def collect(self) -> Iterable[nodes.Item | nodes.Collector]: result.extend(values) return result - def _genfunctions(self, name: str, funcobj) -> Iterator[Function]: - modulecol = self.getparent(Module) - assert modulecol is not None - module = modulecol.obj - clscol = self.getparent(Class) - cls = (clscol and clscol.obj) or None - + def _genfunctions( + self, name: str, funcobj + ) -> Iterator[Function | FunctionDefinition]: definition = FunctionDefinition.from_parent(self, name=name, callobj=funcobj) - fixtureinfo = definition._fixtureinfo - - # pytest_generate_tests impls call metafunc.parametrize() which fills - # metafunc._calls, the outcome of the hook. - metafunc = Metafunc( - definition=definition, - fixtureinfo=fixtureinfo, - config=self.config, - cls=cls, - module=module, - _ispytest=True, - ) - methods = [] - if hasattr(module, "pytest_generate_tests"): - methods.append(module.pytest_generate_tests) - if cls is not None and hasattr(cls, "pytest_generate_tests"): - methods.append(cls().pytest_generate_tests) - self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc)) - - if not metafunc._calls: - yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) + if _collect_function_definition_mode(self.config) == "hidden": + # Legacy flat layout: the definition is used only to drive + # parametrization and is discarded ("hidden"), the invocations are + # collected directly under this collector. + yield from definition._generate_functions(self) else: - metafunc._recompute_direct_params_indices() - # Direct parametrizations taking place in module/class-specific - # `metafunc.parametrize` calls may have shadowed some fixtures, so make sure - # we update what the function really needs a.k.a its fixture closure. Note that - # direct parametrizations using `@pytest.mark.parametrize` have already been considered - # into making the closure using `ignore_args` arg to `getfixtureclosure`. - fixtureinfo.prune_dependency_tree() - - for callspec in metafunc._calls: - subname = f"{name}[{callspec.id}]" if callspec._idlist else name - yield Function.from_parent( - self, - name=subname, - callspec=callspec, - fixtureinfo=fixtureinfo, - keywords={callspec.id: True}, - originalname=name, - ) + # Insert the function definition as a collector node into the tree; + # its ``collect()`` yields the (possibly parametrized) invocations. + yield definition def importtestmodule( @@ -1688,8 +1691,9 @@ def __init__( session: Session | None = None, fixtureinfo: FuncFixtureInfo | None = None, originalname: str | None = None, + nodeid: str | None = None, ) -> None: - super().__init__(name, parent, config=config, session=session) + super().__init__(name, parent, config=config, session=session, nodeid=nodeid) if callobj is not NOTSET: self._obj = callobj @@ -1706,7 +1710,12 @@ def __init__( # Note: when FunctionDefinition is introduced, we should change ``originalname`` # to a readonly property that returns FunctionDefinition.name. - self.own_markers.extend(get_unpacked_marks(self.obj)) + # Function-level markers are owned by the FunctionDefinition scope when + # that node is part of the collection tree; otherwise (flat layout) the + # Function owns them itself. In "messy" mode FunctionDefinition.collect() + # transfers them back onto each invocation for legacy compatibility. + if not isinstance(self.parent, FunctionDefinition): + self.own_markers.extend(get_unpacked_marks(self.obj)) if callspec: self.callspec = callspec self.own_markers.extend(callspec.marks) @@ -1748,17 +1757,16 @@ def instance(self): try: return self._instance except AttributeError: - if isinstance(self.parent, Class): - # Each Function gets a fresh class instance. - self._instance = self._getinstance() - else: - self._instance = None + self._instance = self._getinstance() return self._instance def _getinstance(self): - if isinstance(self.parent, Class): + # The containing class, if any -- skipping over an interposed + # FunctionDefinition node (see the ``collect_function_definition`` option). + cls = self.getparent(Class) + if cls is not None: # Each Function gets a fresh class instance. - return self.parent.newinstance() + return cls.newinstance() else: return None @@ -1767,8 +1775,13 @@ def _getobj(self): if instance is not None: parent_obj = instance else: - assert self.parent is not None - parent_obj = self.parent.obj # type: ignore[attr-defined] + # The namespace this function was collected from -- a Module (or a + # Class handled above), skipping over an interposed FunctionDefinition. + parent = self.parent + while isinstance(parent, FunctionDefinition): + parent = parent.parent + assert parent is not None + parent_obj = parent.obj # type: ignore[attr-defined] return getattr(parent_obj, self.originalname) @property @@ -1823,14 +1836,130 @@ def repr_failure( # type: ignore[override] return self._repr_failure_py(excinfo, style=style) -class FunctionDefinition(Function): - """This class is a stop gap solution until we evolve to have actual function - definition nodes and manage to get rid of ``metafunc``.""" +class FunctionDefinition(PyCollector): + """Collector node for a single test function definition. - def runtest(self) -> None: - raise RuntimeError("function definitions are not supposed to be run as tests") + Its children are the (possibly parametrized) :class:`Function` invocations + generated from the definition via :hook:`pytest_generate_tests`. + + This node is only inserted into the collection tree when the + :confval:`collect_function_definition` option is enabled. Otherwise it is + created transiently to drive parametrization (backing :class:`Metafunc`) + and then discarded, with the resulting :class:`Function` items collected + directly under the containing :class:`Class`/:class:`Module`. + """ - setup = runtest + # Markers are handled explicitly below, mirroring Function. + _ALLOW_MARKERS = False + + def __init__( + self, + name: str, + parent, + callobj, + config: Config | None = None, + session: Session | None = None, + ) -> None: + super().__init__(name, parent, config=config, session=session) + + # The definition always stands for a concrete function object; unlike + # Function it never looks the object up from its parent lazily. + self._obj = callobj + + self.own_markers.extend(get_unpacked_marks(self.obj)) + self.keywords.update((mark.name, mark) for mark in self.own_markers) + self.keywords.update(self.obj.__dict__) + + def collect(self) -> Iterable[nodes.Item | nodes.Collector]: + children = list(self._generate_functions(self)) + if _collect_function_definition_mode(self.config) == "messy": + # Legacy marker layout: transfer this scope's markers back onto each + # invocation and drop them here, so marker resolution matches the flat + # layout exactly (no duplication when walking parents in iter_markers). + marks = self.own_markers + for child in children: + child.own_markers[:0] = marks + child.keywords.update((mark.name, mark) for mark in marks) + self.own_markers = [] + return children + + def _generate_functions(self, parent: nodes.Collector) -> Iterator[Function]: + """Run :hook:`pytest_generate_tests` and yield the resulting + :class:`Function` invocations under ``parent``. + + ``parent`` is this definition node when it is part of the tree, or the + containing :class:`Class`/:class:`Module` in the legacy flat layout. + """ + name = self.name + modulecol = self.getparent(Module) + assert modulecol is not None + module = modulecol.obj + clscol = self.getparent(Class) + cls = (clscol and clscol.obj) or None + + # Compute the function's fixture closure. This drives parametrization and + # is shared with the generated invocations; it is intentionally *not* + # stored on the definition -- fixtures belong to the executed items, not + # to this collector. + # TODO(#3926): getfixtureinfo() is item-scoped, but here the definition + # (a collector) stands in for the not-yet-created invocations. Resolve by + # giving fixture-closure computation a node-level entry point instead of + # casting a collector to an item. + fixtureinfo = self.session._fixturemanager.getfixtureinfo( + cast(nodes.Item, self), self.obj, cls + ) + + # pytest_generate_tests impls call metafunc.parametrize() which fills + # metafunc._calls, the outcome of the hook. + metafunc = Metafunc( + definition=self, + fixtureinfo=fixtureinfo, + config=self.config, + cls=cls, + module=module, + _ispytest=True, + ) + methods = [] + if hasattr(module, "pytest_generate_tests"): + methods.append(module.pytest_generate_tests) + if cls is not None and hasattr(cls, "pytest_generate_tests"): + methods.append(cls().pytest_generate_tests) + self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc)) + + # The invocations keep a flat nodeid anchored at the collector containing + # the definition, regardless of whether the definition itself is part of + # the tree, so nodeids are stable across the ``collect_function_definition`` + # option. + assert self.parent is not None + base_nodeid = self.parent.nodeid + + if not metafunc._calls: + yield Function.from_parent( + parent, + name=name, + fixtureinfo=fixtureinfo, + nodeid=f"{base_nodeid}::{name}", + ) + else: + metafunc._recompute_direct_params_indices() + # Direct parametrizations taking place in module/class-specific + # `metafunc.parametrize` calls may have shadowed some fixtures, so make sure + # we update what the function really needs a.k.a its fixture closure. Note that + # direct parametrizations using `@pytest.mark.parametrize` have already been considered + # into making the closure using `ignore_args` arg to `getfixtureclosure`. + fixtureinfo.prune_dependency_tree() + + for callspec in metafunc._calls: + subname = f"{name}[{callspec.id}]" if callspec._idlist else name + yield Function.from_parent( + parent, + name=subname, + callspec=callspec, + fixtureinfo=fixtureinfo, + keywords={callspec.id: True}, + originalname=name, + nodeid=f"{base_nodeid}::{subname}", + ) def __getattr__(name: str) -> object: diff --git a/testing/test_collect_function_definition.py b/testing/test_collect_function_definition.py new file mode 100644 index 00000000000..5860bd2c356 --- /dev/null +++ b/testing/test_collect_function_definition.py @@ -0,0 +1,193 @@ +"""Tests for the `collect_function_definition` configuration value.""" + +from __future__ import annotations + +from _pytest.pytester import Pytester +import pytest + + +@pytest.fixture +def sample(pytester: Pytester) -> Pytester: + pytester.makepyfile( + """ + import pytest + + def test_plain(): + pass + + @pytest.mark.parametrize("x", [1, 2]) + def test_param(x): + assert x + + class TestCls: + @pytest.mark.parametrize("y", [3, 4]) + def test_method(self, y): + assert y + + def test_single(self): + pass + """ + ) + return pytester + + +def _set_mode(pytester: Pytester, mode: str) -> None: + pytester.makeini(f"[pytest]\ncollect_function_definition = {mode}\n") + + +FLAT_IDS = [ + "test_plain", + "test_param[1]", + "test_param[2]", + "TestCls::test_method[3]", + "TestCls::test_method[4]", + "TestCls::test_single", +] + +TREE_MODES = ["pedantic", "messy"] +ALL_MODES = ["hidden", *TREE_MODES] + + +def test_hidden_is_default(sample: Pytester) -> None: + """Without the option the definition node is not part of the tree.""" + items, _ = sample.inline_genitems() + for item in items: + assert type(item.parent).__name__ in ("Module", "Class") + + +@pytest.mark.parametrize("mode", TREE_MODES) +def test_definition_node_inserted(sample: Pytester, mode: str) -> None: + """In tree modes a FunctionDefinition collector wraps each test.""" + from _pytest.python import Function + from _pytest.python import FunctionDefinition + + _set_mode(sample, mode) + items, _ = sample.inline_genitems() + assert len(items) == len(FLAT_IDS) + for item in items: + assert isinstance(item, Function) + # Every invocation, including non-parametrized ones, is wrapped. + assert isinstance(item.parent, FunctionDefinition) + + +@pytest.mark.parametrize("mode", ALL_MODES) +def test_nodeids_are_stable(sample: Pytester, mode: str) -> None: + """Invocation nodeids stay flat regardless of the mode.""" + _set_mode(sample, mode) + items, _ = sample.inline_genitems() + suffixes = [item.nodeid.split("::", 1)[1] for item in items] + assert suffixes == FLAT_IDS + + +@pytest.mark.parametrize("mode", ALL_MODES) +def test_tests_run(sample: Pytester, mode: str) -> None: + _set_mode(sample, mode) + result = sample.runpytest() + result.assert_outcomes(passed=6) + + +def test_selection_and_reporting(sample: Pytester) -> None: + """-k selection and failure reporting address the flat nodeid.""" + _set_mode(sample, "pedantic") + result = sample.runpytest("-k", "test_param and 1", "-v") + assert "test_param[1] PASSED" in result.stdout.str() + result.assert_outcomes(passed=1, deselected=5) + + +def test_collect_only_tree(sample: Pytester) -> None: + _set_mode(sample, "pedantic") + result = sample.runpytest("--collect-only") + out = result.stdout.str() + for expected in [ + "", + "", + "", + "", + "", + "", + ]: + assert expected in out + + +@pytest.mark.parametrize("mode", ALL_MODES) +def test_markers_are_not_duplicated(pytester: Pytester, mode: str) -> None: + """A function-level marker resolves exactly once in every mode. + + Regression guard: with the definition node in the tree both it and the + invocation carry the same function-level marks, which must not double up + when walking parents in ``iter_markers``. + """ + pytester.makeini(f"[pytest]\ncollect_function_definition = {mode}\n") + pytester.makepyfile( + """ + import pytest + + @pytest.mark.foo + @pytest.mark.parametrize("x", [1]) + def test_it(x): + pass + """ + ) + items, _ = pytester.inline_genitems() + (item,) = items + assert len(list(item.iter_markers(name="foo"))) == 1 + + +def test_pedantic_scopes_markers_to_definition(pytester: Pytester) -> None: + """In pedantic mode function-level markers live on the definition node.""" + from _pytest.python import FunctionDefinition + + pytester.makeini("[pytest]\ncollect_function_definition = pedantic\n") + pytester.makepyfile( + """ + import pytest + + @pytest.mark.foo + def test_it(): + pass + """ + ) + (item,) = pytester.inline_genitems()[0] + assert isinstance(item.parent, FunctionDefinition) + assert [m.name for m in item.own_markers] == [] + assert [m.name for m in item.parent.own_markers] == ["foo"] + + +def test_messy_transfers_markers_to_invocation(pytester: Pytester) -> None: + """In messy mode markers are transferred back onto the invocation.""" + from _pytest.python import FunctionDefinition + + pytester.makeini("[pytest]\ncollect_function_definition = messy\n") + pytester.makepyfile( + """ + import pytest + + @pytest.mark.foo + def test_it(): + pass + """ + ) + (item,) = pytester.inline_genitems()[0] + assert isinstance(item.parent, FunctionDefinition) + assert [m.name for m in item.own_markers] == ["foo"] + # Transferred away from the definition scope to avoid duplication. + assert [m.name for m in item.parent.own_markers] == [] + + +def test_messy_emits_header_warning(sample: Pytester) -> None: + _set_mode(sample, "messy") + result = sample.runpytest() + result.stdout.fnmatch_lines(["*collect_function_definition=messy*"]) + + +def test_pedantic_has_no_header_warning(sample: Pytester) -> None: + _set_mode(sample, "pedantic") + result = sample.runpytest() + result.stdout.no_fnmatch_line("*collect_function_definition=messy*") + + +def test_invalid_value_errors(sample: Pytester) -> None: + _set_mode(sample, "bogus") + result = sample.runpytest() + result.stderr.fnmatch_lines(["*Unknown collect_function_definition: 'bogus'*"]) + assert result.ret != 0