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 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions Lib/unittest/mock.py
Expand Up @@ -989,9 +989,9 @@ 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):
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 @@ -1904,6 +1904,8 @@ def method(self, /, *args, **kw):
'__str__': lambda self: object.__str__(self),
'__sizeof__': lambda self: object.__sizeof__(self),
'__fspath__': lambda self: f"{type(self).__name__}/{self._extract_mock_name()}/{id(self)}",
'__aenter__': lambda self: AsyncMockMixin._mock_call(self),
lisroach marked this conversation as resolved.
Show resolved Hide resolved
'__aexit__': lambda self: AsyncMockMixin._mock_call(self)
}

_return_values = {
Expand Down
49 changes: 42 additions & 7 deletions Lib/unittest/test/testmock/testasync.py
Expand Up @@ -363,22 +363,58 @@ 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
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_set_return_value_of_aenter_magicmock(self):
pc = self.ProductionCode()
pc.session = MagicMock(name='sessionmock')
cm = MagicMock(name='magic_cm')
response = AsyncMock(name='response')
response.json = AsyncMock(return_value={'json':123})
Copy link
Member

Choose a reason for hiding this comment

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

PEP8 nit regarding space here and in below dict declarations.

Suggested change
response.json = AsyncMock(return_value={'json':123})
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})

def test_set_return_value_of_aenter_asyncmock(self):
pc = self.ProductionCode()
pc.session = MagicMock(name='sessionmock')
cm = AsyncMock(name='async_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})
Copy link
Member

Choose a reason for hiding this comment

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

I would turn this in a helper method that accepts a MockClass as argument and call it from both these methods passing MagicMock and AsyncMock.


def test_mock_supports_async_context_manager(self):
called = False
instance = self.WithAsyncContextManager()
mock_instance = MagicMock(instance)
# TODO: Horrible error message if you use a MagicMock here
Copy link
Member

Choose a reason for hiding this comment

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

Please open a bpo in case it will not be part of the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I think this is fixed, will remove comment and make it a subTest to test both classes.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the clarification. Mine was due to the comment over todo #15947 (comment). I would be happy if it was fixed then.

mock_instance = AsyncMock(instance)

async def use_context_manager():
nonlocal called
Expand All @@ -391,7 +427,6 @@ async def use_context_manager():
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
@@ -0,0 +1,3 @@
Fixes AsyncMock so MagicMocks on AsyncContextManagers do not have to
manually set `__aenter__` and `__aexit__`. Also fixes AsyncMock so they
don't crash when used with AsyncContextManager.