Monkey patching frozen attrs instances

[sscherfke]

What's the problem this feature will solve?

It is generally good [citation needed] to use frozen attrs clases to avoid bugs related to unintended alterations of class instances.

To change frozen instances, there is the evolve() function which returns a new instances with the updated attribute(s).

However, in tests you sometimes may want to monkey patch a frozen object in place.

For example, with Typed Settings you usually end up with a frozen SETTINGS object (which is an instance of your Settings class). Other modules import this object directly:

from .settings import SETTINGS

def spam():
    print(SETTINGS.my_option)

which is more convenient than:

from . import settings

def spam():
    print(settings.SETTINGS.my_option)

Since using monkeypatch.setattr() would raise a FrozenError, you need to use evolve(). But this does not work very well because you create an enirely new instances and need to monkeypatch all modules that import SETTINGS.

Describe the solution you'd like

monkeypatch.setattr() automatically (or explicitly) handles frozen attrs instances and sets an attribute (which is technically possible, see below).

Alternative Solutions

I could bundle a stripped down version of pytests monkey patcher but that would be less convenient for users.

Additional context

You can set attributes in frozen instances using object.__setattr__(target, name, value).

The changes to pytest could look like this:

Implicit version

diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py
index 31f95a95a..e71d5ddf9 100644
--- a/src/_pytest/monkeypatch.py
+++ b/src/_pytest/monkeypatch.py
@@ -14,6 +14,8 @@ from typing import Tuple
 from typing import TypeVar
 from typing import Union
 
+from attr.exceptions import FrozenError
+
 from _pytest.compat import final
 from _pytest.fixtures import fixture
 from _pytest.warning_types import PytestWarning
@@ -221,7 +223,10 @@ class MonkeyPatch:
         if inspect.isclass(target):
             oldval = target.__dict__.get(name, notset)
         self._setattr.append((target, name, oldval))
-        setattr(target, name, value)
+        try:
+            setattr(target, name, value)
+        except FrozenError:
+            object.__setattr__(target, name, value)
 
     def delattr(
         self,
@@ -361,7 +366,10 @@ class MonkeyPatch:
         """
         for obj, name, value in reversed(self._setattr):
             if value is not notset:
-                setattr(obj, name, value)
+                try:
+                    setattr(obj, name, value)
+                except FrozenError:
+                    object.__setattr__(obj, name, value)
             else:
                 delattr(obj, name)
         self._setattr[:] = []

Explicit version

diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py
index 31f95a95a..39baa2828 100644
--- a/src/_pytest/monkeypatch.py
+++ b/src/_pytest/monkeypatch.py
@@ -14,6 +14,8 @@ from typing import Tuple
 from typing import TypeVar
 from typing import Union
 
+from attr.exceptions import FrozenError
+
 from _pytest.compat import final
 from _pytest.fixtures import fixture
 from _pytest.warning_types import PytestWarning
@@ -181,6 +183,7 @@ class MonkeyPatch:
         name: Union[object, str],
         value: object = notset,
         raising: bool = True,
+        frozen: bool = False,
     ) -> None:
         """Set attribute value on target, memorizing the old value.
 
@@ -221,7 +224,13 @@ class MonkeyPatch:
         if inspect.isclass(target):
             oldval = target.__dict__.get(name, notset)
         self._setattr.append((target, name, oldval))
-        setattr(target, name, value)
+        try:
+            setattr(target, name, value)
+        except FrozenError:
+            if frozen:
+                object.__setattr__(target, name, value)
+            else:
+                raise
 
     def delattr(
         self,
@@ -361,7 +370,12 @@ class MonkeyPatch:
         """
         for obj, name, value in reversed(self._setattr):
             if value is not notset:
-                setattr(obj, name, value)
+                try:
+                    setattr(obj, name, value)
+                except FrozenError:
+                    # If we did set an attribute on a frozen instance, we surely can undo
+                    # these changes. :)
+                    object.__setattr__(obj, name, value)
             else:
                 delattr(obj, name)
         self._setattr[:] = []

[sscherfke]

Let's hope, @hynek will not hit me with a stick for this request. 🤞 🙊

[RonnyPfannschmidt]

@sscherfke from my pov monkeypatching frozen attrs instance is about the same type of change as monkeypatching tuples

one does not simply monkeypatch frozen instances i'm -1 on the capability

[RonnyPfannschmidt]

@sscherfke you may however provide a pytest-snowwhite plugin that provides a butter_knive fixture to butter things up

[sscherfke]

@sscherfke you may however provide a pytest-snowwhite plugin that provides a butter_knive fixture to butter things up

What is a pytest-snowwhite plugin? 🤔

[sscherfke]

@RonnyPfannschmidt Do you mean something like this: https://gitlab.com/sscherfke/typed-settings/-/issues/12?

[RonnyPfannschmidt]

@sscherfke i consider that considerably worse

[sscherfke]

Do you have an idea how a better solution might look like? 😇

[RonnyPfannschmidt]

Instead of monkeypatching globals, use evolved config objects,

Monkeypatching seemingly immutable configuration never worked out well for staleness issues that will get you

[sscherfke]

But that’s the whole point of this issue. If I use evolve(), I have to monkeypatch the evolved instance in all modules that directly imported it (see the example above).

I know that monkeypatching a frozen instance is hacky, but monkeypatching by itself is hacky, too. A "Frozen Monkey Patcher" would IMHO give users safety at runtime (because they cannot acidentally alter settings) and flexibility at testing time (because they can monkey patch settings objects in-place).

[RonnyPfannschmidt]

ok, based on the information provided i have to reject the feature

what you outline is that you want to monkeypatch "frozen" objects that are global while running tests,
this regularly creates situations of wrongly configured objects in other tests, as such its a bad practice

something like that should not go to pytest core and if its packaged as pytest plugin it certainly shouldn't have a positive name

[hynek]

FWIW, in cases like this I circumvent the problem by not making users import global shit into their apps but hide it behind a function call
that they use on demand. If you then really need to monkey-patch a global symbol, it's private and has a single origin.

[sscherfke]

@RonnyPfannschmidt what you outline is that you want to monkeypatch "frozen" objects that are global while running tests, this regularly creates situations of wrongly configured objects in other tests, as such its a bad practice

This is no different from "normal" (mutable) global config objects that you would monkey patch in a test and undo the patch when the tests finishes. The only difference here is, that the object is "marked immutable" at runtime so that program code cannot accidentally change it.

@hynek: FWIW, in cases like this I circumvent the problem by not making users import global shit into their apps but hide it behind a function call that they use on demand. If you then really need to monkey-patch a global symbol, it's private and has a single origin.

Users don't import shit (from others) into their app, they define global shit in their own app: They have a settings module that defines the settings (as an attrs class) and loads them into a module scoped (global) variable:

# src/myapp/settings.py
import typed_settings as ts

@ts.settings(frozen=True)
class Settings:
    option: str

SETTINGS = ts.load(Settings, "myapp")

# src/myapp/spam.py
from .settings import SETTINGS

def spam():
    print(SETTINGS.option)

This would avoid the issues with evolve() but would make accessing settings "harder" (more typing, less readbility):

# src/myapp/spam.py
from . import settings

def spam():
    print(settings.SETTINGS.option)

This might work, but I think its to complicated, too:

# src/myapp/settings.py
import typed_settings as ts

@ts.settings(frozen=True)
class Settings:
    option: str

_SETTINGS = None

def settings():
    global _SETTINGS
    if _SETTINGS is None:
        _SETTINGS = ts.load(Settings, "myapp")
    return _SETTINGS

# src/myapp/spam.py
from .settings import settings

def spam():
    print(settings().option)

---

Maybe, ts.load() needs to return not an instances of Settings but some kind of a magic wrapper. The longer I think about how this magic wrappr would look like, the less I like that idea.

[RonnyPfannschmidt]

So if any approach to make this nice without putting a daunting hack into pytest is turning your stomach

What makes it OK to put a hack into pytest that makes all frozen classes "fair game"

Please leave the cost of global configuration at their origin