Repository: kaste/mockito-python

Issue:
deepcopy doesn't work with mock()
It seems to me `mock()` is transformed to `None` by `deepcopy`:
```
from copy import deepcopy
from mockito import mock

m = mock()
print(f'{m=} {deepcopy(m)=}')
```

Which gives:
```
m=<Dummy id=140252977120144> deepcopy(m)=None
```

Is there some way around this?

Discussion:
@kaste: How's that even possible?  I was not aware of deep copy ever returning None for anything.  Implementing the deepcopy dunder method could be the obvious solution.@kaste: Ok, so here's what's happening.  Since the mock intercepts all attribute accesses, they intercept the `__deepcopy__`  call made by `deepcopy` as well.  This call is unconfigured so it returns the default callable which returns `None` for any argument.  (Just like all the mocks do.)  (The mocks do intercept all dunder methods so users can override ("stub"/"patch") them like you can override the `__call__` call.)

a) Mocks check explicitly for the `__deepcopy__` access and return `None` instead of the generic dispatcher. This would require a new version of mockito.  Idk if this would be okay for all possible objects.  
b) You configure the `mock`, e.g. `mock({"__deepcopy__": None})`.  This makes the object-to-object interaction explicit too.  E.g. in plain words: you expect that `deepcopy` gets called in your test/mock setup.
c) You stub `copy.deepcopy` to expect a call with the arg of the mock under test, and then to return something.  (In other words, the standard `when(copy).deepcopy(m).thenReturn(n)`.  That way, you also make the interaction (or "usage") of `m` within your program explicit.

Note that when you do `mock({"__deepcopy__": None})` a subsequent `deepcopy(m)` will return something, but it will not be a true *deep* copy, it would reference the same underlying state.  



@jan-spurny: Thanks!
That makes sense - (c) solves all my needs and even though I'm not sure where exactly this lies on bug/feature scale, I'm closing it.@kaste: I tend to like that an interaction with *deepcopy* or *copy* gets recorded and has to be expected so to speak.  After all, deepcopy is also not a property of an object that is just there working for user defined classes in all cases.  Maybe we should intercept the `__deepcopy__` call and print the standard message "`__deepcopy__` is not configured for mock `n`".  

I'm also unsure what a *deep* copy of a (dumb) mock should look like.  Sure it can't be configured or all interactions with it needs to be separate from the original object.  Is a deep copy essentially `when(m).__deepcopy__(...).thenReturn(mock())`, t.i. just a new *mock*?@jan-spurny: I think warning is the way to go - if I got the warning few days ago, that would've saved me a lot of time :)  - and as you've said, it's probably impossible to guess what mock should return on deepcopy - that should be decided by a person who's using it.@kaste: Found the time to fix this -> #91 
Released as 1.5.4  🏭 

Solution patch:
diff --git a/docs/recipes.rst b/docs/recipes.rst
index 6a43efa..4cba021 100644
--- a/docs/recipes.rst
+++ b/docs/recipes.rst
@@ -73,3 +73,28 @@ It's basically the same problem, but we need to add support for the context mana
         assert res.text == 'Ok'
 
 
+Deepcopies
+----------
+
+Python's `deepcopy` is tied to `__deepcopy__`, in a nutshell `deepcopy(m)` will call `m.__deepcopy__()`.
+For a strict mock, `deepcopy(m)` will raise an error as long as the call is unexpected -- as usual.
+
+While you could completely fake it -- `when(m).__deepcopy__(...).thenReturn(42)` -- you could also enable
+the standard implementation by configuring the mock, e.g. `mock({"__deepcopy__": None}, strict=True)`.
+
+Dumb mocks are copied correctly by default.
+
+However, there is a possible catch: deep mutable objects must be set on the mock's instance, not the class.
+And the constructors configuration is set on the class, not the instance.  Huh?  Let's show an example:
+
+    m = mock()
+    m.foo = [1]  # <= this is set on the instance, not the class
+
+    m = mock({"foo": [1]})  # <= this is set on the class, not the instance
+
+Don't rely on that latter "feature", initially the configurataion was meant to only set methods, and especially
+special, dunder methods, -- and properties.  If we get proper support for properties, we'll likely make a change
+here too.
+
+Btw, `copy` will *just work* for strict mocks and does not raise an error when not configured/expected.  This is
+just not implemented and considered not-worth-the-effort.
diff --git a/mockito/mocking.py b/mockito/mocking.py
index 6aa627d..e1b634e 100644
--- a/mockito/mocking.py
+++ b/mockito/mocking.py
@@ -40,7 +40,7 @@
 ]
 
 
-class _Dummy(object):
+class _Dummy:
     # We spell out `__call__` here for convenience. All other magic methods
     # must be configured before use, but we want `mock`s to be callable by
     # default.
@@ -232,7 +232,7 @@ def __repr__(self):
 
 OMITTED = _OMITTED()
 
-def mock(config_or_spec=None, spec=None, strict=OMITTED):
+def mock(config_or_spec=None, spec=None, strict=OMITTED):  # noqa: C901
     """Create 'empty' objects ('Mocks').
 
     Will create an empty unconfigured object, that you can pass
@@ -295,9 +295,22 @@ def __getattr__(self, method_name):
                 __tracebackhide__ = operator.methodcaller(
                     "errisinstance", AttributeError
                 )
-
-                raise AttributeError(
+                # deepcopy catches a possible AttributeError, fallback
+                # to an arbitrary RuntimeError
+                error_type = (
+                    RuntimeError
+                    if method_name == '__deepcopy__'
+                    else AttributeError
+                )
+                raise error_type(
                     "'Dummy' has no attribute %r configured" % method_name)
+
+            if (
+                method_name != "__call__"
+                and method_name == "__{}__".format(method_name[2:-2])
+            ):
+                raise AttributeError(method_name)
+
             return functools.partial(
                 remembered_invocation_builder, theMock, method_name)
 


Test patch:
diff --git a/tests/deepcopy_test.py b/tests/deepcopy_test.py
new file mode 100644
index 0000000..29f0267
--- /dev/null
+++ b/tests/deepcopy_test.py
@@ -0,0 +1,76 @@
+import pytest
+from copy import copy, deepcopy
+from mockito import mock, when
+
+
+class TestDeepcopy:
+    def test_dumb_mocks_are_copied_correctly(self):
+        m = mock()
+        m.foo = [1]
+        n = deepcopy(m)
+        assert m is not n
+        assert n.foo == [1]
+
+        m.foo.append(2)
+        assert n.foo == [1]
+
+    def test_strict_mocks_raise_on_unexpected_calls(self):
+        m = mock(strict=True)
+        with pytest.raises(RuntimeError) as exc:
+            deepcopy(m)
+        assert str(exc.value) == (
+            "'Dummy' has no attribute '__deepcopy__' configured"
+        )
+
+    def test_configured_strict_mock_answers_correctly(self):
+        m = mock(strict=True)
+        when(m).__deepcopy__(...).thenReturn(42)
+        assert deepcopy(m) == 42
+
+    def test_setting_none_enables_the_standard_implementation(self):
+        m = mock({"__deepcopy__": None}, strict=True)
+        m.foo = [1]
+
+        n = deepcopy(m)
+        assert m is not n
+        assert n.foo == [1]
+
+        m.foo.append(2)
+        assert n.foo == [1]
+
+
+    @pytest.mark.xfail(reason=(
+        "the configuration is set on the mock's class, not the instance, "
+        "which deepcopy does not copy"
+    ))
+    def test_deepcopy_of_a_configured_mock_is_a_new_mock(self):
+        m = mock({"foo": [1]}, strict=True)
+        n = deepcopy(m)
+
+        m.foo.append(2)
+        assert n.foo == [1]
+
+
+
+class TestCopy:
+    def test_dumb_mocks_are_copied_correctly(self):
+        m = mock()
+        m.foo = [1]
+        n = copy(m)
+        assert m is not n
+        assert n.foo == [1]
+
+        m.foo.append(2)
+        assert n.foo == [1, 2]
+
+    @pytest.mark.xfail(reason=(
+        "not working for `copy` because __copy__ is accessed on the class, "
+        "not the instance"
+    ))
+    def test_strict_mocks_raise_on_unexpected_calls(self):
+        m = mock(strict=True)
+        with pytest.raises(RuntimeError) as exc:
+            copy(m)
+        assert str(exc.value) == (
+            "'Dummy' has no attribute '__copy__' configured"
+        )
diff --git a/tests/mocking_properties_test.py b/tests/mocking_properties_test.py
index 3411bba..1c991a2 100644
--- a/tests/mocking_properties_test.py
+++ b/tests/mocking_properties_test.py
@@ -1,5 +1,6 @@
 import pytest
-from mockito import mock, when
+from mockito import mock, verify, when
+from mockito.invocation import return_
 
 def test_deprecated_a(unstub):
     # Setting on `__class__` is confusing for users
@@ -40,10 +41,34 @@ def test_deprecated_c(unstub):
         m.tx
 
 
-def test_recommended_approach():
-    prop = mock(strict=True)
+def test_recommended_approach_1(unstub):
+    prop = mock()
     when(prop).__get__(Ellipsis).thenRaise(ValueError)
 
     m = mock({'tx': prop})
     with pytest.raises(ValueError):
         m.tx
+    verify(prop).__get__(...)
+
+
+def test_recommended_approach_2(unstub):
+    prop = mock()
+    when(prop).__get__(Ellipsis).thenReturn(42)
+
+    m = mock({'tx': prop})
+    assert m.tx == 42
+    verify(prop).__get__(...)
+
+
+def test_recommended_approach_3(unstub):
+    prop = mock({"__get__": return_(42)})
+    m = mock({'tx': prop})
+    assert m.tx == 42
+    verify(prop).__get__(...)
+
+
+def test_recommended_approach_4(unstub):
+    # Elegant but you can't `verify` the usage explicitly
+    # which makes it moot -- why not just set `m.tx = 42` then?
+    m = mock({'tx': property(return_(42))})
+    assert m.tx == 42