From ec1f0fcb85c83ec82e950b48fa42491f0af6e4ac Mon Sep 17 00:00:00 2001 From: Friday Date: Wed, 18 Feb 2026 04:12:28 +0000 Subject: [PATCH] Fix monkeypatch.setattr() teardown crash on failed setattr (#14161) When setattr() fails (e.g. because the target is immutable like None), the undo stack already had the entry appended before the setattr call. This left a stale (target, name, NOTSET) entry that would crash during teardown with AttributeError trying to delattr on the target. Fix by moving the _setattr.append() after the setattr() call, so failed setattr operations don't pollute the undo stack. Co-Authored-By: Claude Opus 4.6 --- changelog/14161.bugfix.rst | 1 + src/_pytest/monkeypatch.py | 2 +- testing/test_monkeypatch.py | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 changelog/14161.bugfix.rst diff --git a/changelog/14161.bugfix.rst b/changelog/14161.bugfix.rst new file mode 100644 index 00000000000..d19f3f4ad60 --- /dev/null +++ b/changelog/14161.bugfix.rst @@ -0,0 +1 @@ +Fixed :meth:`monkeypatch.setattr() ` leaving a stale entry on the undo stack when the underlying ``setattr()`` call fails (e.g. on immutable targets), causing an ``AttributeError`` crash during teardown. diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index 6c033f36fda..d6db72455a8 100644 --- a/src/_pytest/monkeypatch.py +++ b/src/_pytest/monkeypatch.py @@ -244,8 +244,8 @@ def setattr( # avoid class descriptors like staticmethod/classmethod if inspect.isclass(target): oldval = target.__dict__.get(name, NOTSET) - self._setattr.append((target, name, oldval)) setattr(target, name, value) + self._setattr.append((target, name, oldval)) def delattr( self, diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index c321439e398..6446d2ec1bb 100644 --- a/testing/test_monkeypatch.py +++ b/testing/test_monkeypatch.py @@ -89,6 +89,22 @@ def test_unknown_attr_non_raising(self, monkeypatch: MonkeyPatch) -> None: mp.setattr("os.path.qweqwe", 42, raising=False) assert os.path.qweqwe == 42 # type: ignore + def test_setattr_failure_does_not_corrupt_undo( + self, monkeypatch: MonkeyPatch + ) -> None: + """If setattr() raises (e.g. target doesn't support attribute + setting), the undo stack should not contain a stale entry that + crashes during teardown (#14161).""" + + class Immutable: + __slots__ = () + + target = Immutable() + with pytest.raises(AttributeError): + monkeypatch.setattr(target, "x", 42, raising=False) + # undo() must not raise — no entry should be on the undo stack. + monkeypatch.undo() + def test_delattr(self, monkeypatch: MonkeyPatch) -> None: with monkeypatch.context() as mp: mp.delattr("os.path.abspath")