Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-38093: Correctly returns AsyncMock for async subclasses. #15947

Merged
merged 15 commits into from
Sep 20, 2019
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions Doc/library/unittest.mock-examples.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import asyncio
import unittest
from unittest.mock import Mock, MagicMock, patch, call, sentinel
from unittest.mock import AsyncMock, Mock, MagicMock, patch, call, sentinel

class SomeClass:
attribute = 'this is a doctest'
Expand Down Expand Up @@ -280,36 +280,38 @@ function returns is what the call returns:
Mocking asynchronous iterators
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Since Python 3.8, ``MagicMock`` has support to mock :ref:`async-iterators`
through ``__aiter__``. The :attr:`~Mock.return_value` attribute of ``__aiter__``
can be used to set the return values to be used for iteration.
Since Python 3.8, ``AsyncMock`` and ``MagicMock`` have support to mock
:ref:`async-iterators` through ``__aiter__``. The :attr:`~Mock.return_value`
attribute of ``__aiter__`` can be used to set the return values to be used for
iteration.

>>> mock = MagicMock()
>>> mock = MagicMock() # AsyncMock also works here
>>> mock.__aiter__.return_value = [1, 2, 3]
>>> async def main():
... return [i async for i in mock]
...
>>> asyncio.run(main())
[1, 2, 3]


Mocking asynchronous context manager
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Since Python 3.8, ``MagicMock`` has support to mock
:ref:`async-context-managers` through ``__aenter__`` and ``__aexit__``. The
return value of ``__aenter__`` is an :class:`AsyncMock`.
Since Python 3.8, ``AsyncMock`` and ``MagicMock`` have support to mock
:ref:`async-context-managers` through ``__aenter__`` and ``__aexit__``.
By default, ``__aenter__`` is an ``AsyncMock`` that returns an async function.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, __aexit__ is an AsyncMock too, isn't it?

Suggested change
By default, ``__aenter__`` is an ``AsyncMock`` that returns an async function.
By default, ``__aenter__`` and ``__aexit__`` are ``AsyncMock`` instances that return an async function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I will add that 😆


>>> class AsyncContextManager:
...
... async def __aenter__(self):
... return self
...
... async def __aexit__(self):
... async def __aexit__(self, exc_type, exc, tb):
... pass
>>> mock_instance = MagicMock(AsyncContextManager())
...
>>> mock_instance = MagicMock(AsyncContextManager()) # AsyncMock also works here
>>> async def main():
... async with mock_instance as result:
... pass
...
>>> asyncio.run(main())
>>> mock_instance.__aenter__.assert_called_once()
>>> mock_instance.__aexit__.assert_called_once()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually we probably want to replace these either with assert_aiwated_once() or by returning a result and checking for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I will update them in this PR if that's okay.

Expand Down
18 changes: 12 additions & 6 deletions Lib/unittest/mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -988,9 +988,13 @@ def _get_child_mock(self, /, **kw):
_type = type(self)
if issubclass(_type, MagicMock) and _new_name in _async_method_magics:
klass = AsyncMock
if issubclass(_type, AsyncMockMixin):
elif _new_name in _sync_async_magics:
# Special case these ones b/c users will assume they are async,
# but they are actually sync
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example of such method in the comment would probably help. Is this about e.g. __aiter__ that sounds async but is actually sync?
Edit: now I saw where _sync_async_magics is defined that __aiter__ is the only one.

klass = MagicMock
if not issubclass(_type, CallableMixin):
elif issubclass(_type, AsyncMockMixin):
klass = AsyncMock
elif not issubclass(_type, CallableMixin):
if issubclass(_type, NonCallableMagicMock):
klass = MagicMock
elif issubclass(_type, NonCallableMock) :
Expand Down Expand Up @@ -1867,7 +1871,7 @@ def _patch_stopall():
'__reduce__', '__reduce_ex__', '__getinitargs__', '__getnewargs__',
'__getstate__', '__setstate__', '__getformat__', '__setformat__',
'__repr__', '__dir__', '__subclasses__', '__format__',
'__getnewargs_ex__', '__aenter__', '__aexit__', '__anext__', '__aiter__',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a test for the default values of these functions? There is a behavior change in 3.8b4 and now where __aexit__ for MagicMock returns False and now it returns a coroutine. The docs can also be updated in the "mock and default values" section since these don't have default values : https://docs.python.org/3.8/library/unittest.mock.html?highlight=asyncmock#unittest.mock.NonCallableMagicMock . I guess this also makes updating _return_values dictionary since the default values are not needed now for these and can be removed.

➜  cpython git:(pr_15947) ✗ python3.8
Python 3.8.0b4 (v3.8.0b4:d93605de72, Aug 29 2019, 21:47:47)
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from unittest.mock import MagicMock
>>> m = MagicMock()
>>> m.__aexit__()
False
➜  cpython git:(pr_15947) ✗ ./python.exe
Python 3.9.0a0 (heads/pr_15947:f476b5845a, Sep 12 2019, 10:52:24)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from unittest.mock import MagicMock
>>> m = MagicMock()
>>> m.__aexit__()
<coroutine object AsyncMockMixin._mock_call at 0x107005d40>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm this is a good point about __aexit__, I think it should have a default return_value of False (which is in the _return_values dict, but maybe needs to be added to the _calculate_return_value instead.

This section specifically is the "non-default" values, so I don't think it make sense to update that section of the docs, and there is a unit test for the default values that are set in test_magicmock_defaults. I will update __aexit__ and add it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah well, this is actually another case of "called" vs "awaited". Once you await __aexit__ it returns False correctly, just calling prints the type of __aexit__ which is a coroutine object.

So I want to leave the __aexit__ in _return_value and I will update the test. Do you think that would resolve this?

Copy link
Member

@tirkarthi tirkarthi Sep 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay so it's a case where once __aexit__ is awaited then it returns the calculated value of False. Sorry, I am not sure of async internals to see if __aexit__ can be used like await __aexit__() or makes sense but I can understand in terms of the PR context with mock that the coroutine needs to be always awaited.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

await cm.__aexit__() is a legal call.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, this is how async cms work: https://www.python.org/dev/peps/pep-0492/#new-syntax
Both __aenter__ and __aexit__ are awaited.

'__getnewargs_ex__',
}


Expand All @@ -1886,10 +1890,12 @@ def method(self, /, *args, **kw):

# Magic methods used for async `with` statements
_async_method_magics = {"__aenter__", "__aexit__", "__anext__"}
# `__aiter__` is a plain function but used with async calls
_async_magics = _async_method_magics | {"__aiter__"}
# Magic methods that are only used with async calls but are synchronous functions themselves
_sync_async_magics = {"__aiter__"}
_async_magics = _async_method_magics | _sync_async_magics

_all_magics = _magics | _non_defaults
_all_sync_magics = _magics | _non_defaults
_all_magics = _all_sync_magics | _async_magics

_unsupported_magics = {
'__getattr__', '__setattr__',
Expand Down
161 changes: 113 additions & 48 deletions Lib/unittest/test/testmock/testasync.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,35 +378,85 @@ def test_add_side_effect_iterable(self):
class AsyncContextManagerTest(unittest.TestCase):

class WithAsyncContextManager:

async def __aenter__(self, *args, **kwargs):
return self

async def __aexit__(self, *args, **kwargs):
pass

def test_magic_methods_are_async_mocks(self):
mock = MagicMock(self.WithAsyncContextManager())
self.assertIsInstance(mock.__aenter__, AsyncMock)
self.assertIsInstance(mock.__aexit__, AsyncMock)
class WithSyncContextManager:
def __enter__(self, *args, **kwargs):
return self

def __exit__(self, *args, **kwargs):
pass

class ProductionCode:
# Example real-world(ish) code
def __init__(self):
self.session = None

async def main(self):
async with self.session.post('https://python.org') as response:
lisroach marked this conversation as resolved.
Show resolved Hide resolved
val = await response.json()
return val

def test_async_magic_methods_are_async_mocks_with_magicmock(self):
cm_mock = MagicMock(self.WithAsyncContextManager)
self.assertIsInstance(cm_mock.__aenter__, AsyncMock)
self.assertIsInstance(cm_mock.__aexit__, AsyncMock)

def test_magicmock_has_async_magic_methods(self):
cm = MagicMock(name='magic_cm')
self.assertTrue(hasattr(cm, "__aenter__"))
self.assertTrue(hasattr(cm, "__aexit__"))

def test_magic_methods_are_async_functions(self):
cm = MagicMock(name='magic_cm')
self.assertTrue(asyncio.iscoroutinefunction(cm.__aenter__))
self.assertTrue(asyncio.iscoroutinefunction(cm.__aexit__))
lisroach marked this conversation as resolved.
Show resolved Hide resolved
# These should pass but cause warnings to be raised
# self.assertTrue(inspect.isawaitable(cm.__aenter__()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just await on the method for the tests? Would prefer not having commented code.

+        for meth in [cm.__aenter__, cm.__aexit__]:
+            awaitable = meth()
+            self.assertTrue(inspect.isawaitable(awaitable))
+            await awaitable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail because we are trying to await outside of an async function :(

I haven't looked at IsolatedAsyncioTestCase yet! Would that allow us to have an async def test_blah that would solve this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked at IsolatedAsyncioTestCase yet! Would that allow us to have an async def test_blah that would solve this?

I just documented it yesterday ;) yes, you can write async def test_blah and that would be collected by test runner. It's like a drop in replacement to TestCase and in addition also takes coroutines/async functions as test cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! That would make testing AsyncMock more robust, I've found a good number of places I would like to test with await but haven't been able to. Let's make it another issue, I can take these commented lines out and they will eventually get added back in with the new IsolatedAsyncioTestCase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsolatedAsyncioTestCase is not used by any CPython test yet (except self-testing) but I want to rewrite the most of asyncio tests with its usage eventually.

# self.assertTrue(inspect.isawaitable(cm.__aexit__()))

def test_set_return_value_of_aenter(self):
def inner_test(mock_type):
pc = self.ProductionCode()
pc.session = MagicMock(name='sessionmock')
cm = mock_type(name='magic_cm')
response = AsyncMock(name='response')
response.json = AsyncMock(return_value={'json': 123})
cm.__aenter__.return_value = response
pc.session.post.return_value = cm
result = asyncio.run(pc.main())
self.assertEqual(result, {'json': 123})

for mock_type in [AsyncMock, MagicMock]:
with self.subTest(f"test set return value of aenter with {mock_type}"):
inner_test(mock_type)

def test_mock_supports_async_context_manager(self):
called = False
instance = self.WithAsyncContextManager()
mock_instance = MagicMock(instance)
def inner_test(mock_type):
called = False
instance = self.WithAsyncContextManager()
mock_instance = mock_type(instance)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about calling these cm and mock_cm?


async def use_context_manager():
nonlocal called
async with mock_instance as result:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async with mock_cm:

called = True
return result
lisroach marked this conversation as resolved.
Show resolved Hide resolved

async def use_context_manager():
nonlocal called
async with mock_instance as result:
called = True
return result
result = asyncio.run(use_context_manager())
self.assertTrue(called)
self.assertTrue(mock_instance.__aenter__.called)
self.assertTrue(mock_instance.__aexit__.called)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably check if they have been awaited.

self.assertIsNot(mock_instance, result)

for mock_type in [AsyncMock, MagicMock]:
with self.subTest(f"test context manager magics with {mock_type}"):
inner_test(mock_type)

result = asyncio.run(use_context_manager())
self.assertTrue(called)
self.assertTrue(mock_instance.__aenter__.called)
self.assertTrue(mock_instance.__aexit__.called)
self.assertIsNot(mock_instance, result)
self.assertIsInstance(result, AsyncMock)

def test_mock_customize_async_context_manager(self):
instance = self.WithAsyncContextManager()
Expand Down Expand Up @@ -474,27 +524,31 @@ async def __anext__(self):

raise StopAsyncIteration

def test_mock_aiter_and_anext(self):
instance = self.WithAsyncIterator()
mock_instance = MagicMock(instance)

self.assertEqual(asyncio.iscoroutine(instance.__aiter__),
asyncio.iscoroutine(mock_instance.__aiter__))
self.assertEqual(asyncio.iscoroutine(instance.__anext__),
asyncio.iscoroutine(mock_instance.__anext__))

iterator = instance.__aiter__()
if asyncio.iscoroutine(iterator):
iterator = asyncio.run(iterator)

mock_iterator = mock_instance.__aiter__()
if asyncio.iscoroutine(mock_iterator):
mock_iterator = asyncio.run(mock_iterator)
def test_aiter_set_return_value(self):
mock_iter = AsyncMock(name="tester")
mock_iter.__aiter__.return_value = [1, 2, 3]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked about briefly this at the sprint:

  • check what happens with a genexp
  • check what happens without setting a return_value (should still be iterable)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not working quite right yet, but it should be fixed with issue38163

async def main():
async for i in mock_iter:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly confused over if this is needed since we just return a list comprehension and assert only once.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right, didn't notice the double loop

return [i async for i in mock_iter]
result = asyncio.run(main())
self.assertEqual(result, [1, 2, 3])

def test_mock_aiter_and_anext_asyncmock(self):
def inner_test(mock_type):
instance = self.WithAsyncIterator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you are missing the (), you have them at line 438 459.
Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, it makes no difference so tests don't fail but want to keep them consistent.

mock_instance = mock_type(instance)
# Check that the mock and the real thing bahave the same
# __aiter__ is not actually async, so not a coroutinefunction
self.assertFalse(asyncio.iscoroutinefunction(instance.__aiter__))
self.assertFalse(asyncio.iscoroutinefunction(mock_instance.__aiter__))
# __anext__ is async
self.assertTrue(asyncio.iscoroutinefunction(instance.__anext__))
self.assertTrue(asyncio.iscoroutinefunction(mock_instance.__anext__))

for mock_type in [AsyncMock, MagicMock]:
with self.subTest(f"test aiter and anext corourtine with {mock_type}"):
inner_test(mock_type)

self.assertEqual(asyncio.iscoroutine(iterator.__aiter__),
asyncio.iscoroutine(mock_iterator.__aiter__))
self.assertEqual(asyncio.iscoroutine(iterator.__anext__),
asyncio.iscoroutine(mock_iterator.__anext__))

def test_mock_async_for(self):
async def iterate(iterator):
Expand All @@ -505,19 +559,30 @@ async def iterate(iterator):
return accumulator

expected = ["FOO", "BAR", "BAZ"]
with self.subTest("iterate through default value"):
mock_instance = MagicMock(self.WithAsyncIterator())
self.assertEqual([], asyncio.run(iterate(mock_instance)))
def test_default(mock_type):
mock_instance = mock_type(self.WithAsyncIterator())
self.assertEqual(asyncio.run(iterate(mock_instance)), [])


with self.subTest("iterate through set return_value"):
mock_instance = MagicMock(self.WithAsyncIterator())
def test_set_return_value(mock_type):
mock_instance = mock_type(self.WithAsyncIterator())
mock_instance.__aiter__.return_value = expected[:]
self.assertEqual(expected, asyncio.run(iterate(mock_instance)))
self.assertEqual(asyncio.run(iterate(mock_instance)), expected)

with self.subTest("iterate through set return_value iterator"):
mock_instance = MagicMock(self.WithAsyncIterator())
def test_set_return_value_iter(mock_type):
mock_instance = mock_type(self.WithAsyncIterator())
mock_instance.__aiter__.return_value = iter(expected[:])
self.assertEqual(expected, asyncio.run(iterate(mock_instance)))
self.assertEqual(asyncio.run(iterate(mock_instance)), expected)

for mock_type in [AsyncMock, MagicMock]:
with self.subTest(f"default value with {mock_type}"):
test_default(mock_type)

with self.subTest(f"set return_value with {mock_type}"):
test_set_return_value(mock_type)

with self.subTest(f"set return_value iterator with {mock_type}"):
test_set_return_value_iter(mock_type)


class AsyncMockAssert(unittest.TestCase):
Expand Down
3 changes: 3 additions & 0 deletions Lib/unittest/test/testmock/testmagicmethods.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import asyncio
import math
import unittest
import os
Expand Down Expand Up @@ -286,6 +287,8 @@ def test_magicmock_defaults(self):
self.assertEqual(math.trunc(mock), mock.__trunc__())
self.assertEqual(math.floor(mock), mock.__floor__())
self.assertEqual(math.ceil(mock), mock.__ceil__())
self.assertTrue(asyncio.iscoroutinefunction(mock.__aexit__))
self.assertTrue(asyncio.iscoroutinefunction(mock.__aenter__))
lisroach marked this conversation as resolved.
Show resolved Hide resolved

# in Python 3 oct and hex use __index__
# so these tests are for __index__ in py3k
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixes AsyncMock so it doesn't crash when used with AsynContextManagers
lisroach marked this conversation as resolved.
Show resolved Hide resolved
or AsyncIterators.