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

AUTHORS

@@ -85,6 +85,7 @@ Bruno Oliveira
 Cal Jacobson
 croc100
 Cal Leeming
+Caner
 Carl Friedrich Bolz
 Carlos Jenkins
 Ceridwen

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.

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))
 

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()