From 82557504ba98439e5c2991ec894e115c65888345 Mon Sep 17 00:00:00 2001 From: Ganden Schaffner Date: Mon, 9 Jan 2023 00:00:00 -0800 Subject: [PATCH] Fix MonkeyPatch.setattr/delattr binding descriptors when raising=False --- changelog/10646.bugfix.rst | 3 +++ src/_pytest/monkeypatch.py | 24 ++++++++++++------------ 2 files changed, 15 insertions(+), 12 deletions(-) create mode 100644 changelog/10646.bugfix.rst diff --git a/changelog/10646.bugfix.rst b/changelog/10646.bugfix.rst new file mode 100644 index 000000000..bfc27bb7c --- /dev/null +++ b/changelog/10646.bugfix.rst @@ -0,0 +1,3 @@ +Fix bug where `monkeypatch.setattr` and `monkeypatch.delattr` bind descriptors (possibly causing unwanted side-effects) to implement their `hasattr` checks even when their checks are disabled by `raising=False`. + +Note that they still must bind descriptors to implement their checks if `raising=True`. diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index dc255357e..47fb4351f 100644 --- a/src/_pytest/monkeypatch.py +++ b/src/_pytest/monkeypatch.py @@ -241,8 +241,7 @@ class MonkeyPatch: "import string" ) - oldval = getattr(target, name, notset) - if raising and oldval is notset: + if raising and not hasattr(target, name): raise AttributeError(f"{target!r} has no attribute {name!r}") # Prevent `undo` from polluting `vars(target)` with an object that was not in it @@ -268,7 +267,6 @@ class MonkeyPatch: ``raising`` is set to False. """ __tracebackhide__ = True - import inspect if isinstance(name, Notset): if not isinstance(target, str): @@ -279,16 +277,18 @@ class MonkeyPatch: ) name, target = derive_importpath(target, raising) - if not hasattr(target, name): - if raising: - raise AttributeError(f"{target!r} has no attribute {name!r}") - else: - oldval = getattr(target, name, notset) - # Avoid class descriptors like staticmethod/classmethod. - if inspect.isclass(target): - oldval = target.__dict__.get(name, notset) - self._setattr.append((target, name, oldval)) + if raising and not hasattr(target, name): + raise AttributeError(f"{target!r} has no attribute {name!r}") + + # Prevent `undo` from overwriting class descriptors (like + # staticmethod/classmethod) with the results of descriptor binding. + oldval = vars(target).get(name, notset) + try: delattr(target, name) + except AttributeError: + pass + else: + self._setattr.append((target, name, oldval)) def setitem(self, dic: MutableMapping[K, V], name: K, value: V) -> None: """Set dictionary entry ``name`` to value."""