Title: Support both side_effect and return_value in a more human way
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.5
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: Dima.Tisnek, michael.foord, r.david.murray
Priority: normal Keywords:

Created on 2014-10-02 13:29 by Dima.Tisnek, last changed 2014-10-03 03:48 by michael.foord. This issue is now closed.

Messages (5)
msg228224 - (view) Author: Dima Tisnek (Dima.Tisnek) * Date: 2014-10-02 13:29
Original use case:

I want to mock something that on invocation does 2 things:
* updates database
* returns custom, unrelated value

The problem:
side_effect is a catch-all setting that is used for:
* true side-effects
* return values
* exceptions

Moreover, side_effect takes precedence over return_value, unless the earlier returns mock.DEFAULT. Very quirky.

Proposed change option 1:
Side-effect is not used as return value; explicit return value may be used for that purpose.
(probably changes a lot of tests)

Proposed change option 2:
return_value, being more specific and explicit, takes precedence over side_effect; that is side_effect is still executed but it's rv is lost if overwritten by return_value is set.
(this is unlikely to change existing tests, as it is currently very odd to use both side_effect and return_value at the same time.)

Current workaround 1:
mock.Mock(side_effect=lambda *args: [db.update(), mock.DEFAULT][-1],

Current workaround 2:
mock.Mock(side_effect=lambda *args: [db.update(), 42][-1])
msg228225 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-02 13:40
Can you explain why you can't use side_effect to set the return value?
msg228226 - (view) Author: Dima Tisnek (Dima.Tisnek) * Date: 2014-10-02 13:51
I can, as shown in workaround #2.

However it's hackey when true side-effect's own return value has nothing in common with return value for the mock.

The shortest workaround I managed is this:

mock.Mock(side_effect=lambda *arg: db.update(...) or my_return_value)  # relies on db.update returning None

or more explicit like this:

mock.Mock(side_effect=lamda *arg: db.update(...) or mock.DEFAULT, return_value=my_return_value)  # -"-
msg228227 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-02 14:03
Oh, sorry, I missed the workarounds when I read the issue.  So your issue is you want to use a lambda rather than a full function for the side effect and it looks ugly.

I agree that given the names 'side_effect' and 'return_value' you'd think return_value would take precedence.  On the other hand, the API is what it is, and I'm not sure we can change it for backward compatibility reasons.  Option 1 is definitely not on the table, and the problem with option 2 is that it is easy to imagine a side_effect that conditionally returns mock.DEFAULT.
msg228303 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2014-10-03 03:48
One of the use cases for side_effect is for dynamically changing the returned value based on input - so your option 1 just doesn't work.

Using two line functions would make your code easier to read, and then your "workarounds" would look natural instead of awkward.

Maybe return_value should take precedence over side_effect if both are set.

It would probably be more pythonic to raise an exception in that case - although that would be annoying if you're just reconfiguring a mock. I've also written side_effect functions that accessed the return_value, which is probably the genesis of the current API.

As you point out, both your suggested changes are backwards incompatible, so I'm closing this issue.
Date User Action Args
2014-10-03 03:48:28michael.foordsetstatus: open -> closed
resolution: wont fix
messages: + msg228303

stage: resolved
2014-10-02 14:03:54r.david.murraysetnosy: + michael.foord

messages: + msg228227
versions: - Python 3.1, Python 2.7, Python 3.2, Python 3.3, Python 3.4
2014-10-02 13:51:43Dima.Tisneksetmessages: + msg228226
2014-10-02 13:40:27r.david.murraysetnosy: + r.david.murray
messages: + msg228225
2014-10-02 13:29:56Dima.Tisnekcreate