◐ Shell
reader mode source ↗
Skip to content

gh-81432: Make create_autospec generate AsyncMocks for awaitable classes#25347

Open
msuozzo wants to merge 4 commits into
python:mainfrom
msuozzo:mock-awaitable-class
Open

gh-81432: Make create_autospec generate AsyncMocks for awaitable classes#25347
msuozzo wants to merge 4 commits into
python:mainfrom
msuozzo:mock-awaitable-class

Conversation

@msuozzo

@msuozzo msuozzo commented Apr 11, 2021 •
edited by gpshead
Loading

Copy link
Copy Markdown
Contributor

@msuozzo msuozzo mentioned this pull request Apr 11, 2021
Merged
@gpshead gpshead requested a review from cjw296 April 11, 2021 17:36
@cjw296 cjw296 requested a review from tirkarthi April 11, 2021 18:51
@cjw296

cjw296 commented Apr 11, 2021

Copy link
Copy Markdown
Contributor

@tirkarthi / @lisroach - thoughts?

cjw296
cjw296 requested changes Apr 12, 2021
View reviewed changes

@cjw296 cjw296 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

@lindacorbett - not sure this is ready to merge, and it's a tricky PR related to a problem that cropped up elsewhere. I don't have time to look until later this week, but I'd like someone with a lot of familiarity with the mock module to think this through...

@bedevere-bot

bedevere-bot commented Apr 12, 2021

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@msuozzo

msuozzo commented Apr 12, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Alright this is in a functional state. I'm not too familiar with AsyncMock but I think these changes achieve the intent of the original bug+test.

@lisroach PTAL

@tirkarthi

tirkarthi commented Apr 14, 2021

Copy link
Copy Markdown
Member

I must admit the change initially looks a little complicated to review since we had several discussion over how to detect async/coroutine properly, when to return AsyncMock, etc. and just want to understand if there is any behavior change though tests pass. I will review this PR a little later as I am working on few other things. Thanks @msuozzo .

@tirkarthi tirkarthi requested review from lisroach and voidspace April 24, 2021 06:12
@github-actions

github-actions Bot commented Jun 3, 2021

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Jun 3, 2021
@jeremycline jeremycline mannequin mentioned this pull request Apr 10, 2022
Open
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Aug 6, 2022
@cjw296

cjw296 commented Dec 27, 2022

Copy link
Copy Markdown
Contributor

@lisroach - ping?

@gpshead gpshead changed the title bpo-37251: Make create_autospec generate AsyncMocks for awaitable classes Dec 27, 2022
@cjw296 cjw296 mentioned this pull request Dec 27, 2022
Merged
cjw296 added a commit that referenced this pull request Dec 28, 2022
@cjw296
Remove skipped test in test for async mocks. (#100559) …
984894a 
Remove skipped test.

See discussion on #25326.
Fix is apparently here, but no-one is confident to review and land: #25347.
cjw296 added a commit to cjw296/mock that referenced this pull request Dec 28, 2022
@cjw296
Remove skipped test in test for async mocks. (#100559) …
cf41727 
Remove skipped test.

See discussion on python/cpython#25326.
Fix is apparently here, but no-one is confident to review and land: python/cpython#25347.

Backports: 984894a9a25c0f8298565b0c0c2e1f41917e4f88
Signed-off-by: Chris Withers <chris@simplistix.co.uk>
cjw296 added a commit to testing-cabal/mock that referenced this pull request Dec 28, 2022
@cjw296
Remove skipped test in test for async mocks. (#100559) …
a88d686 
Remove skipped test.

See discussion on python/cpython#25326.
Fix is apparently here, but no-one is confident to review and land: python/cpython#25347.

Backports: 984894a9a25c0f8298565b0c0c2e1f41917e4f88
Signed-off-by: Chris Withers <chris@simplistix.co.uk>
jessecomeau87 pushed a commit to jessecomeau87/Python that referenced this pull request May 20, 2024
@cjw296
Remove skipped test. …
eea4d79 
See discussion on python/cpython#25326.
Fix is apparently here, but no-one is confident to review and land: python/cpython#25347.
@python-cla-bot

python-cla-bot Bot commented Apr 6, 2025

Copy link
Copy Markdown

The following commit authors need to sign the Contributor License Agreement:

CLA signed

@cjw296 cjw296 removed the request for review from voidspace April 7, 2025 06:53
@github-actions

github-actions Bot commented Apr 9, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tirkarthi tirkarthi tirkarthi left review comments

@cjw296 cjw296 cjw296 requested changes

@lisroach lisroach Awaiting requested review from lisroach

Assignees

@cjw296 cjw296

Labels

awaiting changes stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@msuozzo @cjw296 @bedevere-bot @tirkarthi @the-knights-who-say-ni @ezio-melotti