Skip to content

bpo-32299: Return patched dict when using patch.dict as a context manager #11062

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 28, 2019
Merged

bpo-32299: Return patched dict when using patch.dict as a context manager #11062

merged 2 commits into from
May 28, 2019

Conversation

mariocj89
Copy link
Contributor

@mariocj89 mariocj89 commented Dec 10, 2018 •
edited by bedevere-bot
Loading

Return the patched version of the dict when unittest.mock.patch.dict is used as a context manager. This change mirrors the rest of the APIs that patch objects in unittest.mock

Closes: #4834

This PR brings the commit from the abandoned PR #4834 with the comments addressed. See #4834 (comment)

https://bugs.python.org/issue32299

@mariocj89 mariocj89 mentioned this pull request Dec 10, 2018
Closed
merwok
merwok reviewed Dec 11, 2018
View reviewed changes
@mariocj89 mariocj89 force-pushed the pu/patch-dict-returns branch 2 times, most recently from 070e22f to fc2ec3f Compare December 11, 2018 23:33
@mariocj89
Copy link
Contributor Author

Thanks @merwok, that needed fixing indeed.

@pablogsal pablogsal requested a review from asvetlov February 18, 2019 10:46
merwok
merwok reviewed Feb 28, 2019
View reviewed changes
tirkarthi
tirkarthi reviewed Mar 1, 2019
View reviewed changes
@mariocj89 mariocj89 force-pushed the pu/patch-dict-returns branch from e8c75a7 to 78db402 Compare March 3, 2019 01:19
@mariocj89 mariocj89 force-pushed the pu/patch-dict-returns branch from 78db402 to 8f8a95f Compare April 11, 2019 07:52
merwok
merwok reviewed Apr 11, 2019
View reviewed changes
@mariocj89 mariocj89 force-pushed the pu/patch-dict-returns branch from 0988491 to eefbf07 Compare April 11, 2019 22:55
@mariocj89 @taiye-ts
bpo-32299: Return patched dict when using patch.dict as a context man…
9320c85 
…ager

Return the patched version of the dict when `unittest.mock.patch.dict`
is used as a context manager. This change mirrors the rest of the APIs
that patch objects in `unittest.mock`

Co-authored-by: Vadim Tsander <ts.taiye@gmail.com>
@mariocj89 mariocj89 force-pushed the pu/patch-dict-returns branch from eefbf07 to 9320c85 Compare April 12, 2019 08:11
@mariocj89
Copy link
Contributor Author

@merwok ready for another review.

Also, are you around the PyCon sprints?

@merwok
Copy link
Member

merwok commented May 7, 2019

I am not at PyCon!

A note about PRs for the future: it’s best for reviewers if you add commits, including merge commits, so that we can easily see changes done after reviews. We always squash merge for CPython so no worry about history cleanliness while the PR is open :)

merwok
merwok reviewed May 7, 2019
View reviewed changes
@csabella
Copy link
Contributor

@merwok, it looks like this one is ready to merge. Were you waiting on anything else? Thanks!

@tirkarthi
Copy link
Member

Would it be possible to get this merged? This would be a nice little addition for 3.8

Thanks

@merwok
Copy link
Member

merwok commented May 28, 2019

@csabella These days I tend to approve some PRs but not merge them, as I lack the focus to see buildbots, handle backports, etc.

@csabella
Copy link
Contributor

csabella commented May 28, 2019 •
edited
Loading

Bases on @merwok's earlier approval, I'll merge this. If additional changes need to be made, they can be done in a new PR.

Thanks, @mariocj89 for the contribution and @merwok for the review. And thanks, @tirkarthi for the nudge. 🎉

@csabella csabella merged commit 0453081 into python:master May 28, 2019
@csabella
Copy link
Contributor

@csabella These days I tend to approve some PRs but not merge them, as I lack the focus to see buildbots, handle backports, etc.

That's good to know. I try not to step on others work by merging changes they have been working on, but if you would like me to merge your approvals, I'd be happy to help. Just ping me to let me know. 🙂

@mariocj89 mariocj89 deleted the pu/patch-dict-returns branch May 28, 2019 12:55
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@mariocj89 @DinoV
bpo-32299: Return patched dict when using patch.dict as a context man…
1c7891f 
…ager (pythonGH-11062)
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

@merwok merwok merwok approved these changes

@asvetlov asvetlov Awaiting requested review from asvetlov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mariocj89 @merwok @csabella @tirkarthi @the-knights-who-say-ni @bedevere-bot