Closes issue 21239. unittest.mock.patch.stopall() did not work deterministically when the same name was patched multiple times.

3 files changed, 31 insertions, 6 deletions

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 13fd7b1..5555774 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -1050,7 +1050,7 @@ def _is_started(patcher):
 class _patch(object):
 
     attribute_name = None
-    _active_patches = set()
+    _active_patches = []
 
     def __init__(
             self, getter, attribute, new, spec, create,
@@ -1323,13 +1323,18 @@ class _patch(object):
     def start(self):
         """Activate a patch, returning any created mock."""
         result = self.__enter__()
-        self._active_patches.add(self)
+        self._active_patches.append(self)
         return result
 
 
     def stop(self):
         """Stop an active patch."""
-        self._active_patches.discard(self)
+        try:
+            self._active_patches.remove(self)
+        except ValueError:
+            # If the patch hasn't been started this will fail
+            pass
+
         return self.__exit__()
 
 
@@ -1622,8 +1627,8 @@ def _clear_dict(in_dict):
 
 
 def _patch_stopall():
-    """Stop all active patches."""
-    for patch in list(_patch._active_patches):
+    """Stop all active patches. LIFO to unroll nested patches."""
+    for patch in reversed(_patch._active_patches):
         patch.stop()
 
 
diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py
index c1bc34f..b516f42 100644
--- a/Lib/unittest/test/testmock/testpatch.py
+++ b/Lib/unittest/test/testmock/testpatch.py
@@ -12,7 +12,7 @@ from unittest.test.testmock.support import SomeClass, is_instance
 from unittest.mock import (
     NonCallableMock, CallableMixin, patch, sentinel,
     MagicMock, Mock, NonCallableMagicMock, patch, _patch,
-    DEFAULT, call, _get_target
+    DEFAULT, call, _get_target, _patch
 )
 
 
@@ -1779,6 +1779,23 @@ class PatchTest(unittest.TestCase):
         patched()
         self.assertIs(os.path, path)
 
+    def test_stopall_lifo(self):
+        stopped = []
+        class thing(object):
+            one = two = three = None
+
+        def get_patch(attribute):
+            class mypatch(_patch):
+                def stop(self):
+                    stopped.append(attribute)
+                    return super(mypatch, self).stop()
+            return mypatch(lambda: thing, attribute, None, None,
+                           False, None, None, None, {})
+        [get_patch(val).start() for val in ("one", "two", "three")]
+        patch.stopall()
+
+        self.assertEqual(stopped, ["three", "two", "one"])
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/Misc/NEWS b/Misc/NEWS
index 0441074..1c4a803 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -37,6 +37,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #21239: patch.stopall() didn't work deterministically when the same
+  name was patched more than once.
+
 - Issue #21222: Passing name keyword argument to mock.create_autospec now
   works.