[issue35330] When using mock to wrap an existing object, side_effect requires return_value

[Mario Corchero]

Mario Corchero <mariocj89 at gmail.com> added the comment:

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 :)

----------

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue35330>
_______________________________________