diff --git a/AUTHORS b/AUTHORS index 80b6d5157..d254436a8 100644 --- a/AUTHORS +++ b/AUTHORS @@ -159,6 +159,7 @@ Floris Bruynooghe Fraser Stark Gabriel Landau Gabriel Reis +Ganden Schaffner Garvit Shubham Gene Wood George Kussumoto diff --git a/changelog/10644.bugfix.rst b/changelog/10644.bugfix.rst new file mode 100644 index 000000000..15b4019d3 --- /dev/null +++ b/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. diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index f498d60df..408fd6602 100644 --- a/src/_pytest/monkeypatch.py +++ b/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) diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index 2ad3ccc4d..81f737c98 100644 --- a/testing/test_monkeypatch.py +++ b/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}