Skip to content

Fix monkeypatch undo when deleting missing attrs/items with raising=False#14271

Open
kartikdp wants to merge 1 commit intopytest-dev:mainfrom
kartikdp:bugfix/monkeypatch-restore-notset
Open

Fix monkeypatch undo when deleting missing attrs/items with raising=False#14271
kartikdp wants to merge 1 commit intopytest-dev:mainfrom
kartikdp:bugfix/monkeypatch-restore-notset

Conversation

@kartikdp
Copy link

@kartikdp kartikdp commented Mar 7, 2026

Summary

Fixes a state-restoration bug in MonkeyPatch when deleting a missing attribute/item with raising=False.

Previously:

  • delattr(..., raising=False) and delitem(..., raising=False) did not record the missing state.
  • If the test then added that attribute/item, undo() would not reliably restore the original missing state.

This change records NOTSET for those delete calls, and makes undo() tolerant when the target attribute is already absent.

Closes #14094.

Why this matters

Tests that use non-raising deletes expect monkeypatch teardown to fully restore pre-test state. Without this fix, state could leak after tests.

How tested

  • uv run -m pytest testing/test_monkeypatch.py -k "delattr_non_existing_with_raising_false or delitem_non_existing_with_raising_false or test_delattr or test_delitem"
  • uv run -m pytest testing/test_monkeypatch.py
  • uv run --with pre-commit pre-commit run --files src/_pytest/monkeypatch.py testing/test_monkeypatch.py changelog/14094.bugfix.rst

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Mar 7, 2026
assert A.x == 1


def test_delattr_non_existing_with_raising_false() -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test already passes on main, so it is not reflecting/reproducing the issue.

Did you intend to use setattr directly to set x (instead of monkeypatch.setattr).

assert d == {"hello": "world", "x": 1}


def test_delitem_non_existing_with_raising_false() -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as the other test.

@@ -0,0 +1 @@
Fixed ``MonkeyPatch.delattr(..., raising=False)`` and ``MonkeyPatch.delitem(..., raising=False)`` so ``undo()`` restores the original missing state.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is a bug-fix, seems like an improvement: now delattr will always delete the attribute at the end of the test.

This is a behavior change which I don't think can cause problems later, as it depends on the user explicitly calling monkeydelattr.delattr(..., raising=False).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@The-Compiler you commented on the issue; are you OK with this?

@nicoddemus nicoddemus requested a review from The-Compiler March 13, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Monkeypatch.delitem/delattr does not keep track of non-existing properties

2 participants