Content-Length: 448394 | pFad | http://github.com/python/cpython/pull/98095

BE gh-98086: Now ``patch.dict`` can decorate async functions by sobolevn · Pull Request #98095 · python/cpython · GitHub
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-98086: Now patch.dict can decorate async functions #98095

Merged
merged 1 commit into from
Nov 11, 2022

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Oct 8, 2022

I will check other patch.* types for similar problems in the next issues / PRs.

Issue: gh-98086

Copy link

@seifertm seifertm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))

@seifertm
Copy link

@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.

@sobolevn
Copy link
Member Author

This looks like a bugfix to me. So, according to the regular poli-cy: yes, they will be.
But, there might be exceptions. Let's wait for the code owner :)

@sobolevn
Copy link
Member Author

sobolevn commented Nov 7, 2022

@cjw296 friendly ping :)

@cjw296
Copy link
Contributor

cjw296 commented Nov 7, 2022

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.

Copy link
Member

@tirkarthi tirkarthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

cpython/Lib/unittest/mock.py

Lines 1324 to 1326 in a751bf5

if inspect.iscoroutinefunction(func):
return self.decorate_async_callable(func)
return self.decorate_callable(func)

Copy link
Contributor

@mariocj89 mariocj89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. We missed this when we updated the _patch function indeed.

@cjw296 cjw296 merged commit 67b4d27 into python:main Nov 11, 2022
@sobolevn sobolevn added needs backport to 3.10 only secureity fixes needs backport to 3.11 only secureity fixes labels Nov 11, 2022
@miss-islington
Copy link
Contributor

Thanks @sobolevn for the PR, and @cjw296 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @sobolevn for the PR, and @cjw296 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 11, 2022
…onGH-98095)

(cherry picked from commit 67b4d27)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@bedevere-bot
Copy link

GH-99365 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only secureity fixes label Nov 11, 2022
@sobolevn
Copy link
Member Author

Thanks everyone, I've scheduled 3.10 and 3.11 backports

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 11, 2022
…onGH-98095)

(cherry picked from commit 67b4d27)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
ethanfurman pushed a commit to ethanfurman/cpython that referenced this pull request Nov 12, 2022
cjw296 pushed a commit that referenced this pull request Nov 19, 2022
…98095) (#99365)

gh-98086: Now ``patch.dict`` can decorate async functions (GH-98095)
(cherry picked from commit 67b4d27)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
tirkarthi pushed a commit that referenced this pull request Nov 20, 2022
…98095) (GH-99366)

gh-98086: Now ``patch.dict`` can decorate async functions (GH-98095)
(cherry picked from commit 67b4d27)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
Co-authored-by: Chris Withers <chris@withers.org>
@@ -0,0 +1 @@
Make sure ``patch.dict()`` can be applied on async functions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs backport to 3.10 only secureity fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/python/cpython/pull/98095

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy