Unit tests: Fix mock errors with too few side effects

When a mock hasn't been provided with a long-enough list of side-effects
for the number of times it is called, it raises StopIteration (instead
of something sensible like AssertionError). Prior to Python 3.7, this
exception could bubble up until it just stopped a task. (In Python 3.7
it will eventually be caught and turned into a RuntimeError.)

To ensure that any errors of this sort are not handled silently, and to
enable us to test for them prior to using Python 3.7, catch any
StopIteration errors coming from plugin-provided non-generator functions
and convert them to RuntimeError exceptions.

This reveals many errors in the unit tests, which are also fixed by this
patch. This backport contains fewer fixes than master, because many of
the errors were introduced in the process of moving from mox to mock in
the Rocky release cycle.

Change-Id: I5a1eff6b704dff7c17edcbbe58cdbc380ae6abc9
Story: #2003412
Task: 24553
(cherry picked from commit 38fad07c0a01072ee4e17815f6ad0e5c2ce1f64a)

4 files changed, 76 insertions, 76 deletions

diff --git a/heat/engine/resource.py b/heat/engine/resource.py
index 6e46bbbc3..24c7bf369 100644
--- a/heat/engine/resource.py
+++ b/heat/engine/resource.py
@@ -837,7 +837,10 @@ class Resource(object):
         handler = getattr(self, 'handle_%s' % handler_action, None)
 
         if callable(handler):
-            handler_data = handler(*args)
+            try:
+                handler_data = handler(*args)
+            except StopIteration:
+                raise RuntimeError('Plugin method raised StopIteration')
             yield
             if callable(check):
                 try:
@@ -851,6 +854,8 @@ class Resource(object):
                                 break
                             else:
                                 yield
+                except StopIteration:
+                    raise RuntimeError('Plugin method raised StopIteration')
                 except Exception:
                     raise
                 except:  # noqa
diff --git a/heat/tests/openstack/heat/test_swiftsignal.py b/heat/tests/openstack/heat/test_swiftsignal.py
index 0dc57451a..0ca52ba44 100644
--- a/heat/tests/openstack/heat/test_swiftsignal.py
+++ b/heat/tests/openstack/heat/test_swiftsignal.py
@@ -910,9 +910,10 @@ class SwiftSignalTest(common.HeatTestCase):
         mock_name.return_value = obj_name
         mock_swift_object.get_container.return_value = cont_index(obj_name, 2)
         mock_swift_object.get_object.side_effect = (
-            (obj_header, ''),
             swiftclient_client.ClientException(
-                "Object %s not found" % obj_name, http_status=404)
+                "Object %s not found" % obj_name, http_status=404),
+            (obj_header, '{"id": 1}'),
+            (obj_header, '{"id": 2}'),
         )
 
         st.create()
diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py
index af730d60b..3b731ac6e 100644
--- a/heat/tests/openstack/nova/test_server.py
+++ b/heat/tests/openstack/nova/test_server.py
@@ -216,6 +216,19 @@ resources:
 """
 
 
+class ServerStatus(object):
+    def __init__(self, server, statuses):
+        self._server = server
+        self._status = iter(statuses)
+
+    def __call__(self, server_id):
+        try:
+            self._server.status = next(self._status)
+        except StopIteration:
+            raise AssertionError('Unexpected call to servers.get()')
+        return self._server
+
+
 class ServersTest(common.HeatTestCase):
     def setUp(self):
         super(ServersTest, self).setUp()
@@ -1537,12 +1550,12 @@ class ServersTest(common.HeatTestCase):
         # this makes sure the auto increment worked on server creation
         self.assertGreater(server.id, 0)
 
-        def make_soft_delete(*args):
-            return_server.status = "SOFT_DELETED"
-            return return_server
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[return_server, return_server,
-                                      make_soft_delete()])
+                         side_effect=ServerStatus(return_server,
+                                                  [return_server.status,
+                                                   return_server.status,
+                                                   "SOFT_DELETED",
+                                                   "DELETED"]))
         scheduler.TaskRunner(server.delete)()
         self.assertEqual((server.DELETE, server.COMPLETE), server.state)
 
@@ -1934,23 +1947,22 @@ class ServersTest(common.HeatTestCase):
         update_props['flavor'] = 'm1.small'
         update_template = server.t.freeze(properties=update_props)
 
-        def set_status(status):
-            return_server.status = status
-            return return_server
-
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[set_status('ACTIVE'),
-                                      set_status('RESIZE'),
-                                      set_status('VERIFY_RESIZE'),
-                                      set_status('VERIFY_RESIZE'),
-                                      set_status('ACTIVE')])
+                         side_effect=ServerStatus(return_server,
+                                                  ['ACTIVE',
+                                                   'RESIZE',
+                                                   'VERIFY_RESIZE',
+                                                   'VERIFY_RESIZE',
+                                                   'ACTIVE']))
         mock_post = self.patchobject(self.fc.client,
                                      'post_servers_1234_action',
                                      return_value=(202, None))
         scheduler.TaskRunner(server.update, update_template)()
         self.assertEqual((server.UPDATE, server.COMPLETE), server.state)
-        mock_post.called_once_with(body={'resize': {'flavorRef': 2}})
-        mock_post.called_once_with(body={'confirmResize': None})
+        mock_post.assert_has_calls([
+            mock.call(body={'resize': {'flavorRef': '2'}}),
+            mock.call(body={'confirmResize': None}),
+        ])
 
     def test_server_update_server_flavor_failed(self):
         """Check raising exception due to resize call failing.
@@ -1966,13 +1978,9 @@ class ServersTest(common.HeatTestCase):
         update_props['flavor'] = 'm1.small'
         update_template = server.t.freeze(properties=update_props)
 
-        def set_status(status):
-            return_server.status = status
-            return return_server
-
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[set_status('RESIZE'),
-                                      set_status('ERROR')])
+                         side_effect=ServerStatus(return_server,
+                                                  ['RESIZE', 'ERROR']))
         mock_post = self.patchobject(self.fc.client,
                                      'post_servers_1234_action',
                                      return_value=(202, None))
@@ -1982,7 +1990,7 @@ class ServersTest(common.HeatTestCase):
             "Error: resources.srv_update2: Resizing to '2' failed, "
             "status 'ERROR'", six.text_type(error))
         self.assertEqual((server.UPDATE, server.FAILED), server.state)
-        mock_post.called_once_with(body={'resize': {'flavorRef': 2}})
+        mock_post.assert_called_once_with(body={'resize': {'flavorRef': '2'}})
 
     def test_server_update_flavor_resize_has_not_started(self):
         """Test update of server flavor if server resize has not started.
@@ -2005,17 +2013,14 @@ class ServersTest(common.HeatTestCase):
         # define status transition when server resize
         # ACTIVE(initial) -> ACTIVE -> RESIZE -> VERIFY_RESIZE
 
-        def set_status(status):
-            server.status = status
-            return server
-
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[set_status('ACTIVE'),
-                                      set_status('ACTIVE'),
-                                      set_status('RESIZE'),
-                                      set_status('VERIFY_RESIZE'),
-                                      set_status('VERIFY_RESIZE'),
-                                      set_status('ACTIVE')])
+                         side_effect=ServerStatus(server,
+                                                  ['ACTIVE',
+                                                   'ACTIVE',
+                                                   'RESIZE',
+                                                   'VERIFY_RESIZE',
+                                                   'VERIFY_RESIZE',
+                                                   'ACTIVE']))
 
         mock_post = self.patchobject(self.fc.client,
                                      'post_servers_1234_action',
@@ -2024,8 +2029,10 @@ class ServersTest(common.HeatTestCase):
         scheduler.TaskRunner(server_resource.update, update_template)()
         self.assertEqual((server_resource.UPDATE, server_resource.COMPLETE),
                          server_resource.state)
-        mock_post.called_once_with(body={'resize': {'flavorRef': 2}})
-        mock_post.called_once_with(body={'confirmResize': None})
+        mock_post.assert_has_calls([
+            mock.call(body={'resize': {'flavorRef': '2'}}),
+            mock.call(body={'confirmResize': None}),
+        ])
 
     @mock.patch.object(servers.Server, 'prepare_for_replace')
     def test_server_update_server_flavor_replace(self, mock_replace):
@@ -2228,13 +2235,9 @@ class ServersTest(common.HeatTestCase):
         server.reparse()
         mock_rebuild = self.patchobject(self.fc.servers, 'rebuild')
 
-        def set_status(status):
-            return_server.status = status
-            return return_server
-
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[set_status('REBUILD'),
-                                      set_status('ERROR')])
+                         side_effect=ServerStatus(return_server,
+                                                  ['REBUILD', 'ERROR']))
         updater = scheduler.TaskRunner(server.update, update_template)
         error = self.assertRaises(exception.ResourceFailure, updater)
         self.assertEqual(
@@ -2307,15 +2310,12 @@ class ServersTest(common.HeatTestCase):
         server.resource_id = '1234'
         server.state_set(state[0], state[1])
 
-        def set_status(status):
-            return_server.status = status
-            return return_server
-
         self.patchobject(return_server, 'suspend')
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[set_status('ACTIVE'),
-                                      set_status('ACTIVE'),
-                                      set_status('SUSPENDED')])
+                         side_effect=ServerStatus(return_server,
+                                                  ['ACTIVE',
+                                                   'ACTIVE',
+                                                   'SUSPENDED']))
 
         scheduler.TaskRunner(server.suspend)()
         self.assertEqual((server.SUSPEND, server.COMPLETE), server.state)
@@ -2339,15 +2339,12 @@ class ServersTest(common.HeatTestCase):
                                           'srv_susp_uk')
         server.resource_id = '1234'
 
-        def set_status(status):
-            return_server.status = status
-            return return_server
-
         self.patchobject(return_server, 'suspend')
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[set_status('ACTIVE'),
-                                      set_status('ACTIVE'),
-                                      set_status('TRANSMOGRIFIED')])
+                         side_effect=ServerStatus(return_server,
+                                                  ['ACTIVE',
+                                                   'ACTIVE',
+                                                   'TRANSMOGRIFIED']))
         ex = self.assertRaises(exception.ResourceFailure,
                                scheduler.TaskRunner(server.suspend))
         self.assertIsInstance(ex.exc, exception.ResourceUnknownStatus)
@@ -2364,15 +2361,12 @@ class ServersTest(common.HeatTestCase):
         server.resource_id = '1234'
         server.state_set(state[0], state[1])
 
-        def set_status(status):
-            return_server.status = status
-            return return_server
-
         self.patchobject(return_server, 'resume')
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[set_status('SUSPENDED'),
-                                      set_status('SUSPENDED'),
-                                      set_status('ACTIVE')])
+                         side_effect=ServerStatus(return_server,
+                                                  ['SUSPENDED',
+                                                   'SUSPENDED',
+                                                   'ACTIVE']))
 
         scheduler.TaskRunner(server.resume)()
         self.assertEqual((server.RESUME, server.COMPLETE), server.state)
@@ -2460,13 +2454,10 @@ class ServersTest(common.HeatTestCase):
                                          'srv_sts_bld')
         server.resource_id = '1234'
 
-        def set_status(status):
-            return_server.status = status
-            return return_server
-
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[set_status(uncommon_status),
-                                      set_status('ACTIVE')])
+                         side_effect=ServerStatus(return_server,
+                                                  [uncommon_status,
+                                                   'ACTIVE']))
 
         scheduler.TaskRunner(server.create)()
         self.assertEqual((server.CREATE, server.COMPLETE), server.state)
@@ -3793,10 +3784,12 @@ class ServersTest(common.HeatTestCase):
         mock_create = self.patchobject(self.fc.servers, 'create',
                                        return_value=return_server)
         self.patchobject(self.fc.servers, 'get',
-                         side_effect=[return_server, None])
+                         return_value=return_server)
         self.patchobject(neutron.NeutronClientPlugin,
                          'find_resourceid_by_name_or_id',
                          return_value='aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa')
+        self.patchobject(return_server, 'get', return_value=None)
+
         scheduler.TaskRunner(stack.create)()
         self.assertEqual(1, mock_create.call_count)
         self.assertEqual((stack.CREATE, stack.COMPLETE), stack.state)
diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py
index c84d0d3b8..f76a4c55f 100644
--- a/heat/tests/test_resource.py
+++ b/heat/tests/test_resource.py
@@ -972,10 +972,8 @@ class ResourceTest(common.HeatTestCase):
                                             {'Foo': 'abc'})
         res = generic_rsrc.ResourceWithProps('test_resource', tmpl, self.stack)
 
-        generic_rsrc.ResourceWithProps.handle_create = mock.Mock()
-        generic_rsrc.ResourceWithProps.handle_delete = mock.Mock()
         m_v.side_effect = [True, exception.StackValidationFailed()]
-        generic_rsrc.ResourceWithProps.handle_create.side_effect = [
+        create_excs = [
             exception.ResourceInError(resource_name='test_resource',
                                       resource_status='ERROR',
                                       resource_type='GenericResourceType',
@@ -988,8 +986,11 @@ class ResourceTest(common.HeatTestCase):
                                       status_reason='just because'),
             None
         ]
+        self.patchobject(generic_rsrc.ResourceWithProps, 'handle_create',
+                         side_effect=create_excs)
 
-        generic_rsrc.ResourceWithProps.handle_delete.return_value = None
+        self.patchobject(generic_rsrc.ResourceWithProps, 'handle_delete',
+                         return_value=None)
         m_re.return_value = 0.01
         scheduler.TaskRunner(res.create)()
         self.assertEqual(2, m_re.call_count)