Spy exceptions

[inderpreet99]

On spied methods, we are capturing return value. However when exceptions are raised, return_value comes back as False rather than the exception itself.

This PR change stores the raised exception into the return value.

[nicoddemus]

Hi @inderpreet99, thanks for the PR.

When we want a mock to raise an exception, we usually set side_effect. Wouldn't be better to use side_effect then? Also, returning an exception value is a valid use case.

[inderpreet99]

Hi @inderpreet99, thanks for the PR.

When we want a mock to raise an exception, we usually set side_effect. Wouldn't be better to use side_effect then? Also, returning an exception value is a valid use case.

Hi @nicoddemus,

This PR is not addressing mocked methods raising exceptions, I see that side_effect can help with that nicely. This PR is for spying on methods that raise exceptions. They may go unnoticed to our test because maybe upstream in that code the exception would be silenced (for example) or it may be gobbled up into another Exception. This PR helps make an assertion on one of those spied methods (deep in the code).

I agree returning an exception is a valid use case, which this PR will not be able to distinguish. So this PR is probably not ideal. I've seen pytest code record ExceptionInfos. Not sure how that stuff works.

Let me know if you have thoughts to solving the problem. I'm not tied to this approach.

[nicoddemus]

Sorry I wasn't clear, but perhaps showing what I meant in code is simpler:

        @w
        def wrapper(*args, **kwargs):
            try:
                r = method(*args, **kwargs)
            except Exception as e:
                result.side_effect = e
                raise
            else:
                result.return_value = r
            return r

[inderpreet99]

@nicoddemus, thanks. I completely misread what you were saying. Just updated the code.

I had another problem with not being able to see multiple invocations of a spied method. For example, I think the above code will only store the latest return value.

What do you think about having return_values and side_effects as a list that maybe line up exactly with mock_calls or call_args_list for example?

Thanks.

[nicoddemus]

I'm OK with us having both I guess, so keep the existing return_value and side_effect, but add return_value_list and side_effect_list variants? What do you think?

[nicoddemus]

Looks great, thanks!

Could you please also update the documentation showing the new feature?

[inderpreet99]

Could you please also update the documentation showing the new feature?

Added

I'm OK with us having both I guess, so keep the existing return_value and side_effect, but add return_value_list and side_effect_list variants? What do you think?

I'm facing a bug in regard to recording multiple side effects, so I'll get that code working in a separate branch. Especially since we are OK with keeping both features (last return/exception as well as all returns/exceptions).

[nicoddemus]

1.13 released! Thanks a lot @inderpreet99!