classification
Title: Support both side_effect and return_value in a more human way
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
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],
          return_value=42)

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.
History
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