fix: iter_markers/get_closest_marker returns correct closest MRO marker (#14329) - #14630
fix: iter_markers/get_closest_marker returns correct closest MRO marker (#14329)#14630RonnyPfannschmidt wants to merge 7 commits into
Conversation
a1f29c3 to
3c7d2d5
Compare
There was a problem hiding this comment.
Pull request overview
Fixes marker resolution for inherited test classes so iter_markers/get_closest_marker return the closest (most-derived) class’ marks first, while attempting to preserve backward-compatible own_markers ordering and existing fixture-setup behavior.
Changes:
- Add
Node._iter_own_markers_closest_first()and switch marker iteration to use it for closest-first semantics. - Override marker iteration for
python.Classto traverse class MRO in closest-first order while preserving per-class decorator stacking. - Adjust
usefixturesprocessing to preserve farthest-first fixture requesting, and add regression tests + changelog entry.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| testing/test_mark.py | Adds regression tests for closest-first MRO marker resolution and updates a mock-based test for the new iteration hook. |
| src/_pytest/python.py | Implements Class._iter_own_markers_closest_first() and introduces an override decorator compatibility shim. |
| src/_pytest/nodes.py | Routes marker iteration through _iter_own_markers_closest_first() and updates docstrings to state closest-first behavior. |
| src/_pytest/fixtures.py | Reverses usefixtures marker iteration to keep farthest-first fixture request ordering after marker iteration changes. |
| changelog/14329.bugfix.rst | Documents the bugfix for closest marker resolution with class inheritance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Reverse order (farthest to closest) is more natural for usefixtures, | ||
| # e.g. want a module-level usefixture to be requested before a class one, | ||
| # a parent class' before a child's, etc. | ||
| for marker_node, mark in reversed( | ||
| list(node.iter_markers_with_node(name="usefixtures")) |
| if sys.version_info >= (3, 12): | ||
| from typing import override | ||
| else: | ||
|
|
||
| def override(func): | ||
| return func | ||
|
|
| # Yield any dynamically added markers (via add_marker) not from MRO. | ||
| for mark in self.own_markers: | ||
| if id(mark) not in mro_mark_ids: | ||
| yield mark |
c18c2f5 to
9808ef4
Compare
|
Hi! |
9808ef4 to
79ce1f0
Compare
…O marker Fix get_closest_marker and iter_markers to return markers in correct closest-first order when class inheritance (MRO) is involved (pytest-dev#14329). Previously, own_markers on Class nodes stored MRO-inherited markers in base-first (farthest) order, and iter_markers yielded them in that same order. This caused get_closest_marker to return a base class marker instead of the overriding child class marker. The fix introduces _iter_own_markers_closest_first() on Node, overridden by Class to walk the MRO in natural closest-first order while preserving decorator stacking order within each class. This avoids changing own_markers (keeping its base-first construction order) and avoids breaking parametrize naming order. Also reverses usefixtures marker iteration to maintain farthest-first setup ordering (module -> base class -> child class -> function). Alternative structural approach to PR pytest-dev#14332 that preserves own_markers order for backward compatibility. Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Opus 4.6 <claude@anthropic.com>
- Add @OverRide decorator to Class._iter_own_markers_closest_first (with version-gated import for Python <3.12) - Broaden isinstance check from list to Sequence for cls_marks - Add test for dynamically added markers on Class collectors Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Opus 4.6 <claude@anthropic.com>
Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Opus 4.6 <claude@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Dynamically added markers on a Class collector were always yielded after the MRO-inherited markers, so add_marker(append=False) no longer put the marker closest as it does on a plain node. Iterate own_markers in stored order and substitute the MRO markers in closest-first order at their slots, keeping dynamic markers in the position add_marker gave them. Adds a test asserting prepend/append ordering relative to the MRO markers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address review feedback on pytest-dev#14630: * `Class._iter_own_markers_closest_first` matched MRO markers in `own_markers` by `id()` and consumed a `next()` per hit. When the same `Mark` object reached `own_markers` twice -- e.g. a module-level `mark = pytest.mark.foo(1)` used as a class decorator *and* passed to `add_marker()` -- the iterator ran dry and collection died with `RuntimeError: generator raised StopIteration`. Locate the contiguous MRO run by identity instead and substitute it wholesale, falling back to the stored order when the run is gone. * Do not force `obj` resolution from marker iteration; without a resolved `obj` there are no MRO markers to reorder anyway. * Extract `get_mro_mark_groups()` so the MRO/`pytestmark` normalization lives in one place instead of being duplicated in `python.py`, and build `get_unpacked_marks()` on top of it. * Move the `override` shim to `_pytest.compat`, next to `deprecated`, so type checkers see it on Python < 3.12. * Spell out why `_getusefixturesnames` reverses within a node too, and pin it with a test: stacked `usefixtures` decorators now request their fixtures in source order, matching `usefixtures("a", "b")`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eb15725 to
35cd12e
Compare
The regression test for pytest-dev#14329 is Wintreist's, taken verbatim from pytest-dev#14332; carry the attribution and add them to AUTHORS. Also spell out the parametrize consequence in the changelog: class level `parametrize` markers are consumed through `iter_markers`, so inherited ones now compose IDs closest-first (`[1-x]` -> `[x-1]`). That invalidates pinned ID selections and the `--last-failed` cache, which is worth more warning than the marker reorder itself. Co-authored-by: Wintreist <49996562+Wintreist@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_lookup_derived` scanned `own_markers` front-to-back and returned the first name match, but the eager `keywords.update((mark.name, mark) ...)` it replaces let *later* markers win. A Class stores MRO-inherited markers base class first, so `keywords["foo"]` silently changed from the subclass' marker to the base class' one. Scan in reverse instead. Note this direction is tied to `own_markers` order: it has to flip if pytest-dev#14332 lands rather than pytest-dev#14630. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Answering Bruno's review question: any sequence will do, not just a list. Previously a tuple worked nowhere, and the closest-first MRO walk in this branch had quietly started accepting one for inherited class marks only -- so make it consistent across modules, classes and functions instead of leaving the paths disagreeing. `str`/`bytes` are deliberately not treated as sequences, so an accidental `pytestmark = "xfail"` keeps reporting the string itself rather than its first character. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Hi! Is this Pull Request not forgotten? |
|
Im just mixed capacity as im at a family vacation atm |
|
Oh, I’m sorry. |
|
Np it's absolutely fine to gently ask This started t the pytest sprint and I intend to complete it soon |
Alternative approach to #14332 for fixing #14329 (
get_closest_markerreturning the wrong marker with class inheritance).The regression test is Wintreist's, taken from #14332 — thanks for it, and for the patience.
Node._iter_own_markers_closest_first(), overridden byClassto walk the MRO in natural closest-first order while preserving decorator stacking order within each classown_markersorder — it stays in base-first (construction) orderusefixturesmarker iteration to maintain farthest-first fixture setup orderingKey difference from #14332
#14332 reverses the MRO traversal in
get_unpacked_marks, which changesown_markersorder onClassnodes. This approach keepsown_markersunchanged and fixes the iteration layer (iter_markers/get_closest_marker), which is the actual consumer that needs closest-first semantics.The concrete difference is
keywords.PyobjMixin.objdoesself.keywords.update((mark.name, mark) for mark in self.own_markers)— last wins — so withbase-first
own_markersthe closest mark is the one that ends up inkeywords. Reversingown_markerssilently flips that to the base mark:No test covers this today (
test_collection.pyuses distinct mark names across levels), so itgoes unnoticed. It is fixable in #14332 with a
reversed(), but it is the reason this PRtargets iteration instead.
Behaviour changes to be aware of
Both this PR and #14332 share these — they follow from closest-first iteration itself, not from
either implementation:
parametrizecomposes IDs in the opposite order._genfunctionsconsumesdefinition.iter_markers("parametrize"), so a base class@pytest.mark.parametrize("a", ...)combined with a subclass@pytest.mark.parametrize("b", ...)now yields
TestChild::test_it[x-1]where it used to yieldTestChild::test_it[1-x].Selections pinned to such IDs and the
--last-failedcache need regenerating. Documented in thechangelog.
usefixturesmarkers on one node request their fixtures in source order._getusefixturesnamesreverses the fulliter_markers_with_noderesult, within a node as wellas across the collection chain. That is deliberate: markers are stored bottom-up, so reversing
makes two stacked
@pytest.mark.usefixturesdecorators behave like a singleusefixtures("a", "b"). Pinned bytest_mark_fixture_order_usefixtures_stacking.Also here:
pytestmarkaccepts any sequenceAnswering @nicoddemus' review question — any sequence will do, not just a
list:This started as an accident: routing the MRO walk through the new
get_mro_mark_groups()made tuples work for inherited class marks only, while
consider_mro=Falseandmodule/function
pytestmarkstill rejected them. Rather than leave the three pathsdisagreeing, they now share one
_as_mark_sequence()helper.str/bytesare deliberatelyexcluded, so an accidental
pytestmark = "xfail"still reports the string itself rather thanits first character. Separate
changelog/14630.improvement.rst.Test plan
test_mark_closest_mro— closest-first MRO marker resolution (from BugFix #14329 #14332)test_mark_closest_mro_with_dynamic_class_marker—add_marker(append=)keeps its position relative to MRO markerstest_mark_mro_marker_object_reused_by_add_marker— the sameMarkobject reachingown_markersvia both the MRO andadd_markerno longer aborts collection withRuntimeError: generator raised StopIterationtest_mark_fixture_order_usefixtures_stacking— stackedusefixturesordertest_mark_mro/get_unpacked_markstests pass unchanged (own_markersorder preserved)Interaction with other open PRs
keywordslazily and resolves a name by scanningown_markersfirst-wins,where today's
dict.updateis last-wins. On top of this PR that flipsClass.keywords["foo"]to the base class mark; it needsreversed()there (and the oppositeif BugFix #14329 #14332 lands instead).