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-72889: remove redundant mock.Mock()._is_coroutine = False workarounds #94926

Merged

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Jul 17, 2022

@bedevere-bot bedevere-bot added awaiting review tests Tests in the Lib/test dir labels Jul 17, 2022
Comment on lines 1287 to -1289
self.loop._add_reader = mock.Mock()
self.loop._add_reader._is_coroutine = False
Copy link
Contributor Author

@graingert graingert Jul 17, 2022

Choose a reason for hiding this comment

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

when asyncio.iscoroutinefunction was implemented as:

def iscoroutinefunction(func):
"""Return True if func is a decorated coroutine function."""
return (getattr(func, '_is_coroutine', False) or
_inspect_iscoroutinefunction(func))

this meant

self.loop._add_reader = mock.Mock()
assert asyncio.iscoroutinefunction(self.loop._add_reader) is True  # mock objects have a truthy _is_coroutine attribute!
self.loop._add_reader._is_coroutine = False
assert asyncio.iscoroutinefunction(self.loop._add_reader) is False  # patching it works around the issue

however the implementation was changed to use a marker object:

# A marker for iscoroutinefunction.
_is_coroutine = object()
def iscoroutinefunction(func):
"""Return True if func is a decorated coroutine function."""
return (inspect.iscoroutinefunction(func) or
getattr(func, '_is_coroutine', None) is _is_coroutine)

and so now the _is_coroutine = False work-around is redundant:

self.loop._add_reader = mock.Mock()
assert asyncio.iscoroutinefunction(self.loop._add_reader) is False  # mock objects have an _is_coroutine but it's not the asyncio.coroutines._is_coroutine sentinel
self.loop._add_reader._is_coroutine = False
assert asyncio.iscoroutinefunction(self.loop._add_reader) is False  # it's still False so the workaround is redundant

@graingert
Copy link
Contributor Author

I don't think this needs a news entry

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Since this just changes tests and the tests pass, I think this is okay.

@gvanrossum gvanrossum merged commit 07aeb74 into python:main Jul 17, 2022
@miss-islington
Copy link
Contributor

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

@bedevere-bot
Copy link

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

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 17, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 17, 2022
…rkarounds (pythonGH-94926)

(cherry picked from commit 07aeb74)

Co-authored-by: Thomas Grainger <[email protected]>
@graingert graingert deleted the remove-redudant-mock-is-coroutine-workarounds branch July 17, 2022 17:23
miss-islington added a commit that referenced this pull request Jul 17, 2022
…nds (GH-94926)

(cherry picked from commit 07aeb74)

Co-authored-by: Thomas Grainger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir topic-asyncio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants