Fix asyncio mock warnings

[asvetlov]

BPO	37015
Nosy	@asvetlov, @lisroach, @mariocj89, @miss-islington, @tirkarthi
PRs	bpo-37015: Ensure tasks created by _accept_connection2 due to AsyncMock are completed #13661

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-05-30.10:07:09.670>
created_at = <Date 2019-05-22.17:56:03.539>
labels = ['3.8', 'type-bug', 'tests']
title = 'Fix asyncio mock warnings'
updated_at = <Date 2019-05-30.10:07:09.669>
user = 'https://github.com/asvetlov'

bugs.python.org fields:

activity = <Date 2019-05-30.10:07:09.669>
actor = 'asvetlov'
assignee = 'none'
closed = True
closed_date = <Date 2019-05-30.10:07:09.670>
closer = 'asvetlov'
components = ['Tests']
creation = <Date 2019-05-22.17:56:03.539>
creator = 'asvetlov'
dependencies = []
files = []
hgrepos = []
issue_num = 37015
keywords = ['patch']
message_count = 9.0
messages = ['343223', '343228', '343229', '343230', '343352', '343366', '343368', '343962', '343964']
nosy_count = 5.0
nosy_names = ['asvetlov', 'lisroach', 'mariocj89', 'miss-islington', 'xtreak']
pr_nums = ['13661']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37015'
versions = ['Python 3.8']

[asvetlov]

After merging #9296 asyncio test suite prints a lot of warnings like

Exception ignored in: <coroutine object AsyncMockMixin._mock_call at 0x7fb648a1c170>
Traceback (most recent call last):
  File "/home/andrew/projects/cpython/Lib/warnings.py", line 510, in _warn_unawaited_coroutine
    warn(msg, category=RuntimeWarning, stacklevel=2, source=coro)
RuntimeWarning: coroutine 'AsyncMockMixin._mock_call' was never awaited

I believe this is not a sign of AsyncMock problem but asyncio tests should be fixed.
Need more investigations though.

[tirkarthi]

Not sure if it helps. Set backlog in the below test as 1. Add a print statement print(type(_mock_call)) at [0] . I could see some of _mock_call to be AsyncMock . So instead of calling return _mock_self._mock_call(*args, **kwargs) directly if I call them only with they are not instance of AsyncMock (not isinstance(_mock_self, AsyncMock)) then the warning goes away.

Guess somewhere an AsyncMock is created as I can see _accept_connection2. When the backlog value is set to 100 it shows lot of warnings.

$ ./python.exe -Werror -X tracemalloc -m unittest -vv test.test_asyncio.test_selector_events.BaseSelectorEventLoopTests.test_accept_connection_multiple

test_accept_connection_multiple (test.test_asyncio.test_selector_events.BaseSelectorEventLoopTests) ... <AsyncMock name='_accept_connection2' id='4465398880'>
<class 'unittest.mock.Mock'>
<class 'unittest.mock.Mock'>
<class 'unittest.mock.AsyncMock'>
<class 'unittest.mock.MagicMock'>
<class 'unittest.mock.Mock'>
ok

----------------------------------------------------------------------
Ran 1 test in 0.123s

OK
Exception ignored in: <coroutine object AsyncMockMixin._mock_call at 0x10a2a0e60>
Traceback (most recent call last):
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/warnings.py", line 510, in _warn_unawaited_coroutine
    warn(msg, category=RuntimeWarning, stacklevel=2, source=coro)
RuntimeWarning: coroutine 'AsyncMockMixin._mock_call' was never awaited

[0]

cpython/Lib/unittest/mock.py

Line 991 in b121f63

_mock_self._mock_check_sig(*args, **kwargs)

[tirkarthi]

patching _accept_connection2 attribute on loop object seems to return an AsyncMock.

➜ cpython git:(master) ✗ cat ../backups/bpo37015.py
import asyncio
from unittest.mock import patch
with patch.object(asyncio.get_event_loop(), '_accept_connection2') as f:
print(f)
f()
➜ cpython git:(master) ✗ ./python.exe ../backups/bpo37015.py
<AsyncMock name='_accept_connection2' id='4361411632'>
../backups/bpo37015.py:5: RuntimeWarning: coroutine 'AsyncMockMixin._mock_call' was never awaited
f()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

Relevant test

    def test_accept_connection_multiple(self):
        sock = mock.Mock()
        sock.accept.return_value = (mock.Mock(), mock.Mock())
        backlog = 1
        # Mock the coroutine generation for a connection to prevent
        # warnings related to un-awaited coroutines.
        mock_obj = mock.patch.object
        with mock_obj(self.loop, '_accept_connection2') as accept2_mock:
            print(f"{accept2_mock=}")
            accept2_mock.return_value = None
            with mock_obj(self.loop, 'create_task') as task_mock:
                task_mock.return_value = None
                self.loop._accept_connection(
                    mock.Mock(), sock, backlog=backlog)
        self.assertEqual(sock.accept.call_count, backlog)

When I specify new value which defaults to DEFAULT as Mock() then there is no AsyncMock. Same can be done in test and the warnings go away. My suspicion is that if there is a loop object with _accept_connection2 earlier in Python 3.7 <MagicMock name='_accept_connection2'> is returned by patch.object but now it returns an <AsyncMock name='_accept_connection2'> instead

# use explicit mock

import asyncio
from unittest.mock import patch, Mock

with patch.object(asyncio.get_event_loop(), '_accept_connection2', Mock()) as f:
    print(f)
    f()

➜ cpython git:(master) ✗ ./python.exe ../backups/bpo37015.py
<Mock id='4342533248'>

[tirkarthi]

I guess cause is at [0] . When there is no new value specified and with spec being None if mock.patch is used on an async object (original = _accept_connection2). Here original is an async object then AsyncMock is returned.

Changing this causes test failures where patching AsyncClass.async_method now expects an AsyncMock but as per older behavior it returns MagicMock. I think it's a behavior that needs to be discussed as it differs from 3.7 and not sure if AsyncMock would always be awaited as seen in test_accept_connection_multiple.

[0]

cpython/Lib/unittest/mock.py

Line 1313 in b121f63

if spec is None and _is_async_obj(original):

if spec is None and _is_async_obj(original):
    Klass = AsyncMock
else:
    Klass = MagicMock

[tirkarthi]

This logic is present in create_autospec too at [0]. The behavior for patch seems to be documented at [1] which also needs to be updated to reflect that an AsyncMock by default is created instead of MagicMock if the target is an async function unlike 3.7. Probably also a versionchanged directive to note this if the current behavior is okay. Since it generates a RuntimeWarning I think it will be good if the behavior is decided before 3.8 beta 1.

If new is omitted, then the target is replaced with a MagicMock. If patch() is used as a decorator and new is omitted, the created mock is passed in as an extra argument to the decorated function. If patch() is used as a context manager the created mock is returned by the context manager.

$ python3.7 -q
>>> import asyncio
>>> from unittest.mock import create_autospec
>>> loop = asyncio.get_event_loop()
>>> loop_mock = create_autospec(loop)
>>> loop_mock._accept_connection2
<MagicMock name='mock._accept_connection2' spec='method' id='4455012896'>

$ ./python.exe -q
>>> import asyncio
>>> from unittest.mock import create_autospec
>>> loop = asyncio.get_event_loop()
>>> loop_mock = create_autospec(loop)
>>> loop_mock._accept_connection2
<AsyncMock name='mock._accept_connection2' spec='method' id='4314046368'>

[0]

cpython/Lib/unittest/mock.py

Line 2564 in cccc11b

if asyncio.iscoroutinefunction(original):

[1] https://docs.python.org/3/library/unittest.mock.html#unittest.mock.patch

[asvetlov]

xtreak thanks for investigation.
I have no time to work on it now, sorry.
Maybe the next week.
Anyway, the problem can be fixed even in python beta stage if we don't do it earlier.

[tirkarthi]

@asvetlov No problem then if it can be fixed after beta.

[miss-islington]

New changeset 0f39c2b by Miss Islington (bot) (Xtreak) in branch 'master':
bpo-37015: Ensure tasks created by _accept_connection2 due to AsyncMock are completed (GH-13661)
0f39c2b

[asvetlov]

Fixed