GeezerVison

CC @Martiusweb

Anyone interested in the review?

Rebased on top of the current main.

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

Up

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

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?

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.

Could you resolve a merge conflict, please?

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

@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)).

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

@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?