msg258464 - (view) |
Author: Hiroyuki Takagi (miyakogi) |
Date: 2016-01-17 11:42 |
inspect.iscoroutinefunction and asyncio.iscoroutinefunction with patch of issue25599 (https://bugs.python.org/issue25599) raise TypeError when used to check Mock object which mocks function or coroutinefunction.
How to reproduce:
- For the mock of function
>>> from unittest.mock import Mock
>>> import inspect
>>> def a():...
...
>>> inspect.iscoroutinefunction(Mock(a))
Expected: False
Actual:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File ".../cpython/Lib/inspect.py", line 187, in iscoroutinefunction
object.__code__.co_flags & CO_COROUTINE)
TypeError: unsupported operand type(s) for &: 'Mock' and 'int'
- For the mock of coroutine-function
>>> async def b():...
...
>>> inspect.iscoroutinefunction(Mock(b))
Expected: True
Actual:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File ".../cpython/Lib/inspect.py", line 187, in iscoroutinefunction
object.__code__.co_flags & CO_COROUTINE)
TypeError: unsupported operand type(s) for &: 'Mock' and 'int'
Without the patch of issue25599, asyncio.iscoroutinefunction does not raise error and returns Mock object. But I don't think it is expected behavior, as discussed in that issue.
I wrote a patch to solve this problem.
|
msg258494 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2016-01-18 03:05 |
Why fix this in mock rather than in the iscoroutinefunction functions? (Not
a rhetorical question -- I really am not sure what the better place is to
fix this, or if we should even consider fixing it.)
|
msg258521 - (view) |
Author: Hiroyuki Takagi (miyakogi) |
Date: 2016-01-18 12:54 |
Thank you for review and comment.
Honestly speaking, I couldn't find any other good place to fix it.
One possible solution might be to use isinstance(mock, Mock) in iscoroutinefunction, but I don't think it's good for inspect module to add special check and depend on unittest.mock. Mocks are usually used only in debug/test, but iscoroutinefunction is used in production code. Adding some check to iscoroutinefunction only for test is not good for performance (though, actually its effect will be very small).
The reasons why I think this behavior should be fixed are,
- Raising error and stopping test is not kind for mock users
- After the patch (issue25599), no mock object can become `True` to iscoroutinefunction, which will make it impossible to test the block after if-iscoroutinefunction by using mock.
Now, I checked inspect module again, and found one more unexpected behavior (not error).
>>> def a(): yield 1
>>> inspect.isgeneratorfunction(a)
True
>>> inspect.isgeneratorfunction(Mock(a))
False
With the patch, inspect.isgeneratorfunction(Mock(a)) returns True.
|
msg258538 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-01-18 18:54 |
It looks like this should be fixed in the mock module, as special casing it in inspect doesn't look right. Unfortunately, I can't review this patch, as I don't know the mock module internals and I don't think I understand all consequences of this patch.
Michael, what do you think?
|
msg258575 - (view) |
Author: Michael Foord (michael.foord) * |
Date: 2016-01-19 09:39 |
In inspect checking that __code__ is a code object, or that co_flags is an int, would be better than special casing mock.
However, the patch to mock looks reasonable to me. It copies the whole code object from the original function to the mock object. The patch needs a test (I'd like to see a test for the Mock(func) case and the create_autospec(func) case.)
|
msg258638 - (view) |
Author: Hiroyuki Takagi (miyakogi) |
Date: 2016-01-20 03:46 |
Thank you for reviewing patch.
I wrote test and updated patch. To pass the test, both this patch and issue25599's patch are required.
Changes of the patch:
- copy __code__ not only functions but also methods
- add autospec (create_autospec) suppoort
I have completely missed about autospec, thank you for a mention about it.
For autospec, simply copying original __code__ to funcopy makes error on existing tests.
That's why I changed the src of exec, but it seems to be quite ad-hoc. It may be better to be improved, but I don't have any good idea, sorry.
On the tests of this patch, I wonder if it's better to use assertIs(.., True/False) instead of assertTrue/False, since it was one of the problem in issue25599.
To apply this change and pass test, need to change asyncio.iscoroutinefunction to return bool. The change would be very easy, just update issue25599's patch like `return_value = bool(getattr(func, ...`.
|
msg258834 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2016-01-22 19:28 |
Am I really still needed on this issue?
On Tue, Jan 19, 2016 at 7:46 PM, Hiroyuki Takagi <report@bugs.python.org>
wrote:
>
> Hiroyuki Takagi added the comment:
>
> Thank you for reviewing patch.
>
> I wrote test and updated patch. To pass the test, both this patch and
> issue25599's patch are required.
>
> Changes of the patch:
> - copy __code__ not only functions but also methods
> - add autospec (create_autospec) suppoort
>
> I have completely missed about autospec, thank you for a mention about it.
> For autospec, simply copying original __code__ to funcopy makes error on
> existing tests.
> That's why I changed the src of exec, but it seems to be quite ad-hoc. It
> may be better to be improved, but I don't have any good idea, sorry.
>
> On the tests of this patch, I wonder if it's better to use assertIs(..,
> True/False) instead of assertTrue/False, since it was one of the problem in
> issue25599.
> To apply this change and pass test, need to change
> asyncio.iscoroutinefunction to return bool. The change would be very easy,
> just update issue25599's patch like `return_value = bool(getattr(func, ...`.
>
> ----------
> Added file: http://bugs.python.org/file41664/mock2.patch
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue26140>
> _______________________________________
>
|
msg352323 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2019-09-13 13:29 |
I believe after implementing AsyncMock we can close the issue
|
msg352329 - (view) |
Author: Karthikeyan Singaravelan (xtreak) * |
Date: 2019-09-13 13:38 |
I believe it's still a problem with Mock though MagicMock and AsyncMock work but I am not sure what would be the correct behavior that whether Mock should behave like a coroutine or not as per the discussion here.
./python.exe
Python 3.9.0a0 (heads/master:375a3e2bdb, Sep 13 2019, 14:18:36)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from unittest.mock import Mock, MagicMock, AsyncMock
>>> import inspect
>>> def f(): pass
...
>>> for m in MagicMock, AsyncMock : print(inspect.iscoroutinefunction(m(f)))
...
True
True
>>> for m in Mock, MagicMock, AsyncMock : print(inspect.iscoroutinefunction(m(f)))
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/inspect.py", line 194, in iscoroutinefunction
return _has_code_flag(obj, CO_COROUTINE)
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/inspect.py", line 180, in _has_code_flag
return bool(f.__code__.co_flags & flag)
TypeError: unsupported operand type(s) for &: 'Mock' and 'int'
|
msg352341 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2019-09-13 14:29 |
I believe with AsyncMock implemented we can close the issue
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:26 | admin | set | github: 70328 |
2019-09-13 14:29:34 | asvetlov | set | messages:
+ msg352341 |
2019-09-13 13:38:49 | xtreak | set | messages:
+ msg352329 |
2019-09-13 13:29:04 | asvetlov | set | status: open -> closed
nosy:
+ asvetlov messages:
+ msg352323
resolution: out of date stage: resolved |
2019-06-12 19:32:10 | vstinner | set | nosy:
- vstinner
|
2019-06-12 18:18:55 | xtreak | set | nosy:
+ lisroach, xtreak
versions:
+ Python 3.8, Python 3.9, - Python 3.5 |
2016-01-25 10:31:04 | michael.foord | set | nosy:
- gvanrossum
|
2016-01-22 19:28:59 | gvanrossum | set | messages:
+ msg258834 |
2016-01-20 03:46:14 | miyakogi | set | files:
+ mock2.patch
messages:
+ msg258638 |
2016-01-19 09:39:28 | michael.foord | set | messages:
+ msg258575 |
2016-01-18 18:54:08 | yselivanov | set | nosy:
+ michael.foord messages:
+ msg258538
|
2016-01-18 12:54:46 | miyakogi | set | messages:
+ msg258521 |
2016-01-18 03:05:31 | gvanrossum | set | messages:
+ msg258494 |
2016-01-17 11:42:52 | miyakogi | create | |