openstack
/
heat
Code Issues Proposed changes

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, many introduced in the
process of converting from mox to mock, which are also fixed by this
patch.

Change-Id: I5a1eff6b704dff7c17edcbbe58cdbc380ae6abc9
Story: #2003412
Task: 24553
This commit is contained in:
Zane Bitter 2018-08-07 16:38:03 -04:00
parent bcd430cf4d
commit 38fad07c0a
9 changed files with 143 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
heat/engine/resource.py
View File

@ -976,7 +976,10 @@ class Resource(status.ResourceStatus):
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:
@ -990,6 +993,8 @@ class Resource(status.ResourceStatus):
break
else:
yield
except StopIteration:
raise RuntimeError('Plugin method raised StopIteration')
except Exception:
raise
except: # noqa

52
heat/tests/aws/test_security_group.py
View File

@ -463,6 +463,33 @@ Resources:
# delete old not needed rules
self.stubout_neutron_get_security_group()
stack = self.create_stack(self.test_template_neutron)
sg = stack['the_sg']
self.assertResourceState(sg, 'aaaa')
# make updated template
props = copy.deepcopy(sg.properties.data)
props['SecurityGroupIngress'] = [
{'IpProtocol': 'tcp',
'FromPort': '80',
'ToPort': '80',
'CidrIp': '0.0.0.0/0'},
{'IpProtocol': 'tcp',
'FromPort': '443',
'ToPort': '443',
'CidrIp': '0.0.0.0/0'},
{'IpProtocol': 'tcp',
'SourceSecurityGroupId': 'zzzz'},
]
props['SecurityGroupEgress'] = [
{'IpProtocol': 'tcp',
'FromPort': '22',
'ToPort': '22',
'CidrIp': '0.0.0.0/0'},
{'SourceSecurityGroupName': 'xxxx'},
]
after = rsrc_defn.ResourceDefinition(sg.name, sg.type(), props)
# create missing rules
self.m_csgr.side_effect = [
{
@ -501,32 +528,7 @@ Resources:
}
}
]
stack = self.create_stack(self.test_template_neutron)
sg = stack['the_sg']
self.assertResourceState(sg, 'aaaa')
# make updated template
props = copy.deepcopy(sg.properties.data)
props['SecurityGroupIngress'] = [
{'IpProtocol': 'tcp',
'FromPort': '80',
'ToPort': '80',
'CidrIp': '0.0.0.0/0'},
{'IpProtocol': 'tcp',
'FromPort': '443',
'ToPort': '443',
'CidrIp': '0.0.0.0/0'},
{'IpProtocol': 'tcp',
'SourceSecurityGroupId': 'zzzz'},
]
props['SecurityGroupEgress'] = [
{'IpProtocol': 'tcp',
'FromPort': '22',
'ToPort': '22',
'CidrIp': '0.0.0.0/0'},
{'SourceSecurityGroupName': 'xxxx'},
]
after = rsrc_defn.ResourceDefinition(sg.name, sg.type(), props)
scheduler.TaskRunner(sg.update, after)()
self.assertEqual((sg.UPDATE, sg.COMPLETE), sg.state)

21
heat/tests/aws/test_volume.py
View File

@ -226,14 +226,13 @@ class VolumeTest(vt_base.VolumeTestCase):
# delete script
fva = vt_base.FakeVolume('in-use')
self.fc.volumes.get_server_volume.return_value = fva
self.cinder_fc.volumes.get.side_effect = [
fva, vt_base.FakeVolume('detaching', id=fva.id),
vt_base.FakeVolume('available', id=fva.id)
]
self.fc.volumes.delete_server_volume.return_value = None
self.fc.volumes.get_server_volume.side_effect = [
fva, fakes_nova.fake_exception]
fva, fva, fakes_nova.fake_exception()]
scheduler.TaskRunner(rsrc.delete)()
self.fc.volumes.get_server_volume.assert_called_with(u'WikiDatabase',
@ -261,7 +260,7 @@ class VolumeTest(vt_base.VolumeTestCase):
# delete script
fva = vt_base.FakeVolume('in-use')
self.fc.volumes.get_server_volume.side_effect = [
fva, fva, fakes_nova.fake_exception]
fva, fva, fakes_nova.fake_exception()]
self.cinder_fc.volumes.get.side_effect = [
fva, vt_base.FakeVolume('available', id=fva.id)]
@ -322,8 +321,8 @@ class VolumeTest(vt_base.VolumeTestCase):
rsrc = self.create_attachment(self.t, stack, 'MountPoint')
# delete script
self.cinder_fc.volumes.get.return_value = fva
exc = fakes_nova.fake_exception
self.cinder_fc.volumes.get.side_effect = [fva]
exc = fakes_nova.fake_exception()
self.fc.volumes.get_server_volume.side_effect = exc
scheduler.TaskRunner(rsrc.delete)()
@ -348,7 +347,7 @@ class VolumeTest(vt_base.VolumeTestCase):
# delete script
self.fc.volumes.get_server_volume.side_effect = [
fva, fva, fakes_nova.fake_exception]
fva, fva, fakes_nova.fake_exception()]
self.cinder_fc.volumes.get.side_effect = [
fva, vt_base.FakeVolume('in-use', id=fva.id),
vt_base.FakeVolume('detaching', id=fva.id),
@ -542,6 +541,11 @@ class VolumeTest(vt_base.VolumeTestCase):
rsrc = self.create_volume(self.t, stack, 'DataVolume')
self.cinder_fc.volumes.get.side_effect = [
fv,
vt_base.FakeVolume('available'),
cinder_exp.NotFound('Not found')
]
scheduler.TaskRunner(rsrc.destroy)()
self.m_backups.create.assert_called_once_with(fv.id)
@ -595,6 +599,11 @@ class VolumeTest(vt_base.VolumeTestCase):
self.assertIn('Went to status error due to "Unknown"',
six.text_type(ex))
self.cinder_fc.volumes.get.side_effect = [
fva,
cinder_exp.NotFound('Not found')
]
scheduler.TaskRunner(rsrc.destroy)()
def test_create_from_snapshot(self):

3
heat/tests/openstack/cinder/test_volume_utils.py
View File

@ -61,7 +61,8 @@ class VolumeTestCase(common.HeatTestCase):
result = [fva]
for m in extra_create_server_volume_mocks:
result.append(m)
self.fc.volumes.create_server_volume.side_effect = result
prev = self.fc.volumes.create_server_volume.side_effect or []
self.fc.volumes.create_server_volume.side_effect = list(prev) + result
fv_ready = FakeVolume(final_status, id=fva.id)
return fv_ready

5
heat/tests/openstack/heat/test_swiftsignal.py
View File

@ -913,9 +913,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()

1
heat/tests/openstack/neutron/test_neutron_loadbalancer.py
View File

@ -368,6 +368,7 @@ class PoolTest(common.HeatTestCase):
self.mock_create_vip.return_value = {'vip': {'id': 'xyz'}}
self.mock_show.side_effect = [
{'pool': {'status': 'PENDING_CREATE'}},
{'pool': {'status': 'ACTIVE'}},
{'pool': {'status': 'ACTIVE'}}]
self.mock_show_vip.side_effect = [
{'vip': {'status': 'PENDING_CREATE'}},

138
heat/tests/openstack/nova/test_server.py
View File

@ -229,6 +229,19 @@ def create_fake_iface(port=None, net=None, mac=None, ip=None, subnet=None):
return fake_interface(port, net, mac, ip, subnet)
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()
@ -1696,12 +1709,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)
@ -2090,23 +2103,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.
@ -2122,13 +2134,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))
@ -2138,7 +2146,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.
@ -2161,17 +2169,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',
@ -2180,8 +2185,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):
@ -2384,13 +2391,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(
@ -2463,15 +2466,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)
@ -2495,15 +2495,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)
@ -2520,15 +2517,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)
@ -2616,13 +2610,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)
@ -3903,10 +3894,9 @@ class ServersTest(common.HeatTestCase):
self.assertEqual((server.UPDATE, server.COMPLETE), server.state)
self.assertEqual(1, mock_detach.call_count)
self.assertEqual(1, mock_attach.call_count)
mock_attach.called_once_with(
{'port_id': None,
'net_id': auto_allocate_net,
'fip': None})
mock_attach.assert_called_once_with(None,
[auto_allocate_net],
None)
else:
self.assertRaises(exception.ResourceFailure, updater)
self.assertEqual(0, mock_detach.call_count)
@ -4293,10 +4283,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)

9
heat/tests/test_resource.py
View File

@ -1055,10 +1055,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',
@ -1071,8 +1069,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)

32
heat/tests/test_vpc.py
View File

@ -95,7 +95,7 @@ class VPCTestBase(common.HeatTestCase):
"tenant_id": "c1210485b2424d48804aad5d39c61b8f",
"id": "aaaa"
}}]
for i in range(3):
for i in range(7):
show_network_returns.append(
{"network": {
"status": "ACTIVE",
@ -355,7 +355,7 @@ Resources:
self.mockclient.show_network.assert_called_with('aaaa')
self.mockclient.list_routers.assert_called_with(name=self.vpc_name)
self.assertEqual(2, self.mockclient.list_routers.call_count)
self.assertEqual(5, self.mockclient.list_routers.call_count)
self.assertEqual(7, self.mockclient.show_network.call_count)
@ -719,14 +719,16 @@ Resources:
self.assertEqual(2, self.mockclient.create_router.call_count)
self.mockclient.create_router.assert_called_with(
{'router': {'name': self.rt_name}})
self.mockclient.add_interface_router.assert_called_once_with(
u'bbbb', {'subnet_id': 'cccc'})
self.mockclient.add_interface_router.assert_has_calls([
mock.call('bbbb', {'subnet_id': u'cccc'}),
mock.call('ffff', {'subnet_id': u'cccc'}),
])
self.mockclient.list_networks.assert_called_once_with(
**{'router:external': True})
gateway = stack['the_gateway']
self.assertResourceState(gateway, gateway.physical_resource_name())
self.mockclient.add_gateway_router.assert_called_once_with(
self.mockclient.add_gateway_router.assert_called_with(
'ffff', {'network_id': '0389f747-7785-4757-b7bb-2ab07e4b09c3'})
attachment = stack['the_attachment']
@ -736,9 +738,10 @@ Resources:
self.assertEqual([route_table], list(attachment._vpc_route_tables()))
stack.delete()
self.mockclient.remove_interface_router.assert_called_with(
'ffff',
{'subnet_id': u'cccc'})
self.mockclient.remove_interface_router.assert_has_calls([
mock.call('ffff', {'subnet_id': u'cccc'}),
mock.call('bbbb', {'subnet_id': u'cccc'}),
])
self.mockclient.remove_gateway_router.assert_called_with('ffff')
self.assertEqual(2, self.mockclient.remove_gateway_router.call_count)
self.assertEqual(2, self.mockclient.show_subnet.call_count)
@ -783,8 +786,10 @@ Resources:
stack = self.create_stack(self.test_template)
self.mockclient.create_router.assert_called_with(
{'router': {'name': self.rt_name}})
self.mockclient.add_interface_router.assert_called_once_with(
u'bbbb', {'subnet_id': 'cccc'})
self.mockclient.add_interface_router.assert_has_calls([
mock.call('bbbb', {'subnet_id': u'cccc'}),
mock.call('ffff', {'subnet_id': u'cccc'}),
])
route_table = stack['the_route_table']
self.assertResourceState(route_table, 'ffff')
@ -800,9 +805,10 @@ Resources:
scheduler.TaskRunner(route_table.delete)()
stack.delete()
self.mockclient.remove_interface_router.assert_called_with(
'ffff',
{'subnet_id': u'cccc'})
self.mockclient.remove_interface_router.assert_has_calls([
mock.call('ffff', {'subnet_id': u'cccc'}),
mock.call('bbbb', {'subnet_id': u'cccc'}),
])
self.mockclient.remove_gateway_router.assert_called_once_with('ffff')
self.assertEqual(2, self.mockclient.show_subnet.call_count)
self.mockclient.show_subnet.assert_called_with('cccc')