changeset:   90338:727b7e9c40e3
branch:      3.4
parent:      90333:9fe4d9686162
user:        Michael Foord <michael@voidspace.org.uk>
date:        Tue Apr 15 17:21:08 2014 -0400
files:       Lib/unittest/mock.py Lib/unittest/test/testmock/testpatch.py Misc/NEWS
description:
Closes issue 21239. unittest.mock.patch.stopall() did not work deterministically when the same name was patched multiple times.


diff -r 9fe4d9686162 -r 727b7e9c40e3 Lib/unittest/mock.py
--- a/Lib/unittest/mock.py	Tue Apr 15 16:36:43 2014 -0400
+++ b/Lib/unittest/mock.py	Tue Apr 15 17:21:08 2014 -0400
@@ -1050,7 +1050,7 @@
 class _patch(object):
 
     attribute_name = None
-    _active_patches = set()
+    _active_patches = []
 
     def __init__(
             self, getter, attribute, new, spec, create,
@@ -1323,13 +1323,18 @@
     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 _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 -r 9fe4d9686162 -r 727b7e9c40e3 Lib/unittest/test/testmock/testpatch.py
--- a/Lib/unittest/test/testmock/testpatch.py	Tue Apr 15 16:36:43 2014 -0400
+++ b/Lib/unittest/test/testmock/testpatch.py	Tue Apr 15 17:21:08 2014 -0400
@@ -12,7 +12,7 @@
 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 @@
         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 -r 9fe4d9686162 -r 727b7e9c40e3 Misc/NEWS
--- a/Misc/NEWS	Tue Apr 15 16:36:43 2014 -0400
+++ b/Misc/NEWS	Tue Apr 15 17:21:08 2014 -0400
@@ -37,6 +37,9 @@
 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.