When using mock to wrap an existing object, and using side_effect to set a function to wrap a method, I would expect the wrapper function to be called instead of the wrapped function, and its return value to be returned. Instead, both the wrapper function and the wrapped functions are being called, and the return value of the wrapped function is returned.

If, in addition to side_effect, return_value is set, the return_value is ignored, but my expected behavior actually happens: only the wrapper function is called, and its return value is returned.

Python 3.7.0 (default, Aug 22 2018, 20:50:05)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from unittest import mock
>>> class MyClass(object):
...     def func(self):
...         print('func called')
...         return 1
... 
>>> c = MyClass()
>>> m = mock.Mock(wraps=c)
>>> def func2():
...     print('func2 called')
...     return 2
... 
>>> m.func.side_effect = func2
>>> m.func()
func2 called
func called
1
>>> m.func.return_value = 3
>>> m.func()
func2 called
2

I did some debugging with docstring for wraps.


> `wraps`: Item for the mock object to wrap. If `wraps` is not None then
>  calling the Mock will pass the call through to the wrapped object
>  (returning the real result). Attribute access on the mock will return a
>  Mock object that wraps the corresponding attribute of the wrapped object
>  (so attempting to access an attribute that doesn't exist will raise an
>  `AttributeError`).
>  If the mock has an explicit return_value set then calls are not passed to the wrapped object and the return_value is returned instead.

So calling mock.Mock(wraps=c) sets the _mock_wraps with c. When we set m.func.side_effect and call m.func() it checks for the side_effect (func2) to be a callable and calls it [0]. It also checks if self._mock_wraps is not None which in this case is MyClass() and checks for the func of Myclass that is also called at [1] . As per the docstring since it wraps the actual object calling m.invalid_func without return_value set will cause attribute error like "AttributeError: 'MyClass' object has no attribute 'invalid_func'"

It seems to be a general case with mock itself where when side_effect and return_value are set then side_effect is called and ignores the return_value set unless the side_effect returns the sentinel value DEFAULT as in test [2]. I find this to be surprising.

$ ./python.exe
Python 3.8.0a0 (heads/master:b7278736b3, Nov 28 2018, 10:26:47)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from unittest import mock
>>> m = mock.Mock()
>>> m.side_effect = lambda : 2
>>> m.return_value = 3
>>> m() # side_effect and return_value are set returning side_effect
2
>>> f = mock.Mock()
>>> f.return_value = 3
>>> f() # side_effect is not set thus returns only return_value
3

As per the original report when there is a return_value set without the side_effect it returns the set return_value. When there is a return_value set with the side_effect then return value of the side effect is returned though return_value is explicitly set like above also with wraps

>>> m = mock.Mock(wraps=c)
>>> print(m.func())
func called
1
>>> m.func.return_value = 3
>>> print(m.func())
3
>>> f = mock.Mock(wraps=c)
>>> f.func.side_effect = func2
>>> f.func.return_value = 3
>>> print(f.func())
func2 called
2

[0] https://github.com/python/cpython/blob/bde9d6bbb46ca59bcee5d5060adaa33c3ffee3a6/Lib/unittest/mock.py#L1031
[1] https://github.com/python/cpython/blob/bde9d6bbb46ca59bcee5d5060adaa33c3ffee3a6/Lib/unittest/mock.py#L1041
[2] https://github.com/python/cpython/blob/54ba556c6c7d8fd5504dc142c2e773890c55a774/Lib/unittest/test/testmock/testmock.py#L156

A little more discussion over side_effect and return_value precedence : issue22541

I can indeed reproduce the issue. The problem seems to be here: https://github.com/python/cpython/blob/54ba556c6c7d8fd5504dc142c2e773890c55a774/Lib/unittest/mock.py#L1041

The simplified current logic in that code is:
- call side_effect, save the return value in ret.
- if return_value is not set, call the wraps object and return it.
- return the value in ret from the first step.

That explains why you see your "expected behavior" to happen only when return value is set.

Basically, the logic disables the current wrapped object ONLY if return value is set.
I am not sure why it was not done for `side_effect` as well.
Trying to perform that change in the sourcecode (nor run wraps if side_effect is set) results in no failure from the tests, which might mean it is a bug 

One might claim the code is there because `side_effect` might still be used to cause a side effect but not return, I would disagree, especially as setting `return_value` totally breaks it. As once the `return_value` is set, both `return_value` and `wraps` are ignored and `side_effect` takes preference due to line 1044.
Especially as the docs say about side_effect: 
`unless it returns DEFAULT, the return value of this function is used as the return value.`

I'd suggest a patch adding the `and not effect` to line 1041, so `side_effect` takes preference over `wraps`, the same way `return_value` does today.

The behavior of `side_effect` taken precedence over `return_value` is fine, that is how Mock works.



This bug can be reproduced without a class at all, see:

```
from unittest import mock
def func():
    print('Original func called')
    return "ORIGINAL"

m = mock.Mock(wraps=func)

def side_effect():
    print('Side effect func called')
    return "SIDE EFFECT"


m.side_effect = side_effect
print(m())
```

Results in:
```
Side effect func called
Original func called
ORIGINAL
```

Whilst the expected is:
```
Side effect func called
SIDE EFFECT
```



## TL;DR;

Indeed, `side_effect` seems broken when applied to an object with wraps and setting `return_value` kind of "fixes it".
I'd send a fix to add the check for `effect` unless michael.foord or someone else knows a good reason of why things are not like that. + tests

Noam Yorav-Raphael I am happy to send the patch if you don't have time :)

Thanks @mariocj89 for the explanation. I just got to the docs part about side_effect and return_value precedence. I am curious to know about the behavior as well and at least this can be added as a test as I see only around 3 tests for side_effect and there is no test for wraps behavior in this report I assume given that there were no test failures due to changing the logic.

I'll get ready a PR with a good set of tests and the fix for the original issue. This is quite an interesting bug :)

New changeset f05df0a4b679d0acfd0b1fe6187ba2d553b37afa by Chris Withers (Mario Corchero) in branch 'master':
bpo-35330:  Don't call the wrapped object if `side_effect` is set (GH10973)
https://github.com/python/cpython/commit/f05df0a4b679d0acfd0b1fe6187ba2d553b37afa

New changeset 12b9fb603eea9298c835bae5b8742db4fa52892e by Chris Withers (Miss Islington (bot)) in branch '3.6':
bpo-35330:  Don't call the wrapped object if `side_effect` is set (GH11034)
https://github.com/python/cpython/commit/12b9fb603eea9298c835bae5b8742db4fa52892e

New changeset ee2c5a8e2dcf662048dbcf4e49af9b4aaf81f7d3 by Chris Withers (Miss Islington (bot)) in branch '3.7':
bpo-35330:  Don't call the wrapped object if `side_effect` is set (GH11035)
https://github.com/python/cpython/commit/ee2c5a8e2dcf662048dbcf4e49af9b4aaf81f7d3