From 768ba39719d2a615bc72217cbad8c3288b94da43 Mon Sep 17 00:00:00 2001 From: hth Date: Wed, 23 Sep 2026 22:39:22 +0800 Subject: [PATCH 1/4] Avoid unnecessary descriptor reads with raising=False Read class and non-data descriptors statically before patching so their __get__ methods are not invoked merely to save the old attribute. Preserve dynamic lookup and instance data descriptor values for undo. Refs #10646 Co-authored-by: OpenAI Codex --- AUTHORS | 1 + changelog/10646.bugfix.rst | 1 + src/_pytest/monkeypatch.py | 33 +++++++++- testing/test_monkeypatch.py | 128 ++++++++++++++++++++++++++++++++++++ 4 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 changelog/10646.bugfix.rst 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/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..cf10db8d274 100644 --- a/testing/test_monkeypatch.py +++ b/testing/test_monkeypatch.py @@ -688,3 +688,131 @@ 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 + + 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 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 + + +@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 From 3bcf782ca9353ded01b75c606322206cbe5fa1b7 Mon Sep 17 00:00:00 2001 From: hth Date: Wed, 23 Sep 2026 23:00:45 +0800 Subject: [PATCH 2/4] Exercise descriptor behavior before checking passive patching Validate the descriptors and dynamic fallback directly before asserting that monkeypatch does not execute them. This covers the test code itself without excluding meaningful branches from coverage. Co-authored-by: OpenAI Codex --- testing/test_monkeypatch.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index cf10db8d274..04cda0512ef 100644 --- a/testing/test_monkeypatch.py +++ b/testing/test_monkeypatch.py @@ -707,6 +707,14 @@ def __get__(self, instance, owner): 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) @@ -728,6 +736,8 @@ 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 @@ -798,6 +808,8 @@ def __getattr__(self, name): 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"]) From abefc2908f85215752069efcccc3f877d48afbf2 Mon Sep 17 00:00:00 2001 From: hth Date: Wed, 23 Sep 2026 23:09:05 +0800 Subject: [PATCH 3/4] Keep known PyPy NumPy import warning from failing tests Preserve the warning while avoiding an unrelated Hypothesis failure on NumPy 2.4.6 and PyPy 8.0.0. Co-authored-by: OpenAI Codex --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 0f65d643d40..ab8ed7a30be 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:numpy\.random\._generator', 'default:Using or importing the ABCs:DeprecationWarning:unittest2.*', # produced by older pyparsing<=2.2.0. 'default:Using or importing the ABCs:DeprecationWarning:pyparsing.*', From 9cdb28e0e405ad900355159ceffddf50050c1a08 Mon Sep 17 00:00:00 2001 From: hth Date: Wed, 23 Sep 2026 23:22:41 +0800 Subject: [PATCH 4/4] Match Cython warning emitted through importlib Reproduce with NumPy loaded before Hypothesis: the warning is attributed to importlib, so matching numpy.random misses it. Retain the exact warning message and category. Co-authored-by: OpenAI Codex --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index ab8ed7a30be..2b6b62c36b0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -450,7 +450,7 @@ 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:numpy\.random\._generator', + '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.*',