openstack/ironic
Code Issues Proposed changes

Improve port update API unit tests

Browse Source 
Currently, the RPCAPI update_port method is mocked, and its return
value set in the unit tests to the expected value - the modified port.
This isn't really exercising all of the port update API handler, which
should be modifying the port object appropriately and passing it to the
RPCAPI update_port method.

This change adds a side effect to the RPCAPI update_port mock which
saves the Port object that it is passed to the DB. This allows us to
avoid fudging the answer and test the code more thoroughly.

The TestPost test case already does this for port creation.

Change-Id: I77860b2a24da659418f93c380db67ff4726257ff
Related-Bug: #1666009
This commit is contained in:
Mark Goddard 2017-07-12 14:57:18 +01:00
parent e718b837a0
commit bfd80a5d39
1 changed files with 28 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
ironic/tests/unit/api/v1/test_ports.py
View File

@ -64,6 +64,16 @@ def _rpcapi_create_port(self, context, port, topic):
return port return port
def _rpcapi_update_port(self, context, port, topic):
"""Fake used to mock out the conductor RPCAPI's update_port method.
Saves the updated port object and returns the updated port as-per the real
method.
"""
port.save()
return port
class TestPortObject(base.TestCase): class TestPortObject(base.TestCase):
@mock.patch("pecan.request") @mock.patch("pecan.request")
@ -684,7 +694,8 @@ class TestListPorts(test_api_base.BaseApiTest):
response.json['error_message']) response.json['error_message'])
@mock.patch.object(rpcapi.ConductorAPI, 'update_port') @mock.patch.object(rpcapi.ConductorAPI, 'update_port', autospec=True,
side_effect=_rpcapi_update_port)
class TestPatch(test_api_base.BaseApiTest): class TestPatch(test_api_base.BaseApiTest):
def setUp(self): def setUp(self):
@ -701,7 +712,6 @@ class TestPatch(test_api_base.BaseApiTest):
def _test_success(self, mock_upd, patch, version): def _test_success(self, mock_upd, patch, version):
# Helper to test an update to a port that is expected to succeed at a # Helper to test an update to a port that is expected to succeed at a
# given API version. # given API version.
mock_upd.return_value = self.port
headers = {api_base.Version.string: version} headers = {api_base.Version.string: version}
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
patch, patch,
@ -709,7 +719,7 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual(http_client.OK, response.status_code) self.assertEqual(http_client.OK, response.status_code)
self.assertTrue(mock_upd.called) self.assertTrue(mock_upd.called)
self.assertEqual(self.port.id, mock_upd.call_args[0][1].id) self.assertEqual(self.port.id, mock_upd.call_args[0][2].id)
return response return response
def _test_old_api_version(self, mock_upd, patch, version): def _test_old_api_version(self, mock_upd, patch, version):
@ -728,8 +738,6 @@ class TestPatch(test_api_base.BaseApiTest):
@mock.patch.object(notification_utils, '_emit_api_notification') @mock.patch.object(notification_utils, '_emit_api_notification')
def test_update_byid(self, mock_notify, mock_upd): def test_update_byid(self, mock_notify, mock_upd):
extra = {'foo': 'bar'} extra = {'foo': 'bar'}
mock_upd.return_value = self.port
mock_upd.return_value.extra = extra
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/extra/foo', [{'path': '/extra/foo',
'value': 'bar', 'value': 'bar',
@ -738,7 +746,7 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual(http_client.OK, response.status_code) self.assertEqual(http_client.OK, response.status_code)
self.assertEqual(extra, response.json['extra']) self.assertEqual(extra, response.json['extra'])
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(extra, kargs.extra) self.assertEqual(extra, kargs.extra)
mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update', mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update',
obj_fields.NotificationLevel.INFO, obj_fields.NotificationLevel.INFO,
@ -752,9 +760,6 @@ class TestPatch(test_api_base.BaseApiTest):
portgroup_uuid=wtypes.Unset)]) portgroup_uuid=wtypes.Unset)])
def test_update_byaddress_not_allowed(self, mock_upd): def test_update_byaddress_not_allowed(self, mock_upd):
extra = {'foo': 'bar'}
mock_upd.return_value = self.port
mock_upd.return_value.extra = extra
response = self.patch_json('/ports/%s' % self.port.address, response = self.patch_json('/ports/%s' % self.port.address,
[{'path': '/extra/foo', [{'path': '/extra/foo',
'value': 'bar', 'value': 'bar',
@ -779,8 +784,6 @@ class TestPatch(test_api_base.BaseApiTest):
def test_replace_singular(self, mock_upd): def test_replace_singular(self, mock_upd):
address = 'aa:bb:cc:dd:ee:ff' address = 'aa:bb:cc:dd:ee:ff'
mock_upd.return_value = self.port
mock_upd.return_value.address = address
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/address', [{'path': '/address',
'value': address, 'value': address,
@ -790,7 +793,7 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual(address, response.json['address']) self.assertEqual(address, response.json['address'])
self.assertTrue(mock_upd.called) self.assertTrue(mock_upd.called)
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(address, kargs.address) self.assertEqual(address, kargs.address)
@mock.patch.object(notification_utils, '_emit_api_notification') @mock.patch.object(notification_utils, '_emit_api_notification')
@ -807,7 +810,7 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertTrue(response.json['error_message']) self.assertTrue(response.json['error_message'])
self.assertTrue(mock_upd.called) self.assertTrue(mock_upd.called)
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(address, kargs.address) self.assertEqual(address, kargs.address)
mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update', mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update',
obj_fields.NotificationLevel.INFO, obj_fields.NotificationLevel.INFO,
@ -821,7 +824,6 @@ class TestPatch(test_api_base.BaseApiTest):
portgroup_uuid=wtypes.Unset)]) portgroup_uuid=wtypes.Unset)])
def test_replace_node_uuid(self, mock_upd): def test_replace_node_uuid(self, mock_upd):
mock_upd.return_value = self.port
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/node_uuid', [{'path': '/node_uuid',
'value': self.node.uuid, 'value': self.node.uuid,
@ -831,8 +833,6 @@ class TestPatch(test_api_base.BaseApiTest):
def test_replace_local_link_connection(self, mock_upd): def test_replace_local_link_connection(self, mock_upd):
switch_id = 'aa:bb:cc:dd:ee:ff' switch_id = 'aa:bb:cc:dd:ee:ff'
mock_upd.return_value = self.port
mock_upd.return_value.local_link_connection['switch_id'] = switch_id
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': [{'path':
'/local_link_connection/switch_id', '/local_link_connection/switch_id',
@ -845,7 +845,7 @@ class TestPatch(test_api_base.BaseApiTest):
response.json['local_link_connection']['switch_id']) response.json['local_link_connection']['switch_id'])
self.assertTrue(mock_upd.called) self.assertTrue(mock_upd.called)
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(switch_id, kargs.local_link_connection['switch_id']) self.assertEqual(switch_id, kargs.local_link_connection['switch_id'])
def test_remove_local_link_connection_old_api(self, mock_upd): def test_remove_local_link_connection_old_api(self, mock_upd):
@ -868,7 +868,6 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
def test_add_portgroup_uuid(self, mock_upd): def test_add_portgroup_uuid(self, mock_upd):
mock_upd.return_value = self.port
pg = obj_utils.create_test_portgroup(self.context, pg = obj_utils.create_test_portgroup(self.context,
node_id=self.node.id, node_id=self.node.id,
uuid=uuidutils.generate_uuid(), uuid=uuidutils.generate_uuid(),
@ -890,7 +889,6 @@ class TestPatch(test_api_base.BaseApiTest):
uuid=uuidutils.generate_uuid(), uuid=uuidutils.generate_uuid(),
address='bb:bb:bb:bb:bb:bb', address='bb:bb:bb:bb:bb:bb',
name='bar') name='bar')
mock_upd.return_value = self.port
headers = {api_base.Version.string: '1.24'} headers = {api_base.Version.string: '1.24'}
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/portgroup_uuid', [{'path': '/portgroup_uuid',
@ -906,7 +904,6 @@ class TestPatch(test_api_base.BaseApiTest):
uuid=uuidutils.generate_uuid(), uuid=uuidutils.generate_uuid(),
address='bb:bb:bb:bb:bb:bb', address='bb:bb:bb:bb:bb:bb',
name='bar') name='bar')
mock_upd.return_value = self.port
headers = {api_base.Version.string: '1.24'} headers = {api_base.Version.string: '1.24'}
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/portgroup_uuid', [{'path': '/portgroup_uuid',
@ -914,7 +911,7 @@ class TestPatch(test_api_base.BaseApiTest):
'op': 'remove'}], 'op': 'remove'}],
headers=headers) headers=headers)
self.assertEqual('application/json', response.content_type) self.assertEqual('application/json', response.content_type)
self.assertIsNone(mock_upd.call_args[0][1].portgroup_id) self.assertIsNone(mock_upd.call_args[0][2].portgroup_id)
def test_replace_portgroup_uuid_remove_add(self, mock_upd): def test_replace_portgroup_uuid_remove_add(self, mock_upd):
pg = obj_utils.create_test_portgroup(self.context, pg = obj_utils.create_test_portgroup(self.context,
@ -927,7 +924,6 @@ class TestPatch(test_api_base.BaseApiTest):
uuid=uuidutils.generate_uuid(), uuid=uuidutils.generate_uuid(),
address='bb:bb:bb:bb:bb:b1', address='bb:bb:bb:bb:bb:b1',
name='bbb') name='bbb')
mock_upd.return_value = self.port
headers = {api_base.Version.string: '1.24'} headers = {api_base.Version.string: '1.24'}
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/portgroup_uuid', [{'path': '/portgroup_uuid',
@ -938,7 +934,7 @@ class TestPatch(test_api_base.BaseApiTest):
'op': 'add'}], 'op': 'add'}],
headers=headers) headers=headers)
self.assertEqual('application/json', response.content_type) self.assertEqual('application/json', response.content_type)
self.assertTrue(pg1.id, mock_upd.call_args[0][1].portgroup_id) self.assertTrue(pg1.id, mock_upd.call_args[0][2].portgroup_id)
def test_replace_portgroup_uuid_old_api(self, mock_upd): def test_replace_portgroup_uuid_old_api(self, mock_upd):
pg = obj_utils.create_test_portgroup(self.context, pg = obj_utils.create_test_portgroup(self.context,
@ -946,7 +942,6 @@ class TestPatch(test_api_base.BaseApiTest):
uuid=uuidutils.generate_uuid(), uuid=uuidutils.generate_uuid(),
address='bb:bb:bb:bb:bb:bb', address='bb:bb:bb:bb:bb:bb',
name='bar') name='bar')
mock_upd.return_value = self.port
headers = {api_base.Version.string: '1.15'} headers = {api_base.Version.string: '1.15'}
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/portgroup_uuid', [{'path': '/portgroup_uuid',
@ -958,7 +953,6 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
def test_add_node_uuid(self, mock_upd): def test_add_node_uuid(self, mock_upd):
mock_upd.return_value = self.port
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/node_uuid', [{'path': '/node_uuid',
'value': self.node.uuid, 'value': self.node.uuid,
@ -1020,14 +1014,12 @@ class TestPatch(test_api_base.BaseApiTest):
patch.append({'path': '/extra/%s' % k, patch.append({'path': '/extra/%s' % k,
'value': extra[k], 'value': extra[k],
'op': 'replace'}) 'op': 'replace'})
mock_upd.return_value = self.port
mock_upd.return_value.extra = extra
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
patch) patch)
self.assertEqual('application/json', response.content_type) self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code) self.assertEqual(http_client.OK, response.status_code)
self.assertEqual(extra, response.json['extra']) self.assertEqual(extra, response.json['extra'])
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(extra, kargs.extra) self.assertEqual(extra, kargs.extra)
def test_remove_multi(self, mock_upd): def test_remove_multi(self, mock_upd):
@ -1037,26 +1029,23 @@ class TestPatch(test_api_base.BaseApiTest):
# Removing one item from the collection # Removing one item from the collection
extra.pop('foo1') extra.pop('foo1')
mock_upd.return_value = self.port
mock_upd.return_value.extra = extra
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/extra/foo1', [{'path': '/extra/foo1',
'op': 'remove'}]) 'op': 'remove'}])
self.assertEqual('application/json', response.content_type) self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code) self.assertEqual(http_client.OK, response.status_code)
self.assertEqual(extra, response.json['extra']) self.assertEqual(extra, response.json['extra'])
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(extra, kargs.extra) self.assertEqual(extra, kargs.extra)
# Removing the collection # Removing the collection
extra = {} extra = {}
mock_upd.return_value.extra = extra
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/extra', 'op': 'remove'}]) [{'path': '/extra', 'op': 'remove'}])
self.assertEqual('application/json', response.content_type) self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code) self.assertEqual(http_client.OK, response.status_code)
self.assertEqual({}, response.json['extra']) self.assertEqual({}, response.json['extra'])
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(extra, kargs.extra) self.assertEqual(extra, kargs.extra)
# Assert nothing else was changed # Assert nothing else was changed
@ -1086,8 +1075,6 @@ class TestPatch(test_api_base.BaseApiTest):
def test_add_root(self, mock_upd): def test_add_root(self, mock_upd):
address = 'aa:bb:cc:dd:ee:ff' address = 'aa:bb:cc:dd:ee:ff'
mock_upd.return_value = self.port
mock_upd.return_value.address = address
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/address', [{'path': '/address',
'value': address, 'value': address,
@ -1096,7 +1083,7 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual(http_client.OK, response.status_code) self.assertEqual(http_client.OK, response.status_code)
self.assertEqual(address, response.json['address']) self.assertEqual(address, response.json['address'])
self.assertTrue(mock_upd.called) self.assertTrue(mock_upd.called)
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(address, kargs.address) self.assertEqual(address, kargs.address)
def test_add_root_non_existent(self, mock_upd): def test_add_root_non_existent(self, mock_upd):
@ -1117,14 +1104,12 @@ class TestPatch(test_api_base.BaseApiTest):
patch.append({'path': '/extra/%s' % k, patch.append({'path': '/extra/%s' % k,
'value': extra[k], 'value': extra[k],
'op': 'add'}) 'op': 'add'})
mock_upd.return_value = self.port
mock_upd.return_value.extra = extra
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
patch) patch)
self.assertEqual('application/json', response.content_type) self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code) self.assertEqual(http_client.OK, response.status_code)
self.assertEqual(extra, response.json['extra']) self.assertEqual(extra, response.json['extra'])
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(extra, kargs.extra) self.assertEqual(extra, kargs.extra)
def test_remove_uuid(self, mock_upd): def test_remove_uuid(self, mock_upd):
@ -1150,8 +1135,6 @@ class TestPatch(test_api_base.BaseApiTest):
def test_update_port_address_normalized(self, mock_upd): def test_update_port_address_normalized(self, mock_upd):
address = 'AA:BB:CC:DD:EE:FF' address = 'AA:BB:CC:DD:EE:FF'
mock_upd.return_value = self.port
mock_upd.return_value.address = address.lower()
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/address', [{'path': '/address',
'value': address, 'value': address,
@ -1159,13 +1142,11 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual('application/json', response.content_type) self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code) self.assertEqual(http_client.OK, response.status_code)
self.assertEqual(address.lower(), response.json['address']) self.assertEqual(address.lower(), response.json['address'])
kargs = mock_upd.call_args[0][1] kargs = mock_upd.call_args[0][2]
self.assertEqual(address.lower(), kargs.address) self.assertEqual(address.lower(), kargs.address)
def test_update_pxe_enabled_allowed(self, mock_upd): def test_update_pxe_enabled_allowed(self, mock_upd):
pxe_enabled = True pxe_enabled = True
mock_upd.return_value = self.port
mock_upd.return_value.pxe_enabled = pxe_enabled
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/pxe_enabled', [{'path': '/pxe_enabled',
'value': pxe_enabled, 'value': pxe_enabled,
@ -1176,7 +1157,6 @@ class TestPatch(test_api_base.BaseApiTest):
def test_update_pxe_enabled_old_api_version(self, mock_upd): def test_update_pxe_enabled_old_api_version(self, mock_upd):
pxe_enabled = True pxe_enabled = True
mock_upd.return_value = self.port
headers = {api_base.Version.string: '1.14'} headers = {api_base.Version.string: '1.14'}
response = self.patch_json('/ports/%s' % self.port.uuid, response = self.patch_json('/ports/%s' % self.port.uuid,
[{'path': '/pxe_enabled', [{'path': '/pxe_enabled',
@ -1192,16 +1172,13 @@ class TestPatch(test_api_base.BaseApiTest):
expected_physical_network): expected_physical_network):
# Helper to test an update to a port's physical_network that is # Helper to test an update to a port's physical_network that is
# expected to succeed at API version 1.34. # expected to succeed at API version 1.34.
self.port.physical_network = expected_physical_network
response = self._test_success(mock_upd, patch, '1.34') response = self._test_success(mock_upd, patch, '1.34')
self.assertEqual(expected_physical_network, self.assertEqual(expected_physical_network,
response.json['physical_network']) response.json['physical_network'])
# TODO(mgoddard): Add this when mock_upd has been modified to save the self.port.refresh()
# port to the DB. self.assertEqual(expected_physical_network,
# self.port.refresh() self.port.physical_network)
# self.assertEqual(expected_physical_network,
# self.port.physical_network)
def test_add_physical_network(self, mock_upd): def test_add_physical_network(self, mock_upd):
physical_network = 'physnet1' physical_network = 'physnet1'