gh-61215: Add Mock.call_event to allow waiting for calls

[Kentzo]

This implementation of bpo-17013 is alternative to #16094

Changes are based on my work for asynctest. Specifically on _AwaitEvent that was left out when related code was ported to CPython.

Key features:

• Does not require end users to do anything: change is automatically available in every Mock
• Utilizes existing semantics of python conditionals (both asyncio and threading)

Accepting this change will allow me to port _AwaitEvent therefore giving identical semantics to both wait-for-calls and wait-for-awaits.

I will provide necessary typing annotations for typeshed.

Considerations:

1. This approach changes type of the Mock.called property from bool to private bool-like _CallEvent. However, the only practical problem I could think of is if someone was checking type of Mock.called (e.g. isinstance, is). That does not sound like a plausible problem: after all the property itself is seldom used with exception of conditional expressions where _CallEvent.__bool__ and _CallEvent.__eq__ is sufficient. Added new property instead.
2. It probably makes sense to provide convenience methods like wait_for_call, but I would like to hear the opinion of the reviewers. Implemented.

CC: @vstinner @tirkarthi @mariocj89

https://bugs.python.org/issue17013

• Issue: Allow waiting on a mock #61215

[Kentzo]

CC @Martiusweb

[Kentzo]

Anyone interested in the review?

[Kentzo]

Rebased on top of the current main.

Failure of the address sanitizer tests is unrelated to the changes in this PR.

[Kentzo]

Up

[cjw296]

@mariocj89 / @tirkarthi - I think this makes sense, should we land it if @Kentzo still has energy to rebase it?

[mariocj89]

IIRC Michael Foord preferred to have a custom Mock class targeted to multi threading. Similarly to how AsyncMock was created. We put together #16094 for that purpose. I am fine with either approach, but would be nice to have something. If he does not have the energy I can rebase rhe other PR as well.

…

On Fri, 9 Jun 2023 at 15:44, Chris Withers ***@***.***> wrote: @mariocj89 </~https://github.com/mariocj89> / @tirkarthi </~https://github.com/tirkarthi> - I think this makes sense, should we land it if @Kentzo </~https://github.com/Kentzo> still has energy to rebase it? — Reply to this email directly, view it on GitHub <#20759 (comment)>, or unsubscribe </~https://github.com/notifications/unsubscribe-auth/AA4JV45NCZ3MWLJQZVQWZW3XKMLDLANCNFSM4NZKPGKQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[cjw296]

Ah, yeah, tough call... I guess for consistency of API, we probably should go with #16094.

Can you ever need both AsyncMock and ThreadingMock in the same object?

[mariocj89]

Not an asyncio expert, but that would be weird IIUC, as you will probably not want to mix those two paradigms. For sure you will never need them in the same method.

…

On Fri, 9 Jun 2023 at 16:31, Chris Withers ***@***.***> wrote: Ah, yeah, tough call... I guess for consistency of API, we probably should go with #16094 <#16094>. Can you ever need both AsyncMock and ThreadingMock in the same object? — Reply to this email directly, view it on GitHub <#20759 (comment)>, or unsubscribe </~https://github.com/notifications/unsubscribe-auth/AA4JV43LREVYMBSGXLPNQRLXKMQS5ANCNFSM4NZKPGKQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Kentzo]

AsyncMock was a forced decision in asynctest because it was a 3rd party package. And probably a necessary one in stdlib, because being an awaitable is part of the function signature.

Possibility of being used in a multithreading context is an intrinsic property of all Callables. Such differentiation would be akin to adding CallableMock, AttributeMock and so on.

Can you ever need both AsyncMock and ThreadingMock in the same object?

Both mock function calls, so it's feasible for a user to want to know whether an awaitable was instantiated (but not awaited) from some other thread, such as a run loop running in another thread.

[arhadthedev]

Could you resolve a merge conflict, please?

[cjw296]

@Kentzo / @mariocj89 - would it be crazy to land both these PRs?

[Kentzo]

@arhadthedev Will do.

@cjw296

• If you land this one, the separate class won't be needed
• If you land the other PR then CallEvent could be salvaged and then I can port AwaitEvent for AsyncMock. However AsyncMock users won't be able to wait for calls.

What are your thoughts on this, perhaps I could word better and clarify the use case?

My tl;dr; is that users should be able to wait for calls of functions and for both calls and awaits of coroutines. This is why I advocate for having call_event in MagicMock (base class) rather than a separate class:

my_object = Mock(...)

my_object.some_function.call_event.wait(...)
# post-call asserts

my_object.some_coroutine.call_event.wait(...)
# post-call but pre-await asserts
my_object.some_coroutine.await_event.wait(...)
# post-await asserts

---

If you decide to proceed with this PR, one feedback I'd like to get is whether CallEvent/AwaitEvent should have separate methods for waiting (wait(...), wait_for_call(call, ...), wait_for(predicate, ...)) or just one by merging arguments (wait(self, call=None, /, predicate=None, skip=0, timeout=None)).

[mariocj89]

AsyncMock was a forced decision in asynctest because it was a 3rd party package. And probably a necessary one in stdlib, because being an awaitable is part of the function signature.

I was not meaning that the decision was based on "wanting more mock classes", but that in a conversation in the core-dev sprints in 2019 (IIRC), @mfoord said he preferred having more specialized mocks depending on the usecase.

However AsyncMock users won't be able to wait for calls.

I'd say we should add an asyncio specific waiting function. To wait for something to be awaited. I think this is even more of a reason to not have it in MagicMock TBH. What does it mean to "wait" for a coroutine to be called? The implementation here is thread based, again, not an expert on asyncio, but I'm not sure this makes much sense (saying this works for asyncio). If you need a "await_event" as you posted on the last comment I'd rather move that to AsyncMock.

Having the separation will give you "Mock/MagicMock" as general purpose. "ThreadingMock" for multithreading and "AsyncMock" for asyncio/corrutines.
And note that they can be easily combined. You can do something like:

my_obj = MagicMock()
my_obj.corrutine_func = AsyncMock()
my_obj.threading_call = ThreadingMock()

A user can use both mocks. The only situation where this does not work is when you have a corrutine that you want "to wait" for it to be called. but AFAIK that does not make much sense. The implementation for that is probably different isnt it? Which was the main driver of AsyncMock.

@Kentzo / @mariocj89 - would it be crazy to land both these PRs?

I'd land only one (and I'm fine if it is not the one that @tirkarthi and I put together), but I think we need 2 different implementations. One for multi-threading and another for asyncio. This implements multithreading and exposes it in the base class, adding a call_event attribute (hopefully there are not many clashes) to all mocks.

Morevoer, I subjectively find the AP of threadingmock closer to the existing ones:

my_object = Mock(...)
my_object.some_function.call_event.wait(...)

my_object = ThreadingMock(...)
my_object.some_function.wait_until_any_call(...)

Lastly, having a ThreadingMock would allow us to extend the class for other multithreading use-cases without fear of colliding with user defined functions (new class guarantees no backward compatibility issues).

[cjw296]

@mariocj89 : Yeah, I think I'm persuaded by the 'separate class' argument, and I like the fact that you can compose in the way you show, so let's go for landing the version in #16094.

FWIW, I now remember that I ended up implementing something similar for Twisted when I needed it. Worse still as I needed to capture calls to methods on existing classes where Mocks cannot be subbed in, and where the actual instance cannot be referenced by test code. One thing I do remember that might be useful here is that I ended up needing a list of calls and a list of things waiting which then had to be matched up. I guess AsyncMock must have this already? Threading may not need this?