From 902eea1b1d5c8652d9ac34dcce7a2cc08f3be76d Mon Sep 17 00:00:00 2001
From: Serhiy Storchaka <storchaka@gmail.com>
Date: Sat, 11 Apr 2020 10:59:24 +0300
Subject: [PATCH] bpo-40126: Fix reverting multiple patches in unittest.mock.
 (GH-19351)

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 <chris@simplistix.co.uk>
---
 .../2020-04-04-00-47-40.bpo-40126.Y-bTNP.rst  |  3 +
 mock/mock.py                                  | 74 +++++++------------
 mock/tests/testpatch.py                       |  2 +-
 3 files changed, 30 insertions(+), 49 deletions(-)
 create mode 100644 NEWS.d/2020-04-04-00-47-40.bpo-40126.Y-bTNP.rst

diff --git a/NEWS.d/2020-04-04-00-47-40.bpo-40126.Y-bTNP.rst b/NEWS.d/2020-04-04-00-47-40.bpo-40126.Y-bTNP.rst
new file mode 100644
index 00000000..8f725cfb
--- /dev/null
+++ b/NEWS.d/2020-04-04-00-47-40.bpo-40126.Y-bTNP.rst
@@ -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.
diff --git a/mock/mock.py b/mock/mock.py
index 47666723..523d61ef 100644
--- a/mock/mock.py
+++ b/mock/mock.py
@@ -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
@@ -1325,14 +1320,9 @@ 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:
@@ -1340,19 +1330,6 @@ def decoration_helper(self, patched, args, keywargs):
 
             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):
@@ -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:
@@ -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):
@@ -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)
 
 
 
@@ -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):
diff --git a/mock/tests/testpatch.py b/mock/tests/testpatch.py
index fbf4a537..070d7e81 100644
--- a/mock/tests/testpatch.py
+++ b/mock/tests/testpatch.py
@@ -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)