New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support both side_effect and return_value in a more human way #66731
Comments
Original use case: I want to mock something that on invocation does 2 things:
The problem:
Moreover, side_effect takes precedence over return_value, unless the earlier returns mock.DEFAULT. Very quirky. Proposed change option 1: Proposed change option 2: Current workaround 1: Current workaround 2: |
Can you explain why you can't use side_effect to set the return value? |
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) # -"- |
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. |
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. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: