Avoid unnecessary descriptor reads with raising=False

AUTHORS

@@ -210,6 +210,7 @@ Harshna
 Henk-Jaap Wagenaar
 Henry Schreiner
 Holger Kohr
+Hou,Ting-Han
 Hugo van Kemenade
 Hui Wang (coldnight)
 Ian Bicking

changelog/10646.bugfix.rst

@@ -0,0 +1 @@
+Avoid binding class and non-data descriptors unnecessarily in :meth:`pytest.MonkeyPatch.setattr` and :meth:`pytest.MonkeyPatch.delattr` when ``raising=False``. Instance data descriptors are still read so their values can be restored.

pyproject.toml

@@ -448,6 +448,9 @@ norecursedirs = [
 strict = true
 filterwarnings = [
     'error',
+    # NumPy 2.4.6 emits this Cython warning when importing random on PyPy.
+    # Keep it visible without failing unrelated Hypothesis tests on import.
+    'default:cython\.collection_type only works on PyPy with the C flag CYTHON_USE_TYPE_SPECS=1:RuntimeWarning',
     'default:Using or importing the ABCs:DeprecationWarning:unittest2.*',
     # produced by older pyparsing<=2.2.0.
     'default:Using or importing the ABCs:DeprecationWarning:pyparsing.*',

src/_pytest/monkeypatch.py

@@ -124,6 +124,23 @@ def _is_data_descriptor(cls: type, name: str) -> bool:
     return False
 
 
+def _getattr_for_patch(target: object, name: str) -> object:
+    """Read an attribute without binding descriptors when restoring raw storage.
+
+    Instance data descriptors still need their value read: undo restores that
+    value through the descriptor's setter, rather than replacing the descriptor.
+    """
+    import inspect
+
+    if not inspect.isclass(target) and _is_data_descriptor(type(target), name):
+        return getattr(target, name, NOTSET)
+    value = inspect.getattr_static(target, name, NOTSET)
+    if value is NOTSET:
+        # Preserve attributes provided dynamically by __getattr__.
+        return getattr(target, name, NOTSET)
+    return value
+
+
 @final
 class MonkeyPatch:
     """Helper to conveniently monkeypatch attributes/items/environment
@@ -251,7 +268,10 @@ def setattr(
                     "import string"
                 )
 
-        oldval = getattr(target, name, NOTSET)
+        if raising:
+            oldval = getattr(target, name, NOTSET)
+        else:
+            oldval = _getattr_for_patch(target, name)
         if raising and oldval is NOTSET:
             raise AttributeError(f"{target!r} has no attribute {name!r}")
 
@@ -298,11 +318,18 @@ def delattr(
                 )
             name, target = derive_importpath(target, raising)
 
-        if not hasattr(target, name):
+        if raising:
+            exists = hasattr(target, name)
+            oldval: object = NOTSET
+            if exists:
+                oldval = getattr(target, name, NOTSET)
+        else:
+            oldval = _getattr_for_patch(target, name)
+            exists = oldval is not NOTSET
+        if not exists:
             if raising:
                 raise AttributeError(name)
         else:
-            oldval = getattr(target, name, NOTSET)
             # Avoid class descriptors like staticmethod/classmethod.
             if inspect.isclass(target):
                 oldval = target.__dict__.get(name, NOTSET)

testing/test_monkeypatch.py

@@ -688,3 +688,143 @@ def test_syspath_prepend_with_namespace_packages(
 
     modules_tmpdir.joinpath("main_app.py").write_text("app = True", encoding="utf-8")
     from main_app import app  # noqa: F401
+
+
+@pytest.mark.parametrize("operation", ["setattr", "delattr"])
+@pytest.mark.parametrize("raises", [False, True])
+def test_non_raising_class_descriptor(operation: str, raises: bool) -> None:
+    calls = []
+
+    class Descriptor:
+        def __get__(self, instance, owner):
+            calls.append(True)
+            if raises:
+                raise RuntimeError("descriptor should not execute")
+            return 42
+
+    descriptor = Descriptor()
+
+    class Target:
+        value = descriptor
+
+    if raises:
+        with pytest.raises(RuntimeError, match="descriptor should not execute"):
+            _ = Target.value
+    else:
+        assert Target.value == 42
+    assert calls == [True]
+    calls.clear()
+
+    with MonkeyPatch.context() as mp:
+        if operation == "setattr":
+            mp.setattr(Target, "value", 99, raising=False)
+            assert vars(Target)["value"] == 99
+        else:
+            mp.delattr(Target, "value", raising=False)
+            assert "value" not in vars(Target)
+        assert calls == []
+    assert vars(Target)["value"] is descriptor
+    assert calls == []
+
+
+def test_non_raising_instance_non_data_descriptor() -> None:
+    class Descriptor:
+        def __get__(self, instance, owner):
+            raise RuntimeError("descriptor should not execute")
+
+    class Target:
+        value = Descriptor()
+
+    obj = Target()
+    with pytest.raises(RuntimeError, match="descriptor should not execute"):
+        _ = obj.value
+    with MonkeyPatch.context() as mp:
+        mp.setattr(obj, "value", 99, raising=False)
+        assert obj.value == 99
+    assert "value" not in vars(obj)
+
+
+@pytest.mark.parametrize("operation", ["setattr", "delattr"])
+def test_non_raising_data_descriptor_undo(operation: str) -> None:
+    class Target:
+        def __init__(self):
+            self._value = 42
+
+        @property
+        def value(self):
+            return self._value
+
+        @value.setter
+        def value(self, value):
+            self._value = value
+
+        @value.deleter
+        def value(self):
+            del self._value
+
+    obj = Target()
+    with MonkeyPatch.context() as mp:
+        if operation == "setattr":
+            mp.setattr(obj, "value", 99, raising=False)
+            assert obj.value == 99
+        else:
+            mp.delattr(obj, "value", raising=False)
+            assert not hasattr(obj, "_value")
+    assert obj.value == 42
+
+
+@pytest.mark.parametrize("raising", [False, True])
+def test_descriptor_raising_contract(raising: bool) -> None:
+    class Descriptor:
+        def __get__(self, instance, owner):
+            raise RuntimeError("lookup")
+
+    class Parent:
+        value = Descriptor()
+
+    class Child(Parent):
+        pass
+
+    with MonkeyPatch.context() as mp:
+        if raising:
+            with pytest.raises(RuntimeError, match="lookup"):
+                mp.setattr(Child, "value", 99)
+        else:
+            mp.setattr(Child, "value", 99, raising=False)
+            assert Child.value == 99
+    assert "value" not in vars(Child)
+
+
+def test_non_raising_dynamic_attribute() -> None:
+    class Target:
+        def __getattr__(self, name):
+            if name == "value":
+                return 42
+            raise AttributeError(name)
+
+    obj = Target()
+    with MonkeyPatch.context() as mp:
+        mp.setattr(obj, "value", 99, raising=False)
+        assert obj.value == 99
+    assert "value" not in vars(obj)
+    assert obj.value == 42
+    with pytest.raises(AttributeError, match="missing"):
+        _ = obj.missing
+
+
+@pytest.mark.parametrize("operation", ["setattr", "delattr"])
+def test_non_raising_slot_undo(operation: str) -> None:
+    class Target:
+        __slots__ = ("value",)
+        value: int
+
+    obj = Target()
+    obj.value = 42
+    with MonkeyPatch.context() as mp:
+        if operation == "setattr":
+            mp.setattr(obj, "value", 99, raising=False)
+            assert obj.value == 99
+        else:
+            mp.delattr(obj, "value", raising=False)
+            assert not hasattr(obj, "value")
+    assert obj.value == 42