python / cpython

The Python programming language
https://www.python.org
Other
62.59k stars 30.04k forks source link

bug in unittest.mock around spec and async callable classes #116055

Open kanetkarster opened 7 months ago

kanetkarster commented 7 months ago

Bug description:

repro:

import inspect
import asyncio
from unittest.mock import AsyncMock

class AsyncCallableClass:
    async def __call__(self):
        await asyncio.sleep(0)

real = AsyncCallableClass()
mock = AsyncMock(spec=AsyncCallableClass())

print(f"{inspect.iscoroutinefunction(real.__call__)=}")
print(f"{inspect.iscoroutinefunction(mock.__call__)=}")

output:


inspect.iscoroutinefunction(real.__call__)=True
inspect.iscoroutinefunction(mock.__call__)=False

I expected the output for the second line to be True but it appears Async|MagicMock(spec=MySpec) doesn't check call

i believe the issues is that _is_async_obj should be:

def _is_async_obj(obj):
    if _is_instance_mock(obj) and not isinstance(obj, AsyncMock):
        return False
    if hasattr(obj, '__func__'):
        obj = getattr(obj, '__func__')
    elif hasattr(obj, '__call__'):
        obj = getattr(obj, '__call__')

    return iscoroutinefunction(obj) or inspect.isawaitable(obj)

CPython versions tested on:

3.10

Operating systems tested on:

No response

kanetkarster commented 7 months ago

happy to submit a PR if folks think this is the right course of action

infohash commented 5 months ago

This is intended as mock.__call__ is inherited from CallableMixin whose __call__ method is not a couroutine function but when it is called by the instance of AsyncMock, it returns a coroutine.

>>> from unittest import mock
>>> issubclass(mock.AsyncMock, mock.CallableMixin)
True

>>> mock.AsyncMock().__call__
<bound method CallableMixin.__call__ of <AsyncMock id='...'>>

>>> import inspect
>>> inspect.iscoroutinefunction(mock.AsyncMock().__call__)
False
>>> inspect.iscoroutine(mock.AsyncMock().__call__())
True
>>> inspect.isawaitable(mock.AsyncMock().__call__())
True

This is not a problem because the instance of AsyncMock will be always awaitable even if the spec class has regular __call__ method which in that case you should use MagicMock instead.

This issue can be closed.

cjw296 commented 5 months ago

@infohash - not sure I agree that this can be closed. The issue reported here is that inspect gives a different answer for a specced mock than it does for the original object. I could see this being important for some use cases.

Perhaps AsyncMock should have its own __call__ method that is sync def?

infohash commented 4 months ago

Perhaps AsyncMock should have its own __call__ method that is async def?

@cjw296 This will still give a different answer if the original class implements __call__ as a regular method.

Same Problem But An Inverted Output

import inspect
from unittest import mock

class AsyncCallableClass:
    def __call__(self):
        pass

>>> async_mocked_class = mock.AsyncMock(spec=AsyncCallableClass)
>>> inspect.iscoroutinefunction(AsyncCallableClass.__call__)
False
>>> inspect.iscoroutinefunction(async_mocked_class.__call__)
True

What matters to the user code is that, it should be able to await the value returned by __call__ method of AsyncMock which is already the case,