bpo-32299: unittest.mock.patch.dict now returns patched dict when used as context manager #4834

ts-taiye · 2017-12-13T09:59:15Z

example:

>>> from unittest.mock import patch
>>> with patch.dict(a, a=2) as patched:
...     print(patched)
... 
{'a': 2}
>>> print(a)
{'a': '1'}

https://bugs.python.org/issue32299

…context manager

the-knights-who-say-ni · 2017-12-13T09:59:17Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

We also demand... A SHRUBBERY!

Thanks again to your contribution and we look forward to looking at it!

ts-taiye · 2017-12-13T10:15:30Z

Signed CLA, waiting my profile to be updated.
Should i add something to news?

asvetlov

Looks good but NEWS entry and documentation update are required for the PR

bedevere-bot · 2017-12-13T10:15:46Z

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.

…context manager

ts-taiye · 2017-12-14T07:33:48Z

up, CLA signed

asvetlov · 2017-12-14T10:31:02Z

Docs update is required (https://docs.python.org/3.7/library/unittest.mock.html#patch-dict)
Source file is ./Docs/library/unittest-mock.rst

ts-taiye · 2017-12-15T08:33:33Z

will update it in a hour

…context manager

srinivasreddy · 2017-12-26T06:52:52Z

@asvetlov This is good to go.

asvetlov · 2017-12-26T09:57:19Z

Documentation should be updated to reflect a value returned by context manager.

…into fix-issue-32299

ts-taiye · 2017-12-27T12:03:18Z

omg, forgot to push commit) sorry)

asvetlov · 2017-12-27T12:22:56Z

Please add something like

      :func:`patch.dict` returns a patched dictionary when used as context manager.

at very end of patch.dict chapter, just before next patch.multiple one.

…context manager

ts-taiye · 2018-01-04T10:51:56Z

done

berkerpeksag · 2018-01-23T08:14:32Z

Doc/library/unittest.mock.rst

@@ -1331,6 +1332,8 @@ magic methods :meth:`__getitem__`, :meth:`__setitem__`, :meth:`__delitem__` and
    >>> assert thing['one'] == 1
    >>> assert list(thing) == ['one']

+:func:`patch.dict` returns a patched dictionary when used as context manager.


This needs to be moved after the paragraph that starts with ":func:`patch.dict` can be used as a context manager, [...]".

Also, add a versionchanged marker after that paragraph:

.. versionchanged:: 3.7 :func:`patch.dict` now returns the patched dictionary when it used as a context manager.

(Feel free to use any wording you want, I just gave an example.)

berkerpeksag · 2018-01-23T08:18:07Z

Lib/unittest/test/testmock/testpatch.py

+    def test_patch_dict_as_context_manager(self):
+        foo = {'a': 'b'}
+        with patch.dict(foo, a='c') as patched:
+            self.assertDictEqual(patched, {'a': 'c'})


No need to use assertDictEqual explicitly here. assertEqual will use assertDictEqual if a and b both dicts:

cpython/Lib/unittest/case.py

Line 423 in 83c8675

self.addTypeEqualityFunc(dict, 'assertDictEqual')

berkerpeksag · 2018-01-23T08:18:59Z

Misc/NEWS.d/next/Library/2017-12-13-17-49-56.bpo-32299.eqAPWs.rst

@@ -0,0 +1 @@
+Changed ``mock.patch.dict`` to return patched dictionary when used as context manager


You way want to add 'the' before 'patched'.

berkerpeksag · 2018-01-23T08:19:21Z

Misc/NEWS.d/next/Library/2017-12-13-17-49-56.bpo-32299.eqAPWs.rst

@@ -0,0 +1 @@
+Changed ``mock.patch.dict`` to return patched dictionary when used as context manager


Nitpick: Please finish the sentence with a full stop.

berkerpeksag · 2018-01-23T08:19:45Z

Misc/NEWS.d/next/Library/2017-12-13-17-49-56.bpo-32299.eqAPWs.rst

@@ -0,0 +1 @@
+Changed ``mock.patch.dict`` to return patched dictionary when used as context manager


Please add "Patch by Your Name.".

berkerpeksag · 2018-01-23T08:22:50Z

Doc/library/unittest.mock.rst

    ...     assert foo == {'newkey': 'newvalue'}
+    ...     assert patched_foo == {'newkey': 'newvalue'}
    ...


Could you also add the assignment example you gave in the issue?

patched_foo['spam'] = 'eggs' assert patched_foo['spam'] == 'eggs'

csabella · 2018-01-23T14:03:30Z

@ts-taiye It looks like your patch is really close to being finished. If you include the last suggestions by @berkerpeksag , then they may be able to approve this.

mariocj89 · 2018-10-27T12:50:42Z

@ts-taiye this would be neat! Many people ask about why mock.patch.dict does not return the patched object like mock.patch.object or mock.patch. It makes it not so intuitive

Do you plan on taking the changes? I'd be happy to help you out through the remaining bits if you want :)

ts-taiye · 2018-10-28T15:31:10Z

@ts-taiye this would be neat! Many people ask about why mock.patch.dict does not return the patched object like mock.patch.object or mock.patch. It makes it not so intuitive

Do you plan on taking the changes? I'd be happy to help you out through the remaining bits if you want :)

Yes, i plan to, try to do it tomorrow

ts-taiye · 2018-11-05T18:05:07Z

@ts-taiye this would be neat! Many people ask about why mock.patch.dict does not return the patched object like mock.patch.object or mock.patch. It makes it not so intuitive

Do you plan on taking the changes? I'd be happy to help you out through the remaining bits if you want :)

Sorry for misleading, still have no time, will really appreciate your help

mariocj89 · 2018-11-07T19:17:23Z

I think I've addressed all comments in mariocj89@ff1fbe7

Let me know if you want me to send a new PR which closes this one (you still get all the credit in the commit history 😉 ) or whether you want to apply the patch locally and update this PR. I've rebased it to remove the merge commit that was in this PR.

If you want to apply locally, just get a new version of master and apply this patch.

We can then ask a core dev to review it 😄

mariocj89 · 2018-12-03T22:52:18Z

Hi @ts-taiye, this change would be neat! Every time I use patch.dict I wish this was merged :)

As said, let me know if you don't have time to push the changes you asked and I put in mariocj89/cpython@ff1fbe7 and I will send a different PR (still with the commits and patch in your name).

mariocj89 · 2018-12-10T09:18:59Z

@ts-taiye I've opened with your change: #11062

bpo-32299: unittest.patch.dict now returns patched dict when used as …

ed25324

…context manager

the-knights-who-say-ni added the CLA not signed label Dec 13, 2017

bedevere-bot added the awaiting review label Dec 13, 2017

asvetlov requested changes Dec 13, 2017

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Dec 13, 2017

taiye-ts and others added 2 commits December 13, 2017 18:02

bpo-32299: unittest.patch.dict now returns patched dict when used as …

6f0040f

…context manager

Update 2017-12-13-17-49-56.bpo-32299.eqAPWs.rst

b5ceb1a

asvetlov removed the CLA not signed label Dec 14, 2017

the-knights-who-say-ni added the CLA signed label Dec 14, 2017

asvetlov self-assigned this Dec 14, 2017

bpo-32299: unittest.patch.dict now returns patched dict when used as …

fe3ced7

…context manager

Merge branch 'fix-issue-32299' of https://github.com/ts-taiye/cpython …

396a6ee

…into fix-issue-32299

bpo-32299: unittest.patch.dict now returns patched dict when used as …

f00a266

…context manager

berkerpeksag requested changes Jan 23, 2018

View reviewed changes

mariocj89 mentioned this pull request Dec 10, 2018

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

Merged

csabella closed this in #11062 May 28, 2019

		@@ -0,0 +1 @@
		Changed ``mock.patch.dict`` to return patched dictionary when used as context manager

Uh oh!

bpo-32299: unittest.mock.patch.dict now returns patched dict when used as context manager #4834

bpo-32299: unittest.mock.patch.dict now returns patched dict when used as context manager #4834

Uh oh!

Conversation

ts-taiye commented Dec 13, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Dec 13, 2017

Uh oh!

ts-taiye commented Dec 13, 2017

Uh oh!

asvetlov left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Dec 13, 2017

Uh oh!

ts-taiye commented Dec 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asvetlov commented Dec 14, 2017

Uh oh!

ts-taiye commented Dec 15, 2017

Uh oh!

srinivasreddy commented Dec 26, 2017

Uh oh!

asvetlov commented Dec 26, 2017

Uh oh!

ts-taiye commented Dec 27, 2017

Uh oh!

asvetlov commented Dec 27, 2017

Uh oh!

ts-taiye commented Jan 4, 2018

Uh oh!

berkerpeksag Jan 23, 2018

Choose a reason for hiding this comment

Uh oh!

berkerpeksag Jan 23, 2018

Choose a reason for hiding this comment

Uh oh!

berkerpeksag Jan 23, 2018

Choose a reason for hiding this comment

Uh oh!

berkerpeksag Jan 23, 2018

Choose a reason for hiding this comment

Uh oh!

berkerpeksag Jan 23, 2018

Choose a reason for hiding this comment

Uh oh!

berkerpeksag Jan 23, 2018

Choose a reason for hiding this comment

Uh oh!

csabella commented Jan 23, 2018

Uh oh!

mariocj89 commented Oct 27, 2018

Uh oh!

ts-taiye commented Oct 28, 2018

Uh oh!

ts-taiye commented Nov 5, 2018

Uh oh!

mariocj89 commented Nov 7, 2018

Uh oh!

mariocj89 commented Dec 3, 2018

Uh oh!

mariocj89 commented Dec 10, 2018

Uh oh!

Uh oh!

ts-taiye commented Dec 13, 2017 •

edited by bedevere-bot

Loading

ts-taiye commented Dec 14, 2017 •

edited

Loading