Merge a43a7adddb into dc65bb6a66

AUTHORS

@@ -159,6 +159,7 @@ Floris Bruynooghe
 Fraser Stark
 Gabriel Landau
 Gabriel Reis
+Ganden Schaffner
 Garvit Shubham
 Gene Wood
 George Kussumoto

changelog/10644.bugfix.rst

@@ -0,0 +1,3 @@
+`monkeypatch.setattr` no longer leaves a leftover item in the target's dictionary after cleanup when patching an inherited attribute of a non-class object. This fixes a bug where a `__get__` descriptor's dynamic lookup was blocked and replaced by a cached static lookup after `monkeypatch` teardown.
+
+Previously `monkeypatch.setattr` avoided leaving leftover items in the target's dictionary when patching class objects but not when patching non-class objects.

src/_pytest/monkeypatch.py

@@ -223,7 +223,6 @@ class MonkeyPatch:
         applies to ``monkeypatch.setattr`` as well.
         """
         __tracebackhide__ = True
-        import inspect
 
         if isinstance(value, Notset):
             if not isinstance(target, str):
@@ -246,9 +245,10 @@ class MonkeyPatch:
         if raising and oldval is notset:
             raise AttributeError(f"{target!r} has no attribute {name!r}")
 
-        # avoid class descriptors like staticmethod/classmethod
-        if inspect.isclass(target):
-            oldval = target.__dict__.get(name, notset)
+        # Prevent `undo` from polluting `vars(target)` with an object that was not in it
+        # before monkeypatching, such as inherited attributes or the results of
+        # descriptor binding.
+        oldval = vars(target).get(name, notset)
         self._setattr.append((target, name, oldval))
         setattr(target, name, value)
 

testing/test_monkeypatch.py

@@ -6,12 +6,30 @@ import sys
 import textwrap
 from typing import Dict
 from typing import Generator
+from typing import Optional
+from typing import overload
 from typing import Type
+from typing import TypeVar
 
 from _pytest.monkeypatch import MonkeyPatch
 from _pytest.pytester import Pytester
 import pytest
 
+T = TypeVar("T")
+
+
+class SomeDescriptor:
+    @overload
+    def __get__(self, instance: None, owner: type) -> int:
+        ...
+
+    @overload
+    def __get__(self, instance: T, owner: Optional[Type[T]] = ...) -> int:
+        ...
+
+    def __get__(self, instance: Optional[T], owner: Optional[Type[T]] = None) -> int:
+        return 1
+
 
 @pytest.fixture
 def mp() -> Generator[MonkeyPatch, None, None]:
@@ -23,15 +41,21 @@ def mp() -> Generator[MonkeyPatch, None, None]:
 
 
 def test_setattr() -> None:
+    import inspect
+
     class A:
         x = 1
+        y = SomeDescriptor()
 
     monkeypatch = MonkeyPatch()
     pytest.raises(AttributeError, monkeypatch.setattr, A, "notexists", 2)
-    monkeypatch.setattr(A, "y", 2, raising=False)
-    assert A.y == 2  # type: ignore
+    monkeypatch.setattr(A, "w", 2, raising=False)
+    assert A.w == 2  # type: ignore
     monkeypatch.undo()
-    assert not hasattr(A, "y")
+    assert not hasattr(A, "w")
+
+    with pytest.raises(TypeError):
+        monkeypatch.setattr(A, "w")  # type: ignore[call-overload]
 
     monkeypatch = MonkeyPatch()
     monkeypatch.setattr(A, "x", 2)
@@ -45,8 +69,23 @@ def test_setattr() -> None:
     monkeypatch.undo()  # double-undo makes no modification
     assert A.x == 5
 
-    with pytest.raises(TypeError):
-        monkeypatch.setattr(A, "y")  # type: ignore[call-overload]
+    # Test that inherited attributes don't get written into the target's instance
+    # dictionary.
+    a = A()
+    monkeypatch = MonkeyPatch()
+    assert "x" not in vars(a)
+    monkeypatch.setattr(a, "x", 2)
+    monkeypatch.undo()
+    assert "x" not in vars(a)
+
+    # Test that class/instance descriptors don't get bound and written into the target's
+    # dictionary.
+    for obj in (A, A()):
+        monkeypatch = MonkeyPatch()
+        assert isinstance(inspect.getattr_static(obj, "y"), SomeDescriptor)
+        monkeypatch.setattr(obj, "y", 2)
+        monkeypatch.undo()
+        assert isinstance(inspect.getattr_static(obj, "y"), SomeDescriptor)
 
 
 class TestSetattrWithImportPath:
@@ -97,8 +136,11 @@ class TestSetattrWithImportPath:
 
 
 def test_delattr() -> None:
+    import inspect
+
     class A:
         x = 1
+        y = SomeDescriptor()
 
     monkeypatch = MonkeyPatch()
     monkeypatch.delattr(A, "x")
@@ -107,14 +149,22 @@ def test_delattr() -> None:
     assert A.x == 1
 
     monkeypatch = MonkeyPatch()
+    pytest.raises(AttributeError, monkeypatch.delattr, A, "w")
+    monkeypatch.delattr(A, "w", raising=False)
     monkeypatch.delattr(A, "x")
-    pytest.raises(AttributeError, monkeypatch.delattr, A, "y")
-    monkeypatch.delattr(A, "y", raising=False)
     monkeypatch.setattr(A, "x", 5, raising=False)
     assert A.x == 5
     monkeypatch.undo()
     assert A.x == 1
 
+    # Test that (non-inherited) class descriptors don't get bound and written into the
+    # target's dictionary.
+    monkeypatch = MonkeyPatch()
+    assert isinstance(inspect.getattr_static(A, "y"), SomeDescriptor)
+    monkeypatch.delattr(A, "y")
+    monkeypatch.undo()
+    assert isinstance(inspect.getattr_static(A, "y"), SomeDescriptor)
+
 
 def test_setitem() -> None:
     d = {"x": 1}