gh-98086: Now patch.dict can decorate async functions#98095
gh-98086: Now patch.dict can decorate async functions#98095cjw296 merged 1 commit intopython:mainfrom
patch.dict can decorate async functions#98095Conversation
There was a problem hiding this comment.
Thanks for looking into this! The changes fix the reported issue: coroutines now stay coroutines when applying the patch.dict decorator.
I checked the behavior of all patch.* functions and they seem to be consistent as of this PR. Patching preserves coroutines, even inside of TestCase classes when used as a class decorator.
Note that async generator functions are not preserved. They become functions when decorated with patch.*. However, this is the same for all variants of patch and not part of the reported issue.
Here's the code I used to verify the behavior:
import inspect
from unittest import TestCase
from unittest.mock import DEFAULT, Mock, patch
patchers = (
patch("binascii.b64encode"),
patch.dict("sys.modules", test_module=Mock()),
patch.object(Mock, "my_func"),
patch.multiple(Mock, my_attr=DEFAULT, my_other=DEFAULT),
)
async def coro():
...
patched_coros = (patcher(coro) for patcher in patchers)
assert inspect.iscoroutinefunction(coro)
assert all(map(inspect.iscoroutinefunction, patched_coros))
# Coroutines in test classes
for patcher in patchers:
@patcher
class TestClass(TestCase):
async def test_coro(self):
...
assert inspect.iscoroutinefunction(TestClass.test_coro)
# The behavior across patch functions is consistent with async generator functions
async def async_gen_func():
yield
patched_async_gen_funcs = (patcher(async_gen_func) for patcher in patchers)
assert inspect.isasyncgenfunction(async_gen_func)
assert not all(map(inspect.isasyncgenfunction, patched_async_gen_funcs))
|
@sobolevn Will these changes be considered for backports to earlier Python versions? I'm asking, because I'm trying to assess if pytest-asyncio needs to provide a workaround. |
|
This looks like a bugfix to me. So, according to the regular policy: yes, they will be. |
|
@cjw296 friendly ping :) |
|
I'm afraid I don't have time to review this right now. @tirkarthi / @mariocj89 ? @seifertm - even if merged, this will only be backported to 3.11 and 3.10. |
tirkarthi
left a comment
There was a problem hiding this comment.
LGTM, Thanks @sobolevn . This looks similar to the logic already done for patch and now adopted to patch.dict too. I will wait for review from @mariocj89 and will try to merge it within end of this week along with backport.
Lines 1324 to 1326 in a751bf5
mariocj89
left a comment
There was a problem hiding this comment.
Makes sense. We missed this when we updated the _patch function indeed.
…onGH-98095) (cherry picked from commit 67b4d27) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
|
GH-99365 is a backport of this pull request to the 3.11 branch. |
|
Thanks everyone, I've scheduled 3.10 and 3.11 backports |
…onGH-98095) (cherry picked from commit 67b4d27) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
| @@ -0,0 +1 @@ | |||
| Make sure ``patch.dict()`` can be applied on async functions. | |||
There was a problem hiding this comment.
For future PRs, please make sure that the news fragment makes sense when it’s read in the compiled changelog.
Here patch is mentioned but there is no such builtin, it would be better to name unittest.mock.patch.dict (like the issue helpfully did). Thanks!
I will check other
patch.*types for similar problems in the next issues / PRs.Issue: gh-98086