diff --git a/AUTHORS b/AUTHORS index e2fad5e8364..744d10570e2 100644 --- a/AUTHORS +++ b/AUTHORS @@ -210,6 +210,7 @@ Harshna Henk-Jaap Wagenaar Henry Schreiner Holger Kohr +Hou,Ting-Han Hugo van Kemenade Hui Wang (coldnight) Ian Bicking diff --git a/changelog/10646.bugfix.rst b/changelog/10646.bugfix.rst new file mode 100644 index 00000000000..400929be33a --- /dev/null +++ b/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. diff --git a/pyproject.toml b/pyproject.toml index e7dcef6e007..7d71e2bf2da 100644 --- a/pyproject.toml +++ b/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.*', diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index 453c6728ee2..925166819c4 100644 --- a/src/_pytest/monkeypatch.py +++ b/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) diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index 0d07783b05b..04cda0512ef 100644 --- a/testing/test_monkeypatch.py +++ b/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