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")