[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Change by Raymond Hettinger : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed versions: +Python 3.9 -Python 3.7, Python 3.8 ___ Python tracker _

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Chris Withers added the comment: New changeset 1d0c5e16eab29d55773cc4196bb90d2bf12e09dd by Chris Withers (Emmanuel Arias) in branch 'master': bpo-24928: Add test case for patch.dict using OrderedDict (GH -11437) https://github.com/python/cpython/commit/1d0c5e16eab29d55773cc4196bb90d2bf12e09dd

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Chris Withers added the comment: As I said before, I can't see an additional test like this hurting, especially if it highlights problems with earlier python versions when it's backported. -- ___ Python tracker

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Emmanuel Arias added the comment: Hi Berker, Thanks for you response. I agree when you say that patch.dict is a simply operation, but I think that this test can be necessary, if for some reason the implementation of patch.dict (or its parts) decide change. > I think the relationship bet

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Change by Berker Peksag : -- pull_requests: -10872 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://m

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Berker Peksag added the comment: While I agree having more tests are a good thing, I'm not sure if the test in PR 11437 should be merged as it's not specifically testing a feature of the mock module. patch.dict() basically does the following operation (ignoring possible AttributeErrors):

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Emmanuel Arias added the comment: Ping :-) Thanks Karthikeyan for the PR review. Would someone else like review it, please? Thanks! Regards -- ___ Python tracker ___ _

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Change by Emmanuel Arias : -- pull_requests: +10871, 10872, 10873 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscri

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Change by Emmanuel Arias : -- pull_requests: +10871, 10872 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: ht

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Change by Emmanuel Arias : -- pull_requests: +10871 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://m

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Karthikeyan Singaravelan added the comment: No problem :) I think the test can use a context manager instead of using test() and a decorator but that can be discussed on the PR. Thanks! -- ___ Python tracker _

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Emmanuel Arias added the comment: Sorry I was wrong. On ```python foo = OrderedDict() foo['a'] = object() foo['b'] = 'something' ``` I was write "first" and "second" like key and fail in ```python @patch.dict(foo, OrderedDict(update_values)) def test(): self.assertEqual(list(foo), sort

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Karthikeyan Singaravelan added the comment: Can you please post the error and the command to run the test? On applying the patch on master I cannot see any errors with below commands : # Applying the patch with only test $ wget https://bugs.python.org/file40488/issue24928.patch $ git apply

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Emmanuel Arias added the comment: Hi xtreak, > Thanks @nekobon for the patch. I am triaging old mock related issues. I think > dict insertion order is maintained from 3.6 and guaranteed with 3.7 and > above. But it would be good to add the unit test in the patch as a PR. I ran > the test on

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Vaibhav Gupta added the comment: Hi Emmanuel Please go ahead and make a PR. :) -- ___ Python tracker ___ ___ Python-bugs-list mail

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Emmanuel Arias added the comment: ping Vaibhav :-) > I would like to make a PR for this. -- nosy: +eamanu ___ Python tracker ___ _

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Karthikeyan Singaravelan added the comment: Hi Vaibhav, As noted in the thread the issue is fixed in 3.6 and above due to dict order being guaranteed. But it would be nice to have the test in the patch converted as a unit test. With respect to backport the fixes are backported to https://gi

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Vaibhav Gupta added the comment: Hi. I would like to make a PR for this. Also, I am not very familiar with the process of backporting. Is something specific needs to be done for that which is related to this? -- nosy: +dojutsu-user ___ Python track

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Karthikeyan Singaravelan added the comment: Thanks Mario, I will convert the unit test as a PR before closing the issue since I feel the test is a good one for inclusion and can help if dict order guarantee is changed in future. I will raise a backport PR to cabal's mock repo where the fix w

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Mario Corchero added the comment: I would suggest applying the fix with the latest version in mind to keep the codebase clean. If you want to backport it to previous versions of Python you can do it manually through a PR targetting the right branch. I think the process is to set up a label an

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Karthikeyan Singaravelan added the comment: Thinking about it further the attached test is based on the ordering of dict and hence works only on 3.6 and above. But this test will be backported to mock on PyPI where this will fail on 2.7 and 3.4, 3.5. Is it okay to apply the fix that makes ba

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Chris Withers added the comment: More tests are generally a good thing, so go for it :-) -- ___ Python tracker ___ ___ Python-bugs-

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Karthikeyan Singaravelan added the comment: Thanks @nekobon for the patch. I am triaging old mock related issues. I think dict insertion order is maintained from 3.6 and guaranteed with 3.7 and above. But it would be good to add the unit test in the patch as a PR. I ran the test on master an

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Michael Foord added the comment: Patch looks good to me. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: htt

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Changes by Raymond Hettinger : -- assignee: -> michael.foord ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: h

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Changes by Serhiy Storchaka : -- keywords: +needs review nosy: +michael.foord stage: needs patch -> patch review ___ Python tracker ___ __

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Yu Tomita added the comment: Submitting a patch. To support both iterable and mapping in the same way as with dict(...), `values` is updated to be a list of length-2 iterables instead of using copy call. Patch includes unittest which tests the reported problem. -- keywords: +patch n

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Changes by Adam : -- nosy: +azsorkin ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/ma

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Changes by R. David Murray : -- keywords: +easy stage: -> needs patch versions: +Python 3.5, Python 3.6 -Python 2.7 ___ Python tracker ___ __

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Raymond Hettinger added the comment: > Perhaps replacing the dict call with a copy call would be sufficient? I think that would do it. -- nosy: +rhettinger ___ Python tracker __

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

R. David Murray added the comment: Based on reading the patch.dict doct, I'm guessing that that dict call is making a copy in order to do a restore later. Perhaps replacing the dict call with a copy call would be sufficient? (I haven't looked at the dict.patch code). -- nosy: +r.david

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

Alexander Oblovatniy added the comment: Hi! Current implementation of `patch.dict` spoils order of items in `collections.OrderedDict`, because it explicitly converts passed `values` to `dict` (https://github.com/python/cpython/blob/923527f560acd43d4cc11293060900e56c7cb39b/Lib/unittest/mock.py

[issue24928] mock.patch.dict spoils order of items in collections.OrderedDict

New submission from Alexander Oblovatniy: Hi! Current implementation of `patch.dict` spoils order of items in `collections.OrderedDict`, because it explicitly converts passed `values` to `dict` (https://github.com/python/cpython/blob/923527f560acd43d4cc11293060900e56c7cb39b/Lib/unittest/mock.