bpo-26467: Adds AsyncMock for asyncio Mock library support #9296

lisroach · 2018-09-14T07:17:40Z

This is my initial pass at supporting coroutine mocking via a new Mock subclass, CoroutineMock.

It can be used to mock out coroutines and have them validate as coroutines:

>>> mock = CoroutineMock()
>>> asyncio.iscoroutinefunction(mock)
True

Test that a coroutine was awaited:

class TestTests(unittest.TestCase):
    def test_assert_awaited(self):
        async def main():
            mock = CoroutineMock()
            with self.assertRaises(AssertionError):
                mock.assert_awaited()
            await mock()
            mock.assert_awaited()
        asyncio.run(main())

Also awaited_with, awaited_once_with, etc.

Things that I could use advice on:

Some of the code has been borrowed by asynctest (https://pypi.org/project/asynctest/) which is licensed under Apache 2. I have reached out to the author and am waiting to hear back, but am not sure how we feel about having Apache 2 licensed stuff in CPython. I could rewrite those spots if needed.
inspect.iscoroutine will return False for CoroutineMock objects. This is because the check is isinstance(object, types.CoroutineType), and CoroutineTypes are concrete. I've looked into using something like, register, but it doesn't work here, so I need someway to make a Mock look like a types.CoroutineType but I am unsure how.
In CoroutineMockAssert tests, I have asyncio.events._event_loop_policy unset because it causes the env to change when running tests if it is not unset. There is probably a better way to do this where the environment doesn't get polluted?
I plan on working on getting more example test cases for the CoroutineArguments test section, would be happy for advice though.

And I will be writing up the documentation once the code has settled.

https://bugs.python.org/issue26467

lisroach · 2018-09-14T17:08:52Z

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.

1st1 · 2018-09-14T17:40:31Z

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

+1.

cjw296 · 2019-05-02T14:06:50Z

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

lisroach · 2019-05-06T14:25:46Z

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

cjw296 · 2019-05-07T09:56:37Z

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.

…ences.

lisroach · 2019-05-07T15:20:07Z

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?

asvetlov · 2019-05-07T15:38:26Z

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

Lib/unittest/mock.py

cjw296 · 2019-05-07T17:38:28Z

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

1st1 · 2019-05-16T16:41:27Z

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

tirkarthi · 2019-05-16T17:11:21Z

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

tirkarthi · 2019-05-16T18:43:20Z

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

tirkarthi · 2019-05-17T04:38:29Z

Lib/unittest/test/testmock/testasync.py


        test_async()
        test_normal_method()

+    def test_create_autospec_instance(self):
+        with self.assertRaises(RuntimeError):
+            create_autospec(async_func, instance=True)


This test fails since the RuntimeError check is commented out.

Ah yeah, sorry I was trying to remember the answer to your question on why it was needed. It might not be and I'm debating about taking it out, I think I don't fully understand the purpose of instance=True and what we would want the behavior to be if it is True with create_autospec on a async function.

No problem, we can remove the restriction later if needed. I have added one more doctest suggestion that would make the CI green and this can be merged.

tirkarthi · 2019-05-17T04:44:53Z

Doc/library/unittest.mock.rst

+
+      >>> mock = AsyncMock()
+      >>> async def main(*args):
+      ...     await mock()


Suggested change

... await mock()

... await mock(*args)

tirkarthi · 2019-05-17T04:45:05Z

Doc/library/unittest.mock.rst

+
+      >>> mock = AsyncMock()
+      >>> async def main(*args):
+      ...     await mock()


Suggested change

... await mock()

... await mock(*args)

tirkarthi · 2019-05-18T02:08:24Z

Doc/library/unittest.mock.rst

+      >>> mock.await_args_list
+      []
+      >>> asyncio.run(main('foo'))
+      >>> mock.await_args


Suggested change

>>> mock.await_args

>>> mock.await_args_list

tirkarthi

LGTM. Buildbots are green and it will be great to have this merged for beta. I will create a followup PR that can be reviewed separately without holding this up any further.

Thank you very much @lisroach :)

lisroach · 2019-05-20T16:19:28Z

Thanks for all the help and reviews everyone!

mariocj89 · 2019-05-22T16:35:03Z

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:

Nothing at all, attribute error and let users set attributes if desired.
A normal mock as that might be more common (Patching an async callable class for example).
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.

mariocj89 · 2019-05-22T16:54:01Z

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.

asvetlov · 2019-05-22T18:01:10Z

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.

lisroach · 2019-05-24T00:14:54Z

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!

jeremycline · 2019-06-11T19:34:13Z

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?

lisroach · 2019-06-12T02:04:22Z

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.

njsmith · 2019-06-12T03:34:54Z

Maybe bool(MagicMock) should raise an error?

tirkarthi · 2019-06-12T04:40:32Z

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

jeremycline · 2019-06-12T14:02:20Z

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.

f0k · 2023-02-21T16:52:30Z

Lib/unittest/mock.py

+        _spec_asyncs = []
+
+        for attr in dir(spec):
+            if asyncio.iscoroutinefunction(getattr(spec, attr, None)):


Late note, not sure if anyone gets a notification for this: This broke a use case for me. Before this change, the Mock constructor did not call getattr on any of the attributes of the mocked object specification. I used it to mock an instance of a base class that does not have all its properties implemented, using the **kwargs in the Mock constructor to ensure that unimplemented properties that were accessed by any tests got a sane value. I can solve this by using a subclass that does not have unimplemented properties. But maybe it would be wise to skip all attributes here that appear in **kwargs, because they will be subsequently overwritten?

not sure if anyone gets a notification for this

For this reason it's worth creating a new issue and mentioning this PR there.

@f0k - better yet, if you could work up a PR with a test that shows the problem you encountered, along with a fix for it, that would be better!

@f0k some related issues . Please use search before creating the issue so that you can check for previous issues and make sure it's not duplicate. Thanks.

#89917
#85934

For this reason it's worth creating a new issue

I wasn't sure yet whether this warrants a fix, and was aiming low :)

some related issues

Good spot, yes, they're both the same problem, and there are already three PRs proposing a fix. I will continue the discussion in #85934, as it's the older one of the two issues.

lisroach added 11 commits September 12, 2018 08:59

Initial commmit adding asyncio mock support.

4353041

Adding async support to the mock library.

b83e5a5

Removes superfluous changes.

a9ea983

Cleans up comments.

50581e3

Fixes inspect and attribute error issues.

96ddb0e

Fixes test_unittest changing env because of version issue.

a4d4dbc

Removes newlines from inspect.

bfdd5a7

Removes unneeded comment and newlines.

ed7f13c

Fixes async tests. Removes inspect fix.

34fa74e

Fixes environment test issue.

302ef64

Adds argument tests.

bf749ac

the-knights-who-say-ni added the CLA signed label Sep 14, 2018

bedevere-bot added the awaiting review label Sep 14, 2018

Adding the side_effect exception test.

30b64b5

1st1 self-assigned this Sep 14, 2018

tirkarthi mentioned this pull request May 2, 2019

TypeError when patching a dictionary with a Unicode string literal on Python 2 testing-cabal/mock#458

Closed

Changes CoroutineMock to AsyncMock. Removes old-style coroutine refer…

5edac2a

…ences.

Merge branch 'master' into asyncio_mock

fa978cc

asvetlov changed the title ~~bpo-26467: Adds CoroutineMock for asyncio Mock library support~~ bpo-26467: Adds AsyncMock for asyncio Mock library support May 7, 2019

Changes fnmatch to list comp.

24920a6

tirkarthi reviewed May 7, 2019

View reviewed changes

Lib/unittest/mock.py Outdated Show resolved Hide resolved

cjw296 reviewed May 7, 2019

View reviewed changes

Lib/unittest/mock.py Show resolved Hide resolved

Fixes based on comments.

64301e2

tirkarthi reviewed May 17, 2019

View reviewed changes

Fixes broken docs.

c7cd95e

tirkarthi reviewed May 18, 2019

View reviewed changes

lisroach added 2 commits May 18, 2019 15:45

Fixes broken doc await_arg.

033f7d3

Adds shoutout to Martin Richard for asynctest.

2fef02c

tirkarthi approved these changes May 19, 2019

View reviewed changes

lisroach merged commit 77b3b77 into python:master May 20, 2019

bedevere-bot removed the awaiting merge label May 20, 2019

mariocj89 mentioned this pull request May 30, 2019

docs: Add references to AsyncMock in unittest.mock.patch #13681

Merged

dimaqq mentioned this pull request Jun 24, 2019

Question on upstream AsyncMock Martiusweb/asynctest#130

Open

This was referenced Nov 12, 2019

Integrating with Python 3.8 Martiusweb/asynctest#144

Open

Adding missing acknowledgement #17130

Merged

bpo-17013: Extend Mock.called to allow waiting for calls #17133

Closed

Kentzo mentioned this pull request Jun 9, 2020

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

Closed

f0k reviewed Feb 21, 2023

View reviewed changes

python deleted a comment from fok Feb 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-26467: Adds AsyncMock for asyncio Mock library support #9296

bpo-26467: Adds AsyncMock for asyncio Mock library support #9296

lisroach commented Sep 14, 2018 •

edited by bedevere-bot

Loading

lisroach commented Sep 14, 2018

1st1 commented Sep 14, 2018

cjw296 commented May 2, 2019

lisroach commented May 6, 2019

cjw296 commented May 7, 2019 •

edited

Loading

lisroach commented May 7, 2019

asvetlov commented May 7, 2019

cjw296 commented May 7, 2019

1st1 commented May 16, 2019

tirkarthi commented May 16, 2019

tirkarthi commented May 16, 2019

tirkarthi May 17, 2019

lisroach May 17, 2019

tirkarthi May 18, 2019

tirkarthi May 17, 2019

tirkarthi May 17, 2019

tirkarthi May 18, 2019

tirkarthi left a comment

lisroach commented May 20, 2019

mariocj89 commented May 22, 2019 •

edited

Loading

mariocj89 commented May 22, 2019 •

edited

Loading

asvetlov commented May 22, 2019

lisroach commented May 24, 2019 •

edited

Loading

jeremycline commented Jun 11, 2019

lisroach commented Jun 12, 2019

njsmith commented Jun 12, 2019

tirkarthi commented Jun 12, 2019 •

edited

Loading

jeremycline commented Jun 12, 2019

f0k Feb 21, 2023 •

edited

Loading

arhadthedev Feb 21, 2023

cjw296 Feb 21, 2023

tirkarthi Feb 21, 2023 •

edited by cjw296

Loading

f0k Feb 22, 2023

bpo-26467: Adds AsyncMock for asyncio Mock library support #9296

bpo-26467: Adds AsyncMock for asyncio Mock library support #9296

Conversation

lisroach commented Sep 14, 2018 • edited by bedevere-bot Loading

lisroach commented Sep 14, 2018

1st1 commented Sep 14, 2018

cjw296 commented May 2, 2019

lisroach commented May 6, 2019

cjw296 commented May 7, 2019 • edited Loading

lisroach commented May 7, 2019

asvetlov commented May 7, 2019

cjw296 commented May 7, 2019

1st1 commented May 16, 2019

tirkarthi commented May 16, 2019

tirkarthi commented May 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tirkarthi left a comment

Choose a reason for hiding this comment

lisroach commented May 20, 2019

mariocj89 commented May 22, 2019 • edited Loading

mariocj89 commented May 22, 2019 • edited Loading

asvetlov commented May 22, 2019

lisroach commented May 24, 2019 • edited Loading

jeremycline commented Jun 11, 2019

lisroach commented Jun 12, 2019

njsmith commented Jun 12, 2019

tirkarthi commented Jun 12, 2019 • edited Loading

jeremycline commented Jun 12, 2019

f0k Feb 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tirkarthi Feb 21, 2023 • edited by cjw296 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lisroach commented Sep 14, 2018 •

edited by bedevere-bot

Loading

cjw296 commented May 7, 2019 •

edited

Loading

mariocj89 commented May 22, 2019 •

edited

Loading

mariocj89 commented May 22, 2019 •

edited

Loading

lisroach commented May 24, 2019 •

edited

Loading

tirkarthi commented Jun 12, 2019 •

edited

Loading

f0k Feb 21, 2023 •

edited

Loading

tirkarthi Feb 21, 2023 •

edited by cjw296

Loading