From a15b2a2a161db1bfb78334e2577219635e11164a Mon Sep 17 00:00:00 2001
From: Alistair Coles <alistairncoles@gmail.com>
Date: Fri, 24 Mar 2017 10:41:17 +0000
Subject: [PATCH] Test that Manager.reload does stop/start in that order

The current test for reload does not actually verify the order in
which stop and start are called. Tighten that up to ensure stop is
called before start.

Change-Id: Iede6eb2049e3ab8f3810a69a6b77713de3c71399
Related-Change: I4bf57c8cdba6773ddc1e4013e2b2a9736dacada8
---
 test/unit/common/test_manager.py | 64 ++++++++++++--------------------
 1 file changed, 23 insertions(+), 41 deletions(-)

diff --git a/test/unit/common/test_manager.py b/test/unit/common/test_manager.py
index 78256282b5..7eac135cb6 100644
--- a/test/unit/common/test_manager.py
+++ b/test/unit/common/test_manager.py
@@ -16,6 +16,7 @@ from __future__ import print_function
 import unittest
 from test.unit import temptree
 
+import mock
 import os
 import sys
 import resource
@@ -2079,57 +2080,38 @@ class TestManager(unittest.TestCase):
         self.assertEqual(m.start_was_called, True)
 
     def test_reload(self):
-        class MockManager(object):
+        def do_test(graceful):
             called = defaultdict(list)
 
-            def __init__(self, servers):
-                pass
-
-            @classmethod
-            def reset_called(cls):
-                cls.called = defaultdict(list)
-
             def stop(self, **kwargs):
-                MockManager.called['stop'].append(kwargs)
+                called[self].append(('stop', kwargs))
                 return 0
 
             def start(self, **kwargs):
-                MockManager.called['start'].append(kwargs)
+                called[self].append(('start', kwargs))
                 return 0
 
-        _orig_manager = manager.Manager
-        try:
-            m = _orig_manager(['*-server'])
+            m = manager.Manager(['*-server'])
             self.assertEqual(len(m.servers), 4)
-            for server in m.servers:
-                self.assertTrue(server.server in
-                                manager.GRACEFUL_SHUTDOWN_SERVERS)
-            manager.Manager = MockManager
-            status = m.reload()
-            self.assertEqual(status, 0)
-            expected = {
-                'start': [{'graceful': True}] * 4,
-                'stop': [{'graceful': True}] * 4,
-            }
-            self.assertEqual(MockManager.called, expected)
-            # test force graceful
-            MockManager.reset_called()
-            m = _orig_manager(['*-server'])
-            self.assertEqual(len(m.servers), 4)
-            for server in m.servers:
-                self.assertTrue(server.server in
-                                manager.GRACEFUL_SHUTDOWN_SERVERS)
-            manager.Manager = MockManager
-            status = m.reload(graceful=False)
-            self.assertEqual(status, 0)
-            expected = {
-                'start': [{'graceful': True}] * 4,
-                'stop': [{'graceful': True}] * 4,
-            }
-            self.assertEqual(MockManager.called, expected)
+            expected_servers = set([server.server for server in m.servers])
+            for server in expected_servers:
+                self.assertIn(server, manager.GRACEFUL_SHUTDOWN_SERVERS)
 
-        finally:
-            manager.Manager = _orig_manager
+            with mock.patch('swift.common.manager.Manager.start', start):
+                with mock.patch('swift.common.manager.Manager.stop', stop):
+                    status = m.reload(graceful=graceful)
+
+            self.assertEqual(status, 0)
+            self.assertEqual(4, len(called))
+            actual_servers = set()
+            for m, calls in called.items():
+                self.assertEqual(calls, [('stop', {'graceful': True}),
+                                         ('start', {'graceful': True})])
+                actual_servers.update([server.server for server in m.servers])
+            self.assertEqual(expected_servers, actual_servers)
+
+        do_test(graceful=True)
+        do_test(graceful=False)  # graceful is forced regardless of the kwarg
 
     def test_force_reload(self):
         m = manager.Manager(['test'])