Skip to content

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Jul 17, 2022

@graingert graingert requested review from 1st1 and asvetlov as code owners July 17, 2022 13:56
@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 <tagrain@gmail.com>
@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 <tagrain@gmail.com>
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