Skip to content

fix(monkeypatch): don't leave inherited attributes in the instance dict - #14969

Open
Irahan2 wants to merge 1 commit into
pytest-dev:mainfrom
Irahan2:fix-10644-monkeypatch-inherited-attr
Open

fix(monkeypatch): don't leave inherited attributes in the instance dict#14969
Irahan2 wants to merge 1 commit into
pytest-dev:mainfrom
Irahan2:fix-10644-monkeypatch-inherited-attr

Conversation

@Irahan2

@Irahan2 Irahan2 commented Sep 4, 2026

Copy link
Copy Markdown

Closes #10644.

Problem

MonkeyPatch.setattr() records the old value with getattr(), which follows the MRO. When the patched attribute is inherited rather than owned by the instance, undo() assigns that inherited value back onto the instance, creating a __dict__ entry that was never there:

class Parent:
    x = 1

class Child(Parent):
    pass

obj = Child()
mp.setattr(obj, "x", 2)
mp.undo()
assert "x" not in vars(obj)   # fails on main: vars(obj) == {"x": 1}

For a plain class attribute this only leaves the target in a different state than it was found in. For an inherited non-data descriptor it is worse: the value computed during teardown is stored on the instance and shadows the descriptor, so the attribute is frozen for every later lookup.

class Dynamic:
    def __init__(self):
        self.calls = 0

    def __get__(self, instance, owner=None):
        self.calls += 1
        return self.calls

class Sample:
    value = Dynamic()

obj = Sample()
obj.value, obj.value      # 1, 2 -- resolves dynamically
mp.setattr(obj, "value", "patched")
mp.undo()
obj.value, obj.value      # 3, 3 -- frozen from here on, on main

Patching a method on an instance hits the same path, leaving a bound method behind in vars(obj) after teardown.

Fix

Look the old value up in the instance __dict__ — which is what setattr() and undo() actually operate on — mirroring the handling setattr() already has for classes.

The lookup is guarded by a data-descriptor check, and that guard is load-bearing. A data descriptor intercepts the assignment, so the attribute never reaches the instance __dict__ and there is nothing there to delete on undo. Without the guard, monkeypatch.setattr() on a property fails during undo with AttributeError: property 'x' of 'Sample' object has no deleter, and __slots__ attributes break the same way. test_undo_data_descriptor_on_instance and test_undo_slot_attribute_on_instance cover both.

Notes

Only setattr() is changed. delattr() has the same shape but cannot reach this state: deleting an inherited attribute from an instance raises AttributeError, so nothing is recorded. Its separate undo-ordering issue is #14909.

Verified locally on Python 3.13 (Windows): the three new tests reproducing the bug fail without the change and pass with it, and the full test suite is unaffected.

Checklist

  • Include new tests or update existing tests when applicable.
  • Add text like closes #XYZW to the PR description and/or commits.
  • Create a new changelog file in the changelog directory.
  • Add yourself to AUTHORS in alphabetical order.
  • Allow maintainers to push and squash when merging my commits.

AI/LLM assistance

Per the AI/LLM-Assisted Contributions Policy: this change was developed with AI assistance (Claude Code), credited in the Co-authored-by trailer on the commit. Happy to explain any part of it or adjust the approach.

@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Sep 4, 2026
`MonkeyPatch.setattr()` recorded the old value with `getattr()`, which
follows the MRO. When the attribute was inherited rather than owned by the
instance, `undo()` assigned that inherited value back onto the instance,
adding a `__dict__` entry that had not been there before.

For a plain class attribute this only leaves the target in a different state
than it was found in. For an inherited non-data descriptor it is worse: the
value computed during teardown is stored on the instance and shadows the
descriptor, so every later lookup returns that frozen value.

Look the old value up in the instance `__dict__` instead, which is what
`setattr()` and `undo()` actually operate on -- but only when no data
descriptor is in the way. Data descriptors intercept the assignment, so for
those the `getattr()` value remains the right thing to restore.

Closes pytest-dev#10644.

Co-authored-by: Claude <noreply@anthropic.com>
@Irahan2
Irahan2 force-pushed the fix-10644-monkeypatch-inherited-attr branch from 612cb41 to 0c601d5 Compare September 4, 2026 10:16
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.

MonkeyPatch.setattr leaves a new item in vars(target) after cleanup when overriding an inherited attribute of a non-class object

1 participant