GeezerVison

Talking with @asvetlov, it might be easier/more useful to implement this purely as an AwaitableMock, instead of mocking Coroutines themselves. This should also be more flexible in terms of inspect, because I could assert isawaitable() more easily than iscoroutine().

We also chatted about ideas around mocking AsyncIterators so we can continue to support aiter and anext, but that will probably come in a separate commit.

Talking with @asvetlov, it might be easier/more useful to implement this purely as an AwaitableMock, instead of mocking Coroutines themselves.

+1.

@lisroach - I'm afraid I don't any anywhere near enough experience with asyncio to sensibly review this, however one request: please can you split any tests that use the 3.7+ async syntax into their own file (test..._async.py or something) so that this can more easily be merged into the rolling backport.

I'm not sure if this will be possible, but if you could avoid using any 3.7+ syntax inside mock.py itself, that would, again, make backporting significantly easier :-)

@cjw296 Good point about the backporting. So separating the tests is no problem, easy-peasy. For the internals of mock.py itself it's a bit more challenging because of the async def __anext__ I need.

What if I try to wrap all the AsyncMock related code in mock.py in some regexable comment block (like # Nonbackportable AsyncMock), and all code within those blocks could be searched for and removed before backporting?

What might work better is if you could put the AsyncMock class and anything else that needs 3.7+ syntax into a mockasync.py next to mock.py and then do a from .mockasync import * inside mock.py (for circular import fun reasons, this may need to be at the bottom of mock.py). That way, the code can stay in one place and we can make that a conditional import in the backport.

I don't think it's possible to separate it out into a different file, the code is too integrated with the mock code itself we can't avoid the circular dependencies. I've moved all of the 3.7+ syntax code to the bottom of mock.py so it's all in one spot, will it be possible for it to be dropped when backporting with it like this?

@cjw296 the PR not only adds AsyncMock class but hacks already existing mock classes.
There is nothing new, mock is already has circular dependencies.

Please elaborate how from .mockasync import * at the end for mock.py can make backporting easier?
Do you want to remove this line and all usages for mockasync objects from mock.py during a backport?

@asvetlov - yeah, I've commented on the "hacks already existing mock classes". This doesn't feel like a good idea to me, do we really need it?

The elaboration requested, would be this in cpython:

mock.py:
...all existing mock stuff...
from .mockasync import *

mockasync.py:
from .mock import *  # should work as we've already got the unittest.mock setup above
... anything using 3.7+ syntax...

This would be backported as:

mock.py:
...all existing mock stuff...
if sys.version_info >= (3, 7, ):
    from .mockasync import *

...so the patch can stay pretty much identical bar adding one line and changing another. It does require all Python 3.7+-only syntax going into mockasync.py.

@tirkarthi Most of your comments are on point, but I think we can address them once this is merged.

@1st1 I would request create_autospec to be fixed as it's one line change causing NameError before beta release and if possible as part of this PR. Fixing the ResourceWarning would also be great just to make sure there is no buildbot failure outside of primary CI since I don't know the configuration in which they are run.

More tests and docs could be added after beta and I would be happy to help with follow up PRs for the same. Thanks for approving this.

@lisroach Attached is a patch fixing docs and doctest that would make CI green. I tested it on my fork and seems to work fine.

diff --git a/Doc/library/unittest.mock.rst b/Doc/library/unittest.mock.rst
index a8d05b216c..5d4737b60f 100644
--- a/Doc/library/unittest.mock.rst
+++ b/Doc/library/unittest.mock.rst
@@ -201,9 +201,11 @@ The Mock Class

 .. testsetup::

+    import asyncio
+    import inspect
     import unittest
     from unittest.mock import sentinel, DEFAULT, ANY
-    from unittest.mock import patch, call, Mock, MagicMock, PropertyMock
+    from unittest.mock import patch, call, Mock, MagicMock, PropertyMock, AsyncMock
     from unittest.mock import mock_open

 :class:`Mock` is a flexible mock object intended to replace the use of stubs and
@@ -885,9 +887,9 @@ object::
     ...
     >>> mock = MagicMock(async_func)
     >>> mock
-    <MagicMock spec='function' id='4568403696'>
+    <MagicMock spec='function' id='...'>
     >>> mock()
-    <coroutine object AsyncMockMixin._mock_call at 0x1104cb440>
+    <coroutine object AsyncMockMixin._mock_call at ...>

   .. method:: assert_awaited()

@@ -976,11 +978,11 @@ object::
       Assert the mock has been awaited with the specified calls.
       The :attr:`await_args_list` list is checked for the awaits.

-      If `any_order` is False (the default) then the awaits must be
+      If *any_order* is False (the default) then the awaits must be
       sequential. There can be extra calls before or after the
       specified awaits.

-      If `any_order` is True then the awaits can be in any order, but
+      If *any_order* is True then the awaits can be in any order, but
       they must all appear in :attr:`await_args_list`.

         >>> mock = AsyncMock()

Thanks for all the help and reviews everyone!

I know I am late to the party. Real apologies about it, I had LASIK past week and could not check this on the weekend. I don't know how many times people have asked about AsyncMock, this is just great :).

My main comment here would be whether it is useful or confusing the fact that AsyncMock will generate "inner mocks" when attributes are called on it. I would find that confusing though I am far from an expert on async. Is there any use-case were calling an attribute in an async object it yields naturally another async object?

Example, not sure this is expected:

>>> from unittest.mock import AsyncMock
>>> m = AsyncMock()
>>> m.a.return_value = 1
>>> print(m.a)
<AsyncMock name='mock.a' id='140389785043824'>
>>> print(m.a())
<coroutine object AsyncMockMixin._mock_call at 0x7faf0ab8d240>
<stdin>:1: RuntimeWarning: coroutine 'AsyncMockMixin._mock_call' was never awaited
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

I think I'd use the AsyncMock to mock specific function calls. It is also really handy that it works with patch. I was just wondering whether getting an attribute of the AsyncMock should return:

1. Nothing at all, attribute error and let users set attributes if desired.
2. A normal mock as that might be more common (Patching an async callable class for example).
3. Default to async mock as the current implementation does. People can set normal mocks if wanted.

In short, if we have a class that has async and normal methods, Should we use a normal mock and add AsyncMock to the methods that are async or the other way around?

If this was thought and 3 was chosen purposely because it makes more sense for async users, all good (as said, not an expert in async at all), just raising the question.

Example, how do we expect users to simulate the following class?

class X:
  async def a(self):
    return "a"
  def b(self):
    return "b"

If the plan is to use AsyncMock for it, the following does not work:

m = AsyncMock(**{"a.return_value": "mocka", "b.return_value": "mockb"})
async def check(m):
    print(m.b())
    print(await m.a())

Output:

>>> asyncio.run(check(m))
<coroutine object AsyncMockMixin._mock_call at 0x7f39dea8cac0>
<stdin>:2: RuntimeWarning: coroutine 'AsyncMockMixin._mock_call' was never awaited
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
mocka

If we say that users should specify the exact type of mock on each of the attribute, what is the purpose of automatically generating them for the attribute for AsyncMock then?

As said, minor concern, there might be no better solution, but wanted to have a look at it.

Thanks for the comment.
I see a bunch of RuntimeWarning: coroutine 'AsyncMockMixin._mock_call' was never awaited when running tests.test_asyncio suite.
Need to investigate.
See https://bugs.python.org/issue37015 for the reference.

Thanks for pointing that out @mariocj89! I made it recursive asyncmocks more as a way to copy the normal functionality of Mock, which creates an internal mock of the same kind when calling an attribute, for example:

>>> from unittest.mock import MagicMock
>>> m = MagicMock()
>>> m.a.return_value = 1
>>> print(m.a)
<MagicMock name='mock.a' id='4490221040'>

It is mentioned in the docstring "By default child mocks will be the same type as the parent", although this can be overridden.
I wanted to mimic the same functionality because I do think that is what is expected when using the mock library.

The current functionality is that if you want to mock a mixed class, you use a normal mock and it figures out for you which functions are async:

>>> class X:
...   async def a(self):
...     return "a"
...   def b(self):
...     return "b"
...
>>> m = MagicMock(X, autospec=True)
>>> m.b
<MagicMock name='mock.b' id='4490513224'>
>>> m.a
<AsyncMock name='mock.a' id='4490514088'>

But you are right if someone was attempting to build their own class via mock the would need to specify the mock type of the arguments. I don't usually use mock this way so I hadn't considered that use case. In your example it looks like there would be no way to tell it is supposed to be an awaitable mock until the actual call occurs, which might be tricky to implement. If you think it will be useful though I don't mind if you open up a new bug and we can figure it out there!

Hi folks,

I've hit an issue I think is related to this PR in Python 3.8b1. The minimal reproducer I came up with is:

from unittest import mock

mock_obj = mock.MagicMock()
mock_obj.mock_func = mock.MagicMock(spec=lambda x: x)

with mock.patch.object(mock_obj, "mock_func") as nested:
    print(type(nested))

What happens is that nested is an AsyncMock in Python 3.8, but a MagicMock in Python 3.7. The reason is when the MagicMock has a function spec, __code__ is defined so the check for the coroutine flags returns a truthy MagicMock object. In other words, import inspect; from unittest import mock; inspect.iscoroutinefunction(mock.MagicMock(spec=lambda x: x)) is True (and that holds in Python 3.7 as well).

I thought about just submitting a fix, but it's not clear to me whether adding a check to unittest.mock._is_async_obj for MagicMock and behaving differently is truly the right fix. Having asyncio.iscoroutinefunction return True seems wrong for MagicMocks with function specs. Maybe MagicMock needs to have something to make it behave correctly there?

Hey @jeremycline, would you mind making a bug on bugs.python.org? It will be best to centralize discussion there. Thanks for reporting this! It is definitely a bug.

I understand the desire to change MagicMock to avoid this behavior, but I think it is the wrong approach. It is not evaluating to True because of the __code__.co_flag attribute, that doesn't get set until after the check for _is_async_obj has already evaluated truthy.

The issue comes in when checking:

bool(f.__code__.co_flags & flag)

in inspect._has_code_flag. The & is a call, which with MagicMocks will result in another MagicMock object:

<MagicMock name='mock.mock_func.__code__.co_flags.__and__()' id='4467594096'>

It does not take into account the flag argument at all, and calling bool() on the MagicMock evaluates to True.

The only way I see us getting around this would be to set the mock.__code__.co_flags attribute everytime a spec with __code__ is passed in to MagicMock, but this changes the way mock typically behaves. It would be unexpected for one attribute to be set (I think) if others are not.

It might be best to make a workaround in the _is_async_obj, although it feels more hacky.

Maybe bool(MagicMock) should raise an error?

It might be best to make a workaround in the _is_async_obj, although it feels more hacky.

I feel this is a reasonable trade off that maybe we could check if __code__ is actually a CodeType to ensure MagicMock is not passed to iscoroutinefunction to give a false positive.

def _is_async_obj(obj):
    code = getattr(obj, '__code__', None)
    if code and isinstance(code, CodeType):
        return asyncio.iscoroutinefunction(obj) or inspect.isawaitable(obj)
    else:
        return False

Hey @jeremycline, would you mind making a bug on bugs.python.org? It will be best to centralize discussion there. Thanks for reporting this! It is definitely a bug.

Thanks for the quick response, I've filed an issue at https://bugs.python.org/issue37251.