Skip to content

Opt-in FunctionDefinition collection tree node (#3926) - #14769

Draft
RonnyPfannschmidt wants to merge 1 commit into
pytest-dev:mainfrom
RonnyPfannschmidt:feature/function-definition-collection-node
Draft

Opt-in FunctionDefinition collection tree node (#3926)#14769
RonnyPfannschmidt wants to merge 1 commit into
pytest-dev:mainfrom
RonnyPfannschmidt:feature/function-definition-collection-node

Conversation

@RonnyPfannschmidt

Copy link
Copy Markdown
Member

Closes #3926.

Promotes the internal FunctionDefinition from a transient parametrization
helper to a real collector node in the collection tree, behind an opt-in ini
option.

collect_function_definition (tri-state string, default hidden)

Mode Tree Function-level markers
hidden (default) flat, definition kept out of the tree on the Function (legacy)
pedantic definition node inserted scoped to the definition; invocation owns only its callspec marks
messy definition node inserted transferred back onto each invocation (legacy layout), + header warning

messy is a migration stopgap for code that is not yet prepared for the
definition scope — it reproduces the old duplicated-marker layout so bad code
keeps working. The target is pedantic.

What changes

  • FunctionDefinition becomes a PyCollector (no longer a Function/Item), so
    it is collected rather than run. Its collect() runs pytest_generate_tests
    and yields the invocations.
  • Invocation node ids stay flat and identical across all modes
    (test_mod.py::test_x[0]), so -k, caching, --lf, xdist and reporting are
    unaffected.
  • obj/instance resolution and getmodpath transparently skip an interposed
    definition node.
  • Marker scoping is correct in every mode (iter_markers yields each mark once).

The default (hidden) path is behavior-preserving; the whole test suite passes
unchanged.

Known warts / follow-up TODOs

These are intentionally left as follow-ups (inline TODO(#3926) where relevant),
to keep this PR focused — happy to split into a tracking issue:

  1. Item-scoped getfixtureinfo on a collector. The fixture closure must be
    computed before the invocations exist, so FunctionDefinition._generate_functions
    calls the item-scoped getfixtureinfo on the collector via a cast. Needs a
    node-level entry point for closure computation. (TODO(#3926) inline.)
  2. messy mode is temporary. It exists only to unblock migration and should
    be removed once consumers move to the definition scope. Marked clearly in the
    docs and header warning.
  3. Collector/item node-id collision. For non-parametrized tests the definition
    node and its sole Function child share a node id. Accepted for now; the
    structured-nodeids work will give these a better data structure.
  4. originalname could now become a read-only property returning the
    definition's name (see the existing note in Function.__init__).

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@RonnyPfannschmidt

Copy link
Copy Markdown
Member Author

Heads-up on a defect in this PR as it currently stands: selecting a test by node id is broken in the tree modes.

$ cat pytest.ini
[pytest]
collect_function_definition = pedantic

$ pytest -q "test_x.py::test_a[1]"
ERROR: not found: test_x.py::test_a
(no match in any of [<Module test_x.py>])

Works under hidden, fails under both pedantic and messy. So the PR description's

Invocation node ids stay flat and identical across all modes, so selection, caching and reporting are unaffected.

holds for the node id strings, but not for selection: Session._collect_and_match_nodes walks the tree comparing one name part per level, and the last part is matched as node.name == part + parametrization. With the definition interposed, alpha[1] is compared against the definition's bare name alpha and never matches, so it never descends to the items. -k and full-path invocation are fine — only ::-addressing of a specific test is affected.

The minimal fix is to let the matcher look through an ItemDefinition for the last part (match the bare name, then re-match the same part one level down against the items). I have that plus a regression matrix (6 selector shapes × 3 modes) in #14805, which is stacked on this branch — happy to move it down into this PR instead if you'd rather this one be self-contained.

Worth noting the existing comment right at that spot already anticipates this:

# A non-parameterized arg matches all parametrizations (if any).
# TODO: Remove the hacky split once the collection structure
# contains parametrization.
is_match = node.name.split("[")[0] == matchparts[0]

This PR is exactly what makes the collection structure contain the parametrization, so that split("[") hack is now removable in principle — though doing it properly probably wants the structured NodeId/ParamId from #14758 rather than more string surgery. Will follow up on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

introduce FunctionDefinition with a scope to the official collection tree

1 participant