Skip to content

Commit

Permalink
bpo-40126: Fix reverting multiple patches in unittest.mock. (GH-19351)
Browse files Browse the repository at this point in the history
Patcher's __exit__() is now never called if its __enter__() is failed.
Returning true from __exit__() silences now the exception.

Backports: 4b222c9491d1700e9bdd98e6889b8d0ea1c7321e
Signed-off-by: Chris Withers <[email protected]>
  • Loading branch information
serhiy-storchaka authored and cjw296 committed Dec 10, 2020
1 parent a7002f3 commit 902eea1
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 49 deletions.
3 changes: 3 additions & 0 deletions NEWS.d/2020-04-04-00-47-40.bpo-40126.Y-bTNP.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed reverting multiple patches in unittest.mock. Patcher's ``__exit__()``
is now never called if its ``__enter__()`` is failed. Returning true from
``__exit__()`` silences now the exception.
74 changes: 26 additions & 48 deletions mock/mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -1250,11 +1250,6 @@ def _importer(target):
return thing


def _is_started(patcher):
# XXXX horrible
return hasattr(patcher, 'is_local')


class _patch(object):

attribute_name = None
Expand Down Expand Up @@ -1325,34 +1320,16 @@ def decorate_class(self, klass):
@contextlib.contextmanager
def decoration_helper(self, patched, args, keywargs):
extra_args = []
entered_patchers = []
patching = None

exc_info = tuple()
try:
with contextlib.ExitStack() as exit_stack:
for patching in patched.patchings:
arg = patching.__enter__()
entered_patchers.append(patching)
arg = exit_stack.enter_context(patching)
if patching.attribute_name is not None:
keywargs.update(arg)
elif patching.new is DEFAULT:
extra_args.append(arg)

args += tuple(extra_args)
yield (args, keywargs)
except:
if (patching not in entered_patchers and
_is_started(patching)):
# the patcher may have been started, but an exception
# raised whilst entering one of its additional_patchers
entered_patchers.append(patching)
# Pass the exception to __exit__
exc_info = sys.exc_info()
# re-raise the exception
raise
finally:
for patching in reversed(entered_patchers):
patching.__exit__(*exc_info)


def decorate_callable(self, func):
Expand Down Expand Up @@ -1529,25 +1506,26 @@ def __enter__(self):

self.temp_original = original
self.is_local = local
setattr(self.target, self.attribute, new_attr)
if self.attribute_name is not None:
extra_args = {}
if self.new is DEFAULT:
extra_args[self.attribute_name] = new
for patching in self.additional_patchers:
arg = patching.__enter__()
if patching.new is DEFAULT:
extra_args.update(arg)
return extra_args

return new

self._exit_stack = contextlib.ExitStack()
try:
setattr(self.target, self.attribute, new_attr)
if self.attribute_name is not None:
extra_args = {}
if self.new is DEFAULT:
extra_args[self.attribute_name] = new
for patching in self.additional_patchers:
arg = self._exit_stack.enter_context(patching)
if patching.new is DEFAULT:
extra_args.update(arg)
return extra_args

return new
except:
if not self.__exit__(*sys.exc_info()):
raise

def __exit__(self, *exc_info):
"""Undo the patch."""
if not _is_started(self):
return

if self.is_local and self.temp_original is not DEFAULT:
setattr(self.target, self.attribute, self.temp_original)
else:
Expand All @@ -1562,9 +1540,9 @@ def __exit__(self, *exc_info):
del self.temp_original
del self.is_local
del self.target
for patcher in reversed(self.additional_patchers):
if _is_started(patcher):
patcher.__exit__(*exc_info)
exit_stack = self._exit_stack
del self._exit_stack
return exit_stack.__exit__(*exc_info)


def start(self):
Expand All @@ -1580,9 +1558,9 @@ def stop(self):
self._active_patches.remove(self)
except ValueError:
# If the patch hasn't been started this will fail
pass
return None

return self.__exit__()
return self.__exit__(None, None, None)



Expand Down Expand Up @@ -1882,9 +1860,9 @@ def stop(self):
_patch._active_patches.remove(self)
except ValueError:
# If the patch hasn't been started this will fail
pass
return None

return self.__exit__()
return self.__exit__(None, None, None)


def _clear_dict(in_dict):
Expand Down
2 changes: 1 addition & 1 deletion mock/tests/testpatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ def test_patch_dict_stop_without_start(self):
d = {'foo': 'bar'}
original = d.copy()
patcher = patch.dict(d, [('spam', 'eggs')], clear=True)
self.assertEqual(patcher.stop(), False)
self.assertFalse(patcher.stop())
self.assertEqual(d, original)


Expand Down

0 comments on commit 902eea1

Please sign in to comment.