python / asyncio

asyncio historical repository
https://docs.python.org/3/library/asyncio.html
1.03k stars 178 forks source link

mock.Mock() acts like a Future #454

Closed popravich closed 7 years ago

popravich commented 7 years ago

Since this changes 32edb29d5b0524e0da2400908093de5911f117f4 unittest.mock.Mock being detected as asyncio.Future. asyncio.futures.isfuture checks for private attribute _asyncio_future_blocking to be not None and Mock() (without specs) returns new mock as this attribute.

The use case is here aio-libs/aioredis#161 — basically, function wrapped with asyncio.coroutine and set as Mock's method side_effect started to raise TypeError.

gvanrossum commented 7 years ago

Is there something you want us to do?

--Guido (mobile)

1st1 commented 7 years ago

We can probably check obj.__class__ for _asyncio_future_blocking attribute.

popravich commented 7 years ago

Is there something you want us to do?

Yeah, it was more like FYI. Maybe a little bit stricter check would be better. Some use cases like implementing some kind of RPC with dynamic methods resolution might hit this case

gvanrossum commented 7 years ago

Can someone please translate the original post into an actionable issue for me? I do not understand what the OP is asking.

1st1 commented 7 years ago

@gvanrossum Please see https://github.com/python/asyncio/pull/455.

gvanrossum commented 7 years ago

I'm not at all clear that a Mock should not be considered a Future. The clarification "Some use cases like implementing some kind of RPC with dynamic methods resolution" sounds awefully vague. Maybe there are just as many use cases that would break with this change?

1st1 commented 7 years ago

Maybe there are just as many use cases that would break with this change?

isinstance(unittest.mock.Mock(), asyncio.Future) returns False. If we want to promote to use isfuture instead of isinstance, we should fix it.

gvanrossum commented 7 years ago

isinstance(unittest.mock.Mock(), asyncio.Future) returns False. If we want to promote to use isfuture instead of isinstance, we should fix it.

OK, that makes sense.

gvanrossum commented 7 years ago

Fixed by 26d01d796dccb365d3d6c00f8e35fe0d431b5abf (#455).

popravich commented 7 years ago

isinstance(unittest.mock.Mock(), asyncio.Future) returns False

isinstance(unittest.mock.Mock(asyncio.Future), asyncio.Future) will return True. But isfuture(unittest.mock.Mock(asyncio.Future)) will return False. Is that ok?

1st1 commented 7 years ago

Oh. Let me see how Mocks work.

1st1 commented 7 years ago

But isfuture(unittest.mock.Mock(asyncio.Future)) will return False. Is that ok?

Fixed.