Repository navigation
patch.dict resolves in_dict eagerly (should be late resolved) #79693
Description
Activity
Originally reported in testing-cabal/mock#405, I believe I've discovered an inconsistency that manifests as a flaw:
patchandpatch.objectallow the target to be specified as string referring to the target object and this object is resolved at the time the patch effected, not when the patch is declared.patch.dictcontrarily seems to resolve the dict eagerly, when the patch is declared. Observe with this pytest:import mock @mock.patch.dict('test_patch_dict.target', dict(b=2)) def test_after_patch(): assert dict(a=2, b=2)Here's the output:
$ rwt mock pytest -- -m pytest test_patch_dict.py Collecting mock Using cached mock-2.0.0-py2.py3-none-any.whl Collecting pbr>=0.11 (from mock) Using cached pbr-3.0.0-py2.py3-none-any.whl Collecting six>=1.9 (from mock) Using cached six-1.10.0-py2.py3-none-any.whl Installing collected packages: pbr, six, mock Successfully installed mock-2.0.0 pbr-3.0.0 six-1.10.0 ====================================== test session starts ======================================= platform darwin -- Python 3.6.1, pytest-3.0.5, py-1.4.33, pluggy-0.4.0 rootdir: /Users/jaraco, inifile: collected 1 items test_patch_dict.py F ============================================ FAILURES ============================================ ________________________________________ test_after_patch ________________________________________ @mock.patch.dict('test_patch_dict.target', dict(b=2)) def test_after_patch(): > assert dict(a=2, b=2) E assert {'a': 2} == {'a': 2, 'b': 2} E Omitting 1 identical items, use -v to show E Right contains more items: E {'b': 2} E Use -v to get the full diff test_patch_dict.py:8: AssertionError ==================================== 1 failed in 0.05 seconds ====================================The target is unpatched because
test_patch_dict.targetwas resolved during decoration rather than during test run.Removing the initial assignment of
target = dict(a=1), the failure is thus:______________________________ ERROR collecting test_patch_dict.py _______________________________ ImportError while importing test module '/Users/jaraco/test_patch_dict.py'. Hint: make sure your test modules/packages have valid Python names. Traceback: /var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/rwt-pcm3552g/mock/mock.py:1197: in _dot_lookup return getattr(thing, comp) E AttributeError: module 'test_patch_dict' has no attribute 'target' During handling of the above exception, another exception occurred: <frozen importlib._bootstrap>:942: in _find_and_load_unlocked ??? E AttributeError: module 'test_patch_dict' has no attribute '__path__' During handling of the above exception, another exception occurred: test_patch_dict.py:4: in <module> @mock.patch.dict('test_patch_dict.target', dict(b=2)) /var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/rwt-pcm3552g/mock/mock.py:1708: in __init__ in_dict = _importer(in_dict) /var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/rwt-pcm3552g/mock/mock.py:1210: in _importer thing = _dot_lookup(thing, comp, import_path) /var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/rwt-pcm3552g/mock/mock.py:1199: in _dot_lookup __import__(import_path) E ModuleNotFoundError: No module named 'test_patch_dict.target'; 'test_patch_dict' is not a package !!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!! ==================================== 1 error in 0.41 seconds =====================================Is there any reason
patch.dictdoesn't have a similar deferred resolution behavior as its sister methods?If I understand the issue correctly it's as below is a simple reproducer where target is resolved with {'a': 1} during the construction of the decorator [0] though it's redefined later in the program as Also here due to this since target is reassigned the decorator just stores a reference to {'a': 1} and updates it with {'b': 2} leaving the reference to dict object {'a': 2} unpatched in test_with_decorator. Meanwhile in case of the context manager (test_with_context_manager) it's created and resolved at the time it's executed hence updating the object {'a': 2} correctly. A possible fix would be to store the reference to the string path of the patch '__main__.target' and try it again with importer function. I will take a further look into this. It would be helpful if you can confirm this reproducer is good enough and resembles the original report.
from unittest import mock target = dict(a=1)@mock.patch.dict('__main__.target', dict(b=2))
def test_with_decorator():
print(f"target inside decorator : {target}")def test_with_context_manager(): with mock.patch.dict('__main__.target', dict(b=2)): print(f"target inside context : {target}") target = dict(a=2) test_with_decorator() test_with_context_manager()
$ python3 test_foo.py target inside decorator : {'a': 2} target inside context : {'a': 2, 'b': 2}
[0]
Line 1624 in 3208880
I agree that’s a good reproducer. Thanks for looking into this!
Looking further this can be solved for a string target in patch.dict which can be resolved again while calling the decorated function. There could be a case where the actual target is specified and in that case mock could only updates the reference and cannot track if the variable has been redefined to reference a different dict object. In the below case also it's resolved with {'a': 1} in the decorator and later redefining target to {'a': 2} whose reference is not updated. I can propose a PR for string target but I am not sure if this case can be solved or it's expected. This seems to be not a problem with patch.object where redefining a class later like dict seems to work correctly and maybe it's due to creating a new class itself that updates the local to reference new class?
Any thoughts would be helpful.
# script with dict target passed
from unittest import mock target = dict(a=1) @mock.patch.dict(target, dict(b=2)) def test_with_decorator(): print(f"target inside decorator : {target}") def test_with_context_manager(): with mock.patch.dict(target, dict(b=2)): print(f"target inside context : {target}") target = dict(a=2) test_with_decorator() test_with_context_manager()
$ ./python.exe test_foo.py target inside decorator : {'a': 2} target inside context : {'a': 2, 'b': 2}
mariocj89 commented
on Feb 23, 2019 mariocj89mannequinMannequinInteresting,
patchdoes resolve it when the patched function is called (see) vs patch.dict that resolves it at the time the patcher is created - when decorating - (seeLine 1269 in 175421b
).Line 1624 in 175421b
An option might be to delay the resolution as done for patch, changing
toLine 1625 in 175421b
self.in_dict_name = in_dictExample untested patch:
diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 8f46050462..5328fda417 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -1620,9 +1620,7 @@ class _patch_dict(object): """ def __init__(self, in_dict, values=(), clear=False, **kwargs): - if isinstance(in_dict, str): - in_dict = _importer(in_dict) - self.in_dict = in_dict + self.in_dict_name = in_dict # support any argument supported by dict(...) constructor self.values = dict(values) self.values.update(kwargs) @@ -1649,7 +1647,7 @@ class _patch_dict(object): attr_value = getattr(klass, attr) if (attr.startswith(patch.TEST_PREFIX) and hasattr(attr_value, "__call__")): - decorator = _patch_dict(self.in_dict, self.values, self.clear) + decorator = _patch_dict(self.in_dict_name, self.values, self.clear) decorated = decorator(attr_value) setattr(klass, attr, decorated) return klass @@ -1662,7 +1660,11 @@ class _patch_dict(object): def _patch_dict(self): values = self.values - in_dict = self.in_dict + if isinstance(self.in_dict_name, str): + in_dict = _importer(self.in_dict_name) + else: + in_dict = self.in_dict_name + self.in_dict = in_dictThis seems to be not a problem with patch.object where redefining a class later like dict seems to work correctly and maybe it's due to creating a new class itself that updates the local to reference new class?
For patch, when you create a new class, the new one is patched as the name is resolved at the time the decorated function is executed, not when it is decorated. See:
$ cat t.py from unittest import mock import c @mock.patch("c.A", "target", "updated") def test_with_decorator(): print(f"target inside decorator : {A.target}") def test_with_context_manager(): with mock.patch("c.A", "target", "updated"): print(f"target inside context : {A.target}") class A: c.A = A test_with_decorator() test_with_context_manager() xarmariocj89 at DESKTOP-9B6VH3A in ~/workspace/cpython on master* $ cat c.py class A: mariocj89 at DESKTOP-9B6VH3A in ~/workspace/cpython on master* $ ./python ./t.py target inside decorator : changed target inside context : changedIf
patchwas implemented likepatch.dict, you would see the first as "changed" as the reference toc.Awould have been resolved when the decorator was run (before the re-definition ofA).About
patch.object, it cannot be compared, as it grabs the name at the time you execute the decorator because you are not passing a string, but the actual object to patch.Thanks Mario for the details. I had almost the same patch while writing msg336300 :) There were no test case failures except that I had resolved it in the constructor storing the string representation as a separate variable and also while calling the function which could be avoided as per your approach to just store the string and resolve once during function call. I think this is a good way to do this. Are you planning to create a PR or shall I go ahead with this?
mariocj89 commented
on Feb 23, 2019 mariocj89mannequinMannequinGreat, all yours :) I'll be happy to review.
Confirmed the fix. Thank you very much!
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: