From 0059d1584a39983995008651b5afeea238ecf3d6 Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Fri, 25 Sep 2026 00:06:24 +0530 Subject: [PATCH 1/6] fix(monkeypatch): restore attributes on objects with custom __setattr__ In #14969, MonkeyPatch.setattr was changed to record the old value from the instance __dict__ instead of getattr() to avoid leaving behind an inherited class attribute in the instance dict upon undo(). However, that change assumed setattr() always writes into the instance __dict__. When an object defines a custom __setattr__, attribute storage is handled by custom machinery and attributes may not exist in __dict__. Consequently, oldval was recorded as NOTSET, causing undo() to call delattr(), raising AttributeError and leaving the attribute patched. Check that type(target).__setattr__ is object.__setattr__ before looking in the instance __dict__. When __setattr__ is overridden, retain the resolved getattr() value so undo() restores it through setattr(). Closes #15099. Co-authored-by: Antigravity --- AUTHORS | 1 + changelog/15099.bugfix.rst | 1 + src/_pytest/monkeypatch.py | 19 ++++++++++++------- testing/test_monkeypatch.py | 31 +++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 changelog/15099.bugfix.rst diff --git a/AUTHORS b/AUTHORS index e2fad5e8364..d97fca162af 100644 --- a/AUTHORS +++ b/AUTHORS @@ -206,6 +206,7 @@ Guido Wesdorp Guoqiang Zhang Hamza Mobeen Harald Armin Massa +Hardik Kaurani Harshna Henk-Jaap Wagenaar Henry Schreiner diff --git a/changelog/15099.bugfix.rst b/changelog/15099.bugfix.rst new file mode 100644 index 00000000000..1fc71bf29f2 --- /dev/null +++ b/changelog/15099.bugfix.rst @@ -0,0 +1 @@ +``monkeypatch.setattr()`` now correctly restores attributes on objects with a custom ``__setattr__``. diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index 453c6728ee2..2e09f5a31f0 100644 --- a/src/_pytest/monkeypatch.py +++ b/src/_pytest/monkeypatch.py @@ -258,13 +258,18 @@ def setattr( # avoid class descriptors like staticmethod/classmethod if inspect.isclass(target): oldval = target.__dict__.get(name, NOTSET) - elif not _is_data_descriptor(type(target), name): - # With no data descriptor in the way, the `setattr()` below writes - # into the instance `__dict__`, so `undo()` has to restore that - # `__dict__` entry. Assigning an inherited `oldval` back onto the - # instance would instead leave behind a new entry shadowing the - # class attribute, which permanently freezes descriptors that - # resolve dynamically (#10644). + elif ( + type(target).__setattr__ is object.__setattr__ + and not _is_data_descriptor(type(target), name) + ): + # With no data descriptor in the way and default `object.__setattr__`, + # the `setattr()` below writes into the instance `__dict__`, so + # `undo()` has to restore that `__dict__` entry. Assigning an + # inherited `oldval` back onto the instance would instead leave behind + # a new entry shadowing the class attribute, which permanently freezes + # descriptors that resolve dynamically (#10644). + # When `__setattr__` is overridden, attribute setting is routed through + # custom machinery which may store values elsewhere (#15099). target_dict = getattr(target, "__dict__", None) if isinstance(target_dict, Mapping): oldval = target_dict.get(name, NOTSET) diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index 0d07783b05b..97601602638 100644 --- a/testing/test_monkeypatch.py +++ b/testing/test_monkeypatch.py @@ -607,6 +607,37 @@ def __init__(self) -> None: assert obj.x == 1 +def test_undo_custom_setattr_on_instance() -> None: + """An object with custom __setattr__ must restore its old value on undo. + + See #15099. + """ + + class Config: + """Stores attributes in a private dict instead of __dict__.""" + + def __init__(self) -> None: + object.__setattr__(self, "_data", {"debug": False}) + + def __getattr__(self, name: str) -> object: + try: + return self._data[name] # type: ignore[attr-defined] + except KeyError: + raise AttributeError(name) from None + + def __setattr__(self, name: str, value: object) -> None: + self._data[name] = value # type: ignore[attr-defined] + + cfg = Config() + monkeypatch = MonkeyPatch() + + monkeypatch.setattr(cfg, "debug", True) + assert cfg.debug is True + + monkeypatch.undo() + assert cfg.debug is False + + def test_issue1338_name_resolving() -> None: pytest.importorskip("requests") monkeypatch = MonkeyPatch() From 51d1d75fa37a986b31b8fac21c30856bd74f60b6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:37:03 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/_pytest/monkeypatch.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index 2e09f5a31f0..a2317a03d32 100644 --- a/src/_pytest/monkeypatch.py +++ b/src/_pytest/monkeypatch.py @@ -258,9 +258,8 @@ def setattr( # avoid class descriptors like staticmethod/classmethod if inspect.isclass(target): oldval = target.__dict__.get(name, NOTSET) - elif ( - type(target).__setattr__ is object.__setattr__ - and not _is_data_descriptor(type(target), name) + elif type(target).__setattr__ is object.__setattr__ and not _is_data_descriptor( + type(target), name ): # With no data descriptor in the way and default `object.__setattr__`, # the `setattr()` below writes into the instance `__dict__`, so From 8d1f543f973da02eda6f90f6ed276d3751efab84 Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Fri, 25 Sep 2026 01:02:01 +0530 Subject: [PATCH 3/6] test(monkeypatch): fix typing and cover missing attr in custom __setattr__ test --- testing/test_monkeypatch.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index 97601602638..6a8b0ba23ea 100644 --- a/testing/test_monkeypatch.py +++ b/testing/test_monkeypatch.py @@ -616,21 +616,26 @@ def test_undo_custom_setattr_on_instance() -> None: class Config: """Stores attributes in a private dict instead of __dict__.""" + _data: dict[str, object] + def __init__(self) -> None: object.__setattr__(self, "_data", {"debug": False}) def __getattr__(self, name: str) -> object: try: - return self._data[name] # type: ignore[attr-defined] + return self._data[name] except KeyError: raise AttributeError(name) from None def __setattr__(self, name: str, value: object) -> None: - self._data[name] = value # type: ignore[attr-defined] + self._data[name] = value cfg = Config() monkeypatch = MonkeyPatch() + with pytest.raises(AttributeError, match="has no attribute 'nonexistent'"): + monkeypatch.setattr(cfg, "nonexistent", True) + monkeypatch.setattr(cfg, "debug", True) assert cfg.debug is True From 1d47a2ef9be22790bb083d0837a4352d64b0dc76 Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Sat, 26 Sep 2026 09:23:05 +0530 Subject: [PATCH 4/6] monkeypatch: tolerate AttributeError in undo() for absent attributes on custom objects --- src/_pytest/monkeypatch.py | 5 ++++- testing/test_monkeypatch.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index a2317a03d32..ca35b15b05d 100644 --- a/src/_pytest/monkeypatch.py +++ b/src/_pytest/monkeypatch.py @@ -438,7 +438,10 @@ def undo(self) -> None: if value is not NOTSET: setattr(obj, name, value) else: - delattr(obj, name) + try: + delattr(obj, name) + except AttributeError: + pass self._setattr[:] = [] for dictionary, key, value in reversed(self._setitem): if value is NOTSET: diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index 6a8b0ba23ea..79947261f52 100644 --- a/testing/test_monkeypatch.py +++ b/testing/test_monkeypatch.py @@ -630,17 +630,35 @@ def __getattr__(self, name: str) -> object: def __setattr__(self, name: str, value: object) -> None: self._data[name] = value + def __delattr__(self, name: str) -> None: + try: + del self._data[name] + except KeyError: + raise AttributeError(name) from None + cfg = Config() monkeypatch = MonkeyPatch() with pytest.raises(AttributeError, match="has no attribute 'nonexistent'"): monkeypatch.setattr(cfg, "nonexistent", True) + # Test setting a missing attribute with raising=False + monkeypatch.setattr(cfg, "nonexistent", True, raising=False) + assert cfg.nonexistent is True + + # Test setting an existing attribute monkeypatch.setattr(cfg, "debug", True) assert cfg.debug is True monkeypatch.undo() assert cfg.debug is False + assert not hasattr(cfg, "nonexistent") + + # Test that undoing an already-deleted attribute doesn't raise (the AttributeError tolerance) + monkeypatch.setattr(cfg, "brand_new", True, raising=False) + delattr(cfg, "brand_new") + monkeypatch.undo() + assert not hasattr(cfg, "brand_new") def test_issue1338_name_resolving() -> None: From d5b1be21b1d4e5193e4ed583cf6488ef9212970f Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Sat, 26 Sep 2026 09:31:42 +0530 Subject: [PATCH 5/6] monkeypatch: add regression test covering exact reviewer reproduction --- testing/test_monkeypatch.py | 44 ++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index 79947261f52..86dbc3931ee 100644 --- a/testing/test_monkeypatch.py +++ b/testing/test_monkeypatch.py @@ -607,12 +607,50 @@ def __init__(self) -> None: assert obj.x == 1 -def test_undo_custom_setattr_on_instance() -> None: - """An object with custom __setattr__ must restore its old value on undo. +def test_undo_custom_setattr_without_delattr() -> None: + """An object with custom __setattr__ but NO __delattr__ should not crash on undo. - See #15099. + See #15099 maintainer review. Since the object doesn't support deletion, + undo() catches AttributeError and tolerates it, though it cannot physically remove the attribute. """ + class ConfigNoDel: + _data: dict[str, object] + + def __init__(self) -> None: + object.__setattr__(self, "_data", {}) + + def __getattr__(self, name: str) -> object: + try: + return self._data[name] + except KeyError: + raise AttributeError(name) from None + + def __setattr__(self, name: str, value: object) -> None: + self._data[name] = value + + cfg = ConfigNoDel() + monkeypatch = MonkeyPatch() + + # Attribute initially absent + assert not hasattr(cfg, "brand_new") + + # Set missing attribute with raising=False + monkeypatch.setattr(cfg, "brand_new", True, raising=False) + assert cfg.brand_new is True + + # Undo should not raise AttributeError, even though delattr(cfg, "brand_new") fails + monkeypatch.undo() + + # Note: We CANNOT assert not hasattr(cfg, "brand_new") here, because without + # a custom __delattr__, monkeypatch has no generic way to remove the attribute + # from the custom storage (_data). The tolerance avoids the teardown crash. + assert cfg.brand_new is True + + +def test_undo_custom_setattr_with_delattr() -> None: + """An object with custom __setattr__ and __delattr__ must restore its old value on undo.""" + class Config: """Stores attributes in a private dict instead of __dict__.""" From 85c512a38a76627c0a5770dd356571a848d29503 Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Sat, 26 Sep 2026 09:45:55 +0530 Subject: [PATCH 6/6] monkeypatch tests: avoid mypy type narrowing issue by using getattr --- testing/test_monkeypatch.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index 86dbc3931ee..3e5587373c6 100644 --- a/testing/test_monkeypatch.py +++ b/testing/test_monkeypatch.py @@ -682,14 +682,14 @@ def __delattr__(self, name: str) -> None: # Test setting a missing attribute with raising=False monkeypatch.setattr(cfg, "nonexistent", True, raising=False) - assert cfg.nonexistent is True + assert getattr(cfg, "nonexistent") is True # Test setting an existing attribute monkeypatch.setattr(cfg, "debug", True) - assert cfg.debug is True + assert getattr(cfg, "debug") is True monkeypatch.undo() - assert cfg.debug is False + assert getattr(cfg, "debug") is False assert not hasattr(cfg, "nonexistent") # Test that undoing an already-deleted attribute doesn't raise (the AttributeError tolerance)