-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
Changes from 12 commits
7fbd8fa
8ef74fe
d3f2706
bd02a3a
d656592
b831e58
5b3f96a
b4e164c
14e684e
f476b58
0d3af26
7d9dfa0
d671277
bb57b0c
8bc3e32
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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' | ||
|
@@ -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. | ||
|
||
>>> 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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eventually we probably want to replace these either with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, I will update them in this PR if that's okay. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
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) : | ||
|
@@ -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__', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm this is a good point about 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 So I want to leave the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah okay so it's a case where once There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
'__getnewargs_ex__', | ||
} | ||
|
||
|
||
|
@@ -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__', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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__())) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I just documented it yesterday ;) yes, you can write There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
# 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about calling these |
||
|
||
async def use_context_manager(): | ||
nonlocal called | ||
async with mock_instance as result: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||
|
@@ -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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We talked about briefly this at the sprint:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you are missing the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
|
@@ -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): | ||
|
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh,
__aexit__
is anAsyncMock
too, isn't it?There was a problem hiding this comment.
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 😆