Repository navigation
monkeypatch.setattr fails to undo on objects with a custom __setattr__ (regression from #14969) #15099
Description
Since #14969 (0c601d5, not released yet), monkeypatch.setattr doesn't restore attributes on objects that store them somewhere other than __dict__ through a custom __setattr__/__getattr__. Undo raises AttributeError, and the patched value leaks into later tests.
class Config:
"""Stores attributes in a private dict instead of __dict__."""
def __init__(self):
object.__setattr__(self, "_data", {"debug": False})
def __getattr__(self, name):
try:
return self._data[name]
except KeyError:
raise AttributeError(name) from None
def __setattr__(self, name, value):
self._data[name] = value
cfg = Config()
def test_patch(monkeypatch):
monkeypatch.setattr(cfg, "debug", True)
assert cfg.debug is True
def test_restored():
assert cfg.debug is FalseOn main:
ERROR test_proxy.py::test_patch - AttributeError: 'Config' object has no attribute 'debug'
FAILED test_proxy.py::test_restored - assert True is False
1 failed, 1 passed, 1 error
Just before 0c601d5, the same file passes (2 passed).
The new branch in MonkeyPatch.setattr takes the old value from target.__dict__.get(name, NOTSET) whenever target has a __dict__. It assumes that a plain setattr() writes into that dict. With a custom __setattr__ it doesn't, so the old value is recorded as NOTSET. undo() then calls delattr(cfg, "debug") instead of setting it back to False. That delattr fails, and the value stays patched.
@Shriprasad-P pointed out this exact case in a review on #14969, but it wasn't addressed before the merge. I'm opening this issue so it gets fixed before the next release.
One possible direction: only use the __dict__ lookup when name is actually present in the instance __dict__, or when the type uses object.__setattr__. Otherwise fall back to the value from getattr(), as before.
pytest 9.2.0.dev345+g872117358, Python 3.13.5, Windows 11.
Activity
Tejas5405 commentedon Oct 1, 2026
Reproduced on main (9.2.0.dev450, Python 3.13):
undo()raisesAttributeError: 'Config' object has no attribute 'debug'and the patched value leaks into the next test.The
__dict__lookup added in 0c601d5 assumes thatsetattr()writes into the instance__dict__, which is not true when the class defines__setattr__. For those objects the old value has to come fromgetattr()and be restored through the same hook, so the old value is wrongly recorded asNOTSETandundo()tries to delete the attribute.I'll send a pull request which keeps the
__dict__lookup for the case it was added for (a plainobject.__setattr__), with a regression test and a changelog entry.Disclosure per the project's AI/LLM policy: this is an AI-assisted contribution; the pull request states what was machine-produced and what I verified manually.
fix(monkeypatch): restore attributes of objects with a custom __setat…
fix(monkeypatch): restore attributes of objects with a custom __setat…
DawnofGenX commentedon Oct 4, 2026
I'd like to work on this.
The fix direction is clear: only use the
__dict__lookup whennameis actually present in the instance__dict__, or when the type usesobject.__setattr__. Otherwise fall back to the value fromgetattr(), as before.I'll add a regression test and implement the fix.
Tejas5405 commentedon Oct 4, 2026
Heads up: this already has an open PR — #15118, up since 2026-10-01, with the regression test and a changelog entry.
Your proposed direction matches what it does (
__dict__lookup only when the name is really there or the type usesobject.__setattr, otherwise fall back togetattr()). Worth checking that first. Happy to coordinate here if you still want to take the regression-test side.DawnofGenX commentedon Oct 7, 2026
Thanks @Tejas5405 — noted. I did end up going with #15130 for this (merged and green); leaving #15118 to the maintainers to reconcile. Nothing further from me here.
DawnofGenX commentedon Oct 7, 2026
Correction to my previous comment: I mis-stated that #15130 was merged — it is still open (green, 36/36 checks). So there are two open PRs for this: #15118 and #15130. Leaving it to the maintainers to pick one; I'll stand down on whichever they don't take.
DawnofGenX commentedon Oct 7, 2026
Second correction, and this one matters: #15118 was opened Oct 1 by @Tejas5405, three days before my #15130 (Oct 4) — and @Tejas5405 flagged exactly that on Oct 4, which I did not act on. So mine is the later duplicate, not the other way round. I've closed #15130 to leave the field to #15118.