Martiusweb / asynctest

Enhance the standard unittest package with features for testing asyncio libraries
https://asynctest.readthedocs.org/
Apache License 2.0
309 stars 41 forks source link

Option to always return CoroutineMock #100

Open mshandrovskiy opened 6 years ago

mshandrovskiy commented 6 years ago

Hi,

Sorry if this is documented somewhere, but is there a way to force either Mock, MagicMock or CoroutineMock to always return CoroutineMock for method calls? I understand that this can work with a spec parameter, but the library I am using (aioredis) implements the methods in a way that they are not detected as awaitable.

Thanks!

Kentzo commented 6 years ago

May I ask you for an example test case that you would like to make it work to better address this question?

Martiusweb commented 6 years ago

Hi,

This is a tricky problem because there are a lot of edge cases.

CoroutineMock actually mocks a coroutine functions¹, hence if the result of a mocked function call is a CoroutineMock, this will likely not work as you expect (you'll need to call mock.method()() to get an awaitable object).

What you want to do then is to assume that attributes of the mock are coroutine functions (CoroutineMock instances). It's doable by creating subclasses which override MagicMock._get_child_mock(**kwargs), as documented in unittest (https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock._get_child_mock), but it seems usually unsafe (any attribute will be mocked as a CouroutineMock).

However it's still brittle, because it can mock objects incorrectly and hide actual race conditions in subtle ways. For instance:

def not_really_a_coroutine(raise_exception=False):
    if raise_exception:
        raise Exception

    f = asyncio.Future()
    f.set_result("OK!")
    return f

mock_not_really_a_coroutine = CoroutineMock(side_effect=Excetion)

both objects don't raise at the same time:

coro = asyncio.ensure_future(not_really_a_coroutine(True))  # raise right now
coro = asyncio.ensure_future(mock_not_really_a_coroutine(True)) # raise later (when awaited)

Overall, I think there's an issue with the fact that there's not reliable way to identify if a function returns an awaitable or is a coroutine, maybe we should open the discussion in the mailing list of asyncio.

(edit: posted too fast) ¹ I think that this class should be renamed to CoroutineFunctionMock for the sake of clarity.

mshandrovskiy commented 6 years ago

Hey,

Thanks for the thorough answer!

I suppose my ask is more narrow then that. Since the option to do what I am expecting already exists when using the spec parameter, I'd just like a way to force that introspection when I am not using spec, but I am certain about the object that I am mocking. I understand that could be expanding the API surface in a way that is not acceptable to the maintainers.

We've already implemented an internal test framework (that sort of ignores the edge cases), but I would like to switch to an open source alternative. If I need to get into overrides with MagicMock._get_child_mock(**kwargs), I am better off staying with the custom solution we've already built.

Here is a simplified example of code I am testing.

import asyncio

import aioredis
import aioredis as redis

from asynctest import TestCase as AsyncTestCase, Mock as AsyncMock, CoroutineMock
class Foo:
    def __init__(self, redis):
        """ redis initialized elsewhere with
        loop.run_until_complete(
            aioredis.create_redis_pool(redis_addr, loop=loop))  # type: aioredis.Redis
        """
        self.app = app = {}
        app['redis'] = redis

async def set(foo_inst, value):
    await foo_inst.app['redis'].set(value)

class TestFoo(AsyncTestCase):
    def setUp(self):
        self.redis_mock = AsyncMock(spec=redis.Redis)
        self.redis_mock.get = CoroutineMock(return_value=0)

    async def test_set(self):
        """ self.redis.set is called here"""
        await set(Foo(self.redis_mock), 1)
        self.redis_mock.set.assert_called_once_with(1)

results in

    await foo_inst.app['redis'].set(value)
Exception: object Mock can't be used in 'await' expression
Kentzo commented 6 years ago

Overriding _get_child_mock seems like the right approach. Ignore the fact that it starts with _, it's part of the public interface:

from asynctest import Mock, CoroutineMock

class AsyncMock(Mock):
    def _get_child_mock(self, *args, **kwargs):
        return CoroutineMock(*args, **kwargs)
Martiusweb commented 6 years ago

Hm, I'm not exactly sure I understand your request, do you think that an attribute allowing to specify which attributes of a mock should be CoroutineMock instances would answer it?

For instance: redis_mock = asynctest.Mock(coroutine_functions=["set", "get", "read", "write"]) isinstance(redis_mock.get, asynctest.CoroutineMock) # true isinstance(redis_mock.something, asynctest.CoroutineMock) # false

redis_mock = asynctest.Mock(coroutine_functions=Mock.ANY)
isinstance(redis_mock.get, asynctest.CoroutineMock)  # true
isinstance(redis_mock.something, asynctest.CoroutineMock)  # true

I'm still not sure it's a good idea, but I can't find a good solution for this problem (detecting that a function returns an awaitable).

mshandrovskiy commented 6 years ago

Yes, that would definitely work. I am not sure how often the first syntax would be useful (coroutine_functions=["set", "get", "read", "write"]); I think if you are going to list the functions, might as well go ahead and mock them directly.

I think supporting the Mock.ANY flag would address my issue. The tricky part is deciding how (and if) it should be propagated to the child mocks. My intuition is that if you go down this path, all your code is async so propagation is fine, but there could be additional edge cases here.

redis_mock = asynctest.Mock(coroutine_functions=Mock.ANY)
isinstance(redis_mock.execute, asynctest.CoroutineMock)  # true
isinstance(redis_mock.execute.read, asynctest.CoroutineMock)  # ???