From 0c601d510c6ba370fa89d6e019cf43e2d02d22d0 Mon Sep 17 00:00:00 2001 From: Caner <118784594+Irahan2@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:09:35 +0200 Subject: [PATCH] fix(monkeypatch): don't leave inherited attributes in the instance dict `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 #10644. Co-authored-by: Claude --- AUTHORS | 1 + changelog/10644.bugfix.rst | 3 + src/_pytest/monkeypatch.py | 24 +++++++ testing/test_monkeypatch.py | 137 ++++++++++++++++++++++++++++++++++++ 4 files changed, 165 insertions(+) create mode 100644 changelog/10644.bugfix.rst diff --git a/AUTHORS b/AUTHORS index ba5672d4c51..11daaa98218 100644 --- a/AUTHORS +++ b/AUTHORS @@ -85,6 +85,7 @@ Bruno Oliveira Cal Jacobson croc100 Cal Leeming +Caner Carl Friedrich Bolz Carlos Jenkins Ceridwen diff --git a/changelog/10644.bugfix.rst b/changelog/10644.bugfix.rst new file mode 100644 index 00000000000..d7135f80f5b --- /dev/null +++ b/changelog/10644.bugfix.rst @@ -0,0 +1,3 @@ +``monkeypatch.setattr()`` no longer leaves a new entry in the instance ``__dict__`` when it patches an attribute that the instance inherits from its class. + +Previously ``undo()`` assigned the inherited value onto the instance, which shadowed the class attribute -- permanently freezing the result for descriptors that resolve dynamically. diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index d6db72455a8..d468b4afe1b 100644 --- a/src/_pytest/monkeypatch.py +++ b/src/_pytest/monkeypatch.py @@ -110,6 +110,20 @@ def derive_importpath(import_path: str, raising: bool) -> tuple[str, object]: return attr, target +def _is_data_descriptor(cls: type, name: str) -> bool: + """Return True if looking up ``name`` on ``cls`` finds a data descriptor. + + Data descriptors take precedence over the instance ``__dict__``, so + ``setattr()`` on an instance is routed through the descriptor instead of + writing an entry into the instance ``__dict__``. + """ + for klass in cls.__mro__: + if name in klass.__dict__: + descr_type = type(klass.__dict__[name]) + return hasattr(descr_type, "__set__") or hasattr(descr_type, "__delete__") + return False + + @final class MonkeyPatch: """Helper to conveniently monkeypatch attributes/items/environment @@ -244,6 +258,16 @@ def setattr( # avoid class descriptors like staticmethod/classmethod if inspect.isclass(target): oldval = target.__dict__.get(name, NOTSET) + elif not _is_data_descriptor(type(target), name): + # With no data descriptor in the way, the `setattr()` below writes + # into the instance `__dict__`, so `undo()` has to restore that + # `__dict__` entry. Assigning an inherited `oldval` back onto the + # instance would instead leave behind a new entry shadowing the + # class attribute, which permanently freezes descriptors that + # resolve dynamically (#10644). + target_dict = getattr(target, "__dict__", None) + if isinstance(target_dict, Mapping): + oldval = target_dict.get(name, NOTSET) setattr(target, name, value) self._setattr.append((target, name, oldval)) diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index 04b16a1e8c2..721f3011e38 100644 --- a/testing/test_monkeypatch.py +++ b/testing/test_monkeypatch.py @@ -418,6 +418,143 @@ class SampleChild(SampleParent): assert original_world == SampleChild.world +def test_undo_inherited_attribute_on_instance() -> None: + """Undo must not leave an inherited attribute behind in the instance dict. + + See #10644. + """ + + class Parent: + x = 1 + + class Child(Parent): + pass + + obj = Child() + monkeypatch = MonkeyPatch() + + monkeypatch.setattr(obj, "x", 2) + assert obj.x == 2 + assert vars(obj)["x"] == 2 + + monkeypatch.undo() + assert obj.x == 1 + assert "x" not in vars(obj) + + +def test_undo_inherited_non_data_descriptor_on_instance() -> None: + """Undo must not freeze a descriptor which resolves dynamically. + + See #10644. + """ + + class Dynamic: + def __init__(self) -> None: + self.calls = 0 + + def __get__(self, instance: object, owner: type | None = None) -> int: + self.calls += 1 + return self.calls + + class Sample: + value = Dynamic() + + obj = Sample() + first, second = obj.value, obj.value + assert first != second + + monkeypatch = MonkeyPatch() + monkeypatch.setattr(obj, "value", -1) + assert obj.value == -1 + + monkeypatch.undo() + assert "value" not in vars(obj) + third, fourth = obj.value, obj.value + assert third != fourth + + +def test_undo_inherited_method_on_instance() -> None: + """Patching a method on an instance must not leave a bound method behind.""" + + class Sample: + def hello(self) -> str: + return "hello" + + obj = Sample() + monkeypatch = MonkeyPatch() + + monkeypatch.setattr(obj, "hello", lambda: "patched") + assert obj.hello() == "patched" + + monkeypatch.undo() + assert obj.hello() == "hello" + assert "hello" not in vars(obj) + + +def test_undo_own_attribute_on_instance() -> None: + """An attribute owned by the instance is still restored to its old value.""" + + class Sample: + x = "class" + + def __init__(self) -> None: + self.x = "instance" + + obj = Sample() + monkeypatch = MonkeyPatch() + + monkeypatch.setattr(obj, "x", "patched") + assert obj.x == "patched" + + monkeypatch.undo() + assert vars(obj)["x"] == "instance" + + +def test_undo_data_descriptor_on_instance() -> None: + """A data descriptor owns the attribute, so the old value is set back through it.""" + + class Sample: + def __init__(self) -> None: + self._x = 1 + + @property + def x(self) -> int: + return self._x + + @x.setter + def x(self, value: int) -> None: + self._x = value + + obj = Sample() + monkeypatch = MonkeyPatch() + + monkeypatch.setattr(obj, "x", 2) + assert obj.x == 2 + + monkeypatch.undo() + assert obj.x == 1 + assert "x" not in vars(obj) + + +def test_undo_slot_attribute_on_instance() -> None: + """Slot descriptors are data descriptors, so undo restores the slot value.""" + + class Sample: + __slots__ = ("x",) + + def __init__(self) -> None: + self.x = 1 + + obj = Sample() + monkeypatch = MonkeyPatch() + + monkeypatch.setattr(obj, "x", 2) + assert obj.x == 2 + + monkeypatch.undo() + assert obj.x == 1 + + def test_issue1338_name_resolving() -> None: pytest.importorskip("requests") monkeypatch = MonkeyPatch()