Skip to content

/ cpython

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

bpo-25597: Ensure wraps' return value is used for magic methods in MagicMock #16029

Merged
merged 4 commits into from Jan 27, 2020
Merged

bpo-25597: Ensure wraps' return value is used for magic methods in MagicMock #16029

merged 4 commits into from Jan 27, 2020
+56 −0

Conversation

@tirkarthi
Copy link
Member

tirkarthi commented Sep 12, 2019 •
edited by bedevere-bot

https://bugs.python.org/issue25597
tirkarthi added 3 commits Aug 19, 2019
@tirkarthi
Ensure wraps is called for magic methods in MagicMock.
af4026c
@tirkarthi
Ensure wraps is used for magic methods when passed.
cac9f78
@tirkarthi
Add NEWS entry.
Loading status checks…
859d1b9
@mangrisano
mangrisano approved these changes Sep 12, 2019
View changes
Copy link
Contributor

mangrisano left a comment

LGTM.
@csabella csabella requested a review from cjw296 Jan 25, 2020
@cjw296
cjw296 requested changes Jan 26, 2020
View changes
Copy link
Contributor

cjw296 left a comment

Great, but the news entry needs some work, not sure my interpretation is correct?
Misc/NEWS.d/next/Library/2019-09-12-12-11-05.bpo-25597.mPMzVx.rst Outdated
@@ -0,0 +1,3 @@
Ensure ``wraps`` is supplied to :class:`unittest.mock.MagicMock` is used to
calculate return values for the magic methods instead of using the default

This comment has been minimized.

Sign in to view
Copy link
@cjw296

cjw296 Jan 26, 2020

Contributor

"Ensure, if wraps is supplied to :class:unittest.mock.MagicMock, it is used to" maybe?

This comment has been minimized.

Sign in to view
Copy link
@tirkarthi

tirkarthi Jan 26, 2020

Author Member

Thanks Chris, updated.
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 26, 2020

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.
@tirkarthi
Update news entry.
Loading status checks…
a76e310
@tirkarthi

This comment has been minimized.

Sign in to view
Copy link
Member Author

tirkarthi commented Jan 26, 2020

I have made the requested changes; please review again
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 26, 2020

Thanks for making the requested changes!

@cjw296: please review the changes made to this pull request.
@bedevere-bot bedevere-bot requested a review from cjw296 Jan 26, 2020
@cjw296
cjw296 approved these changes Jan 27, 2020
View changes
@cjw296 cjw296 merged commit 72b1004 into python:master Jan 27, 2020
9 checks passed
9 checks passed
Docs
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20200126.11 succeeded
Details
bedevere/issue-number Issue number 25597 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 27, 2020

@cjw296: Please replace # with GH- in the commit message next time. Thanks!
@tirkarthi

This comment has been minimized.

Sign in to view
Copy link
Member Author

tirkarthi commented Jan 27, 2020

Thanks Chris :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjw296 cjw296

@mangrisano mangrisano

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
5 participants
@tirkarthi @bedevere-bot @cjw296 @mangrisano @the-knights-who-say-ni
You can’t perform that action at this time.