From cee5922674a571e82632fbc2c25488ea10e903e0 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 28 Apr 2020 12:14:47 +0200 Subject: [PATCH] Hacking: enforce usage of autospec=True in tests Using autospec ensures that mocked functions are called with correct arguments, so it's highly desired to have it. Change-Id: I9c8395adf852495d2ef6db732d727990e8abd5d7 --- .../test/unit/test_common_ironic.py | 6 ++--- ironic_inspector/test/unit/test_introspect.py | 4 ++-- ironic_inspector/test/unit/test_iptables.py | 2 +- ironic_inspector/test/unit/test_main.py | 16 ++++++------- ironic_inspector/test/unit/test_manager.py | 4 ++-- ironic_inspector/test/unit/test_migrations.py | 22 +++++++++-------- ironic_inspector/test/unit/test_node_cache.py | 12 +++++----- .../test/unit/test_plugins_lldp_basic.py | 24 +++++++++---------- .../test_plugins_local_link_connection.py | 3 ++- .../test/unit/test_plugins_pci_devices.py | 18 +++++++------- .../unit/test_plugins_physnet_cidr_map.py | 11 +++++---- .../test/unit/test_plugins_standard.py | 4 ++-- ironic_inspector/test/unit/test_process.py | 9 ++++--- tox.ini | 3 ++- 14 files changed, 73 insertions(+), 65 deletions(-) diff --git a/ironic_inspector/test/unit/test_common_ironic.py b/ironic_inspector/test/unit/test_common_ironic.py index 4567fc5ad..c3eaa8cab 100644 --- a/ironic_inspector/test/unit/test_common_ironic.py +++ b/ironic_inspector/test/unit/test_common_ironic.py @@ -24,7 +24,7 @@ from ironic_inspector.test import base from ironic_inspector import utils -@mock.patch.object(keystone, 'get_session') +@mock.patch.object(keystone, 'get_session', autospec=True) @mock.patch.object(openstack.connection, 'Connection', autospec=True) class TestGetClientBase(base.BaseTest): def setUp(self): @@ -55,7 +55,7 @@ class TestGetIpmiAddress(base.BaseTest): self.assertEqual((self.ipmi_ipv6, None, self.ipmi_ipv6), ir_utils.get_ipmi_address(node)) - @mock.patch('socket.getaddrinfo') + @mock.patch('socket.getaddrinfo', autospec=True) def test_good_hostname_resolves(self, mock_socket): node = mock.Mock(spec=['driver_info', 'uuid'], driver_info={'ipmi_address': self.ipmi_address}) @@ -67,7 +67,7 @@ class TestGetIpmiAddress(base.BaseTest): mock_socket.assert_called_once_with(self.ipmi_address, None, 0, 0, socket.SOL_TCP) - @mock.patch('socket.getaddrinfo') + @mock.patch('socket.getaddrinfo', autospec=True) def test_bad_hostname_errors(self, mock_socket): node = mock.Mock(spec=['driver_info', 'uuid'], driver_info={'ipmi_address': 'meow'}, diff --git a/ironic_inspector/test/unit/test_introspect.py b/ironic_inspector/test/unit/test_introspect.py index a723af87a..d56e71858 100644 --- a/ironic_inspector/test/unit/test_introspect.py +++ b/ironic_inspector/test/unit/test_introspect.py @@ -380,7 +380,7 @@ class TestIntrospect(BaseTest): self.assertTrue(start_mock.called) - @mock.patch.object(time, 'time') + @mock.patch.object(time, 'time', autospec=True) def test_introspection_delay(self, time_mock, client_mock, start_mock): time_mock.return_value = 42 introspect._LAST_INTROSPECTION_TIME = 40 @@ -400,7 +400,7 @@ class TestIntrospect(BaseTest): # updated to the current time.time() self.assertEqual(42, introspect._LAST_INTROSPECTION_TIME) - @mock.patch.object(time, 'time') + @mock.patch.object(time, 'time', autospec=True) def test_introspection_delay_not_needed(self, time_mock, client_mock, start_mock): diff --git a/ironic_inspector/test/unit/test_iptables.py b/ironic_inspector/test/unit/test_iptables.py index 7734954b2..25cdff305 100644 --- a/ironic_inspector/test/unit/test_iptables.py +++ b/ironic_inspector/test/unit/test_iptables.py @@ -358,7 +358,7 @@ class TestIBMapping(test_base.BaseTest): def test_open_no_such_file(self): with mock.patch('builtins.open', - side_effect=IOError()) as mock_open: + side_effect=IOError(), autospec=True) as mock_open: iptables._ib_mac_to_rmac_mapping(self.ports) self.assertEqual(self.ib_address, self.ib_port.address) diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index f933e1579..dc4643314 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/ironic_inspector/test/unit/test_main.py @@ -417,7 +417,7 @@ class TestApiReapply(BaseAPITest): class TestApiRules(BaseAPITest): - @mock.patch.object(rules, 'get_all') + @mock.patch.object(rules, 'get_all', autospec=True) def test_get_all(self, get_all_mock): get_all_mock.return_value = [ mock.Mock(spec=rules.IntrospectionRule, @@ -444,7 +444,7 @@ class TestApiRules(BaseAPITest): for m in get_all_mock.return_value: m.as_dict.assert_called_with(short=True) - @mock.patch.object(rules, 'delete_all') + @mock.patch.object(rules, 'delete_all', autospec=True) def test_delete_all(self, delete_all_mock): res = self.app.delete('/v1/rules') self.assertEqual(204, res.status_code) @@ -501,7 +501,7 @@ class TestApiRules(BaseAPITest): res = self.app.post('/v1/rules', data=json.dumps(data)) self.assertEqual(400, res.status_code) - @mock.patch.object(rules, 'get') + @mock.patch.object(rules, 'get', autospec=True) def test_get_one(self, get_mock): get_mock.return_value = mock.Mock(spec=rules.IntrospectionRule, **{'as_dict.return_value': @@ -517,7 +517,7 @@ class TestApiRules(BaseAPITest): get_mock.assert_called_once_with(self.uuid) get_mock.return_value.as_dict.assert_called_once_with(short=False) - @mock.patch.object(rules, 'delete') + @mock.patch.object(rules, 'delete', autospec=True) def test_delete_one(self, delete_mock): res = self.app.delete('/v1/rules/' + self.uuid) self.assertEqual(204, res.status_code) @@ -697,7 +697,7 @@ class TestTopic(test_base.BaseTest): self.target_mock.assert_called_with( topic='hello', version=mock.ANY) - @mock.patch.object(coordination, 'get_coordinator') + @mock.patch.object(coordination, 'get_coordinator', autospec=True) def test_get_random_topic(self, mock_get_coordinator): mock_coordinator = mock.Mock(spec=['get_members']) members = [('ironic_inspector.conductor.host%s' % i).encode('ascii') @@ -709,7 +709,7 @@ class TestTopic(test_base.BaseTest): topic = main.get_random_topic() self.assertIn(topic, topics) - @mock.patch.object(coordination, 'get_coordinator') + @mock.patch.object(coordination, 'get_coordinator', autospec=True) def test_get_random_topic_host_with_domain(self, mock_get_coordinator): mock_coordinator = mock.Mock(spec=['get_members']) members = ['ironic_inspector.conductor.' @@ -719,7 +719,7 @@ class TestTopic(test_base.BaseTest): topic = main.get_random_topic() self.assertEqual(topic, 'ironic_inspector.conductor.local.domain') - @mock.patch.object(coordination, 'get_coordinator') + @mock.patch.object(coordination, 'get_coordinator', autospec=True) def test_get_random_topic_host_bypass_invalid(self, mock_get_coordinator): mock_coordinator = mock.Mock(spec=['get_members']) members = ['this_should_not_happen'.encode('ascii')] @@ -729,7 +729,7 @@ class TestTopic(test_base.BaseTest): 'No available conductor', main.get_random_topic) - @mock.patch.object(coordination, 'get_coordinator') + @mock.patch.object(coordination, 'get_coordinator', autospec=True) def test_get_random_topic_no_member(self, mock_get_coordinator): mock_coordinator = mock.Mock(spec=['get_members']) mock_coordinator.get_members.return_value = set() diff --git a/ironic_inspector/test/unit/test_manager.py b/ironic_inspector/test/unit/test_manager.py index e4b27c274..c08611c84 100644 --- a/ironic_inspector/test/unit/test_manager.py +++ b/ironic_inspector/test/unit/test_manager.py @@ -154,7 +154,7 @@ class TestManagerInitHost(BaseManagerTest): mock_get_coord.assert_called_once_with(prefix='conductor') mock_coordinator.start.assert_called_once_with(heartbeat=True) - @mock.patch.object(manager.ConductorManager, 'del_host') + @mock.patch.object(manager.ConductorManager, 'del_host', autospec=True) @mock.patch.object(coordination, 'get_coordinator', autospec=True) @mock.patch.object(keystone, 'get_endpoint', autospec=True) def test_init_host_with_coordinator_failed(self, mock_endpoint, @@ -169,7 +169,7 @@ class TestManagerInitHost(BaseManagerTest): self.mock_filter.init_filter.assert_called_once_with() self.assert_periodics() mock_get_coord.assert_called_once_with(prefix='conductor') - mock_del_host.assert_called_once_with() + mock_del_host.assert_called_once_with(self.manager) class TestManagerDelHost(BaseManagerTest): diff --git a/ironic_inspector/test/unit/test_migrations.py b/ironic_inspector/test/unit/test_migrations.py index 6ca6a1ed4..defd0896b 100644 --- a/ironic_inspector/test/unit/test_migrations.py +++ b/ironic_inspector/test/unit/test_migrations.py @@ -52,8 +52,10 @@ LOG = logging.getLogger(__name__) @contextlib.contextmanager def patch_with_engine(engine): - with mock.patch.object(db, 'get_writer_session') as patch_w_sess, \ - mock.patch.object(db, 'get_reader_session') as patch_r_sess: + with mock.patch.object(db, 'get_writer_session', + autospec=True) as patch_w_sess, \ + mock.patch.object(db, 'get_reader_session', + autospec=True) as patch_r_sess: patch_w_sess.return_value = patch_r_sess.return_value = ( orm.get_maker(engine)()) yield @@ -134,10 +136,10 @@ class TestWalkVersions(base.BaseTest, WalkVersionsMixin): self._pre_upgrade_141.assert_called_with(self.engine) self._check_141.assert_called_with(self.engine, test_value) - @mock.patch.object(script, 'ScriptDirectory') - @mock.patch.object(WalkVersionsMixin, '_migrate_up') + @mock.patch.object(script, 'ScriptDirectory', autospec=True) + @mock.patch.object(WalkVersionsMixin, '_migrate_up', autospec=True) def test_walk_versions_all_default(self, _migrate_up, script_directory): - fc = script_directory.from_config() + fc = script_directory.from_config.return_value fc.walk_revisions.return_value = self.versions self.migration_ext.version.return_value = None @@ -145,20 +147,20 @@ class TestWalkVersions(base.BaseTest, WalkVersionsMixin): self.migration_ext.version.assert_called_with() - upgraded = [mock.call(self.engine, self.config, v.revision, + upgraded = [mock.call(self, self.engine, self.config, v.revision, with_data=True) for v in reversed(self.versions)] self.assertEqual(self._migrate_up.call_args_list, upgraded) - @mock.patch.object(script, 'ScriptDirectory') - @mock.patch.object(WalkVersionsMixin, '_migrate_up') + @mock.patch.object(script, 'ScriptDirectory', autospec=True) + @mock.patch.object(WalkVersionsMixin, '_migrate_up', autospec=True) def test_walk_versions_all_false(self, _migrate_up, script_directory): - fc = script_directory.from_config() + fc = script_directory.from_config.return_value fc.walk_revisions.return_value = self.versions self.migration_ext.version.return_value = None self._walk_versions(self.engine, self.config) - upgraded = [mock.call(self.engine, self.config, v.revision, + upgraded = [mock.call(self, self.engine, self.config, v.revision, with_data=True) for v in reversed(self.versions)] self.assertEqual(upgraded, self._migrate_up.call_args_list) diff --git a/ironic_inspector/test/unit/test_node_cache.py b/ironic_inspector/test/unit/test_node_cache.py index 6fae6ed46..736090ada 100644 --- a/ironic_inspector/test/unit/test_node_cache.py +++ b/ironic_inspector/test/unit/test_node_cache.py @@ -104,8 +104,8 @@ class TestNodeCache(test_base.NodeTest): self.assertIsNone(row_option) @mock.patch.object(locking, 'get_lock', autospec=True) - @mock.patch.object(node_cache, '_list_node_uuids') - @mock.patch.object(node_cache, '_delete_node') + @mock.patch.object(node_cache, '_list_node_uuids', autospec=True) + @mock.patch.object(node_cache, '_delete_node', autospec=True) def test_delete_nodes_not_in_list(self, mock__delete_node, mock__list_node_uuids, mock_get_lock): @@ -354,7 +354,7 @@ class TestNodeCacheCleanUp(test_base.NodeTest): self.assertEqual(1, db.model_query(db.Option).count()) @mock.patch.object(locking, 'get_lock', autospec=True) - @mock.patch.object(timeutils, 'utcnow') + @mock.patch.object(timeutils, 'utcnow', autospec=True) def test_ok(self, time_mock, get_lock_mock): time_mock.return_value = datetime.datetime.utcnow() @@ -370,7 +370,7 @@ class TestNodeCacheCleanUp(test_base.NodeTest): self.assertFalse(get_lock_mock.called) @mock.patch.object(node_cache.NodeInfo, 'acquire_lock', autospec=True) - @mock.patch.object(timeutils, 'utcnow') + @mock.patch.object(timeutils, 'utcnow', autospec=True) def test_timeout(self, time_mock, lock_mock): # Add a finished node to confirm we don't try to timeout it time_mock.return_value = self.started_at @@ -400,7 +400,7 @@ class TestNodeCacheCleanUp(test_base.NodeTest): lock_mock.assert_called_once_with(mock.ANY, blocking=False) @mock.patch.object(locking, 'get_lock', autospec=True) - @mock.patch.object(timeutils, 'utcnow') + @mock.patch.object(timeutils, 'utcnow', autospec=True) def test_timeout_active_state(self, time_mock, lock_mock): time_mock.return_value = self.started_at session = db.get_writer_session() @@ -422,7 +422,7 @@ class TestNodeCacheCleanUp(test_base.NodeTest): res) @mock.patch.object(node_cache.NodeInfo, 'acquire_lock', autospec=True) - @mock.patch.object(timeutils, 'utcnow') + @mock.patch.object(timeutils, 'utcnow', autospec=True) def test_timeout_lock_failed(self, time_mock, get_lock_mock): time_mock.return_value = self.started_at CONF.set_override('timeout', 1) diff --git a/ironic_inspector/test/unit/test_plugins_lldp_basic.py b/ironic_inspector/test/unit/test_plugins_lldp_basic.py index 86283687c..834b30f35 100644 --- a/ironic_inspector/test/unit/test_plugins_lldp_basic.py +++ b/ironic_inspector/test/unit/test_plugins_lldp_basic.py @@ -240,7 +240,7 @@ class TestLLDPBasicProcessingHook(test_base.NodeTest): self.hook.before_update(self.data, self.node_info) self.assertEqual(self.expected, self.data['all_interfaces']) - @mock.patch('ironic_inspector.common.lldp_parsers.LOG') + @mock.patch.object(nv.LOG, 'warning', autospec=True) def test_null_strings(self, mock_log): self.data['inventory']['interfaces'] = [{ 'name': 'em1', @@ -260,9 +260,9 @@ class TestLLDPBasicProcessingHook(test_base.NodeTest): self.hook.before_update(self.data, self.node_info) self.assertEqual(self.expected, self.data['all_interfaces']) - self.assertEqual(2, mock_log.warning.call_count) + self.assertEqual(2, mock_log.call_count) - @mock.patch('ironic_inspector.common.lldp_parsers.LOG') + @mock.patch.object(nv.LOG, 'warning', autospec=True) def test_truncated_int(self, mock_log): self.data['inventory']['interfaces'] = [{ 'name': 'em1', @@ -275,9 +275,9 @@ class TestLLDPBasicProcessingHook(test_base.NodeTest): # nothing should be written to lldp_processed self.hook.before_update(self.data, self.node_info) self.assertEqual(self.expected, self.data['all_interfaces']) - self.assertEqual(3, mock_log.warning.call_count) + self.assertEqual(3, mock_log.call_count) - @mock.patch('ironic_inspector.common.lldp_parsers.LOG') + @mock.patch.object(nv.LOG, 'warning', autospec=True) def test_invalid_ip(self, mock_log): self.data['inventory']['interfaces'] = [{ 'name': 'em1', @@ -287,9 +287,9 @@ class TestLLDPBasicProcessingHook(test_base.NodeTest): }] self.hook.before_update(self.data, self.node_info) self.assertEqual(self.expected, self.data['all_interfaces']) - self.assertEqual(2, mock_log.warning.call_count) + self.assertEqual(2, mock_log.call_count) - @mock.patch('ironic_inspector.common.lldp_parsers.LOG') + @mock.patch.object(nv.LOG, 'warning', autospec=True) def test_truncated_mac(self, mock_log): self.data['inventory']['interfaces'] = [{ 'name': 'em1', @@ -299,9 +299,9 @@ class TestLLDPBasicProcessingHook(test_base.NodeTest): self.hook.before_update(self.data, self.node_info) self.assertEqual(self.expected, self.data['all_interfaces']) - self.assertEqual(1, mock_log.warning.call_count) + self.assertEqual(1, mock_log.call_count) - @mock.patch('ironic_inspector.common.lldp_parsers.LOG') + @mock.patch.object(nv.LOG, 'warning', autospec=True) def test_bad_value_macphy(self, mock_log): self.data['inventory']['interfaces'] = [{ 'name': 'em1', @@ -313,9 +313,9 @@ class TestLLDPBasicProcessingHook(test_base.NodeTest): self.hook.before_update(self.data, self.node_info) self.assertEqual(self.expected, self.data['all_interfaces']) - self.assertEqual(3, mock_log.warning.call_count) + self.assertEqual(3, mock_log.call_count) - @mock.patch('ironic_inspector.common.lldp_parsers.LOG') + @mock.patch.object(nv.LOG, 'warning', autospec=True) def test_bad_value_linkagg(self, mock_log): self.data['inventory']['interfaces'] = [{ 'name': 'em1', @@ -326,4 +326,4 @@ class TestLLDPBasicProcessingHook(test_base.NodeTest): self.hook.before_update(self.data, self.node_info) self.assertEqual(self.expected, self.data['all_interfaces']) - self.assertEqual(2, mock_log.warning.call_count) + self.assertEqual(2, mock_log.call_count) diff --git a/ironic_inspector/test/unit/test_plugins_local_link_connection.py b/ironic_inspector/test/unit/test_plugins_local_link_connection.py index 6838dc51b..54bcaeaf2 100644 --- a/ironic_inspector/test/unit/test_plugins_local_link_connection.py +++ b/ironic_inspector/test/unit/test_plugins_local_link_connection.py @@ -214,7 +214,8 @@ class TestGenericLocalLinkConnectionHook(test_base.NodeTest): self.hook.before_update(self.data, self.node_info) self.assertCalledWithPatch(patches, mock_patch) - @mock.patch('ironic_inspector.plugins.local_link_connection.LOG') + @mock.patch('ironic_inspector.plugins.local_link_connection.LOG', + autospec=True) @mock.patch.object(node_cache.NodeInfo, 'patch_port', autospec=True) def test_patch_port_exception(self, mock_patch, mock_log): self.data['all_interfaces'] = { diff --git a/ironic_inspector/test/unit/test_plugins_pci_devices.py b/ironic_inspector/test/unit/test_plugins_pci_devices.py index 72e6bdb68..f4f381786 100644 --- a/ironic_inspector/test/unit/test_plugins_pci_devices.py +++ b/ironic_inspector/test/unit/test_plugins_pci_devices.py @@ -38,7 +38,7 @@ class TestPciDevicesHook(test_base.NodeTest): parsed_pci_alias = pci_devices._parse_pci_alias_entry() self.assertFalse(parsed_pci_alias) - @mock.patch('ironic_inspector.plugins.pci_devices.LOG') + @mock.patch.object(pci_devices.LOG, 'error', autospec=True) def test_parse_pci_alias_entry_invalid_json(self, mock_oslo_log): pci_alias = ['{"vendor_id": "foo1", "product_id": "bar1",' ' "name": "baz1"}', '{"invalid" = "entry"}'] @@ -46,9 +46,9 @@ class TestPciDevicesHook(test_base.NodeTest): valid_pci_alias = {("foo1", "bar1"): "baz1"} parsed_pci_alias = pci_devices._parse_pci_alias_entry() self.assertEqual(valid_pci_alias, parsed_pci_alias) - mock_oslo_log.error.assert_called_once() + mock_oslo_log.assert_called_once() - @mock.patch('ironic_inspector.plugins.pci_devices.LOG') + @mock.patch.object(pci_devices.LOG, 'error', autospec=True) def test_parse_pci_alias_entry_invalid_keys(self, mock_oslo_log): pci_alias = ['{"vendor_id": "foo1", "product_id": "bar1",' ' "name": "baz1"}', '{"invalid": "keys"}'] @@ -56,7 +56,7 @@ class TestPciDevicesHook(test_base.NodeTest): valid_pci_alias = {("foo1", "bar1"): "baz1"} parsed_pci_alias = pci_devices._parse_pci_alias_entry() self.assertEqual(valid_pci_alias, parsed_pci_alias) - mock_oslo_log.error.assert_called_once() + mock_oslo_log.assert_called_once() @mock.patch.object(hook, 'aliases', {("1234", "5678"): "pci_dev1", ("9876", "5432"): "pci_dev2"}) @@ -74,7 +74,7 @@ class TestPciDevicesHook(test_base.NodeTest): mock_update_props.assert_called_once_with(self.node_info, **expected_pci_devices_count) - @mock.patch('ironic_inspector.plugins.pci_devices.LOG') + @mock.patch.object(pci_devices.LOG, 'warning', autospec=True) @mock.patch.object(node_cache.NodeInfo, 'update_capabilities', autospec=True) def test_before_update_no_pci_info_from_ipa(self, mock_update_props, @@ -83,11 +83,11 @@ class TestPciDevicesHook(test_base.NodeTest): ' "name": "baz1"}'] base.CONF.set_override('alias', pci_alias, 'pci_devices') self.hook.before_update(self.data, self.node_info) - mock_oslo_log.warning.assert_called_once() + mock_oslo_log.assert_called_once() self.assertFalse(mock_update_props.called) - @mock.patch.object(pci_devices, '_parse_pci_alias_entry') - @mock.patch('ironic_inspector.plugins.pci_devices.LOG') + @mock.patch.object(pci_devices, '_parse_pci_alias_entry', autospec=True) + @mock.patch.object(pci_devices.LOG, 'info', autospec=True) @mock.patch.object(node_cache.NodeInfo, 'update_capabilities', autospec=True) def test_before_update_no_match(self, mock_update_props, mock_oslo_log, @@ -99,4 +99,4 @@ class TestPciDevicesHook(test_base.NodeTest): mock_parse_pci_alias.return_value = {("9876", "5432"): "pci_dev"} self.hook.before_update(self.data, self.node_info) self.assertFalse(mock_update_props.called) - self.assertFalse(mock_oslo_log.info.called) + self.assertFalse(mock_oslo_log.called) diff --git a/ironic_inspector/test/unit/test_plugins_physnet_cidr_map.py b/ironic_inspector/test/unit/test_plugins_physnet_cidr_map.py index 0158e236f..5c27a00e3 100644 --- a/ironic_inspector/test/unit/test_plugins_physnet_cidr_map.py +++ b/ironic_inspector/test/unit/test_plugins_physnet_cidr_map.py @@ -100,14 +100,15 @@ class TestPhysnetCidrMapHook(test_base.NodeTest): self.assertRaises(utils.Error, self.hook.before_update, self.data, self.node_info) - @mock.patch('ironic_inspector.plugins.base_physnet.LOG') + @mock.patch('ironic_inspector.plugins.base_physnet.LOG.debug', + autospec=True) @mock.patch.object(node_cache.NodeInfo, 'patch_port', autospec=True) def test_interface_not_in_ironic(self, mock_patch, mock_log): cfg.CONF.set_override('cidr_map', '1.1.1.0/24:physnet_a', group='port_physnet') self.node_info._ports = {} self.hook.before_update(self.data, self.node_info) - self.assertTrue(mock_log.debug.called) + self.assertTrue(mock_log.called) @mock.patch.object(node_cache.NodeInfo, 'patch_port', autospec=True) def test_no_overwrite(self, mock_patch): @@ -122,7 +123,8 @@ class TestPhysnetCidrMapHook(test_base.NodeTest): self.hook.before_update(self.data, node_info) self.assertFalse(mock_patch.called) - @mock.patch('ironic_inspector.plugins.base_physnet.LOG') + @mock.patch('ironic_inspector.plugins.base_physnet.LOG.warning', + autospec=True) @mock.patch.object(node_cache.NodeInfo, 'patch_port', autospec=True) def test_patch_port_exception(self, mock_patch, mock_log): cfg.CONF.set_override('cidr_map', '1.1.1.0/24:physnet_a', @@ -130,8 +132,7 @@ class TestPhysnetCidrMapHook(test_base.NodeTest): mock_patch.side_effect = exceptions.BadRequestException('invalid data') self.hook.before_update(self.data, self.node_info) log_msg = "Failed to update port %(uuid)s: %(error)s" - mock_log.warning.assert_called_with(log_msg, mock.ANY, - node_info=mock.ANY) + mock_log.assert_called_with(log_msg, mock.ANY, node_info=mock.ANY) @mock.patch.object(node_cache.NodeInfo, 'patch_port', autospec=True) def test_no_ip_address_on_interface(self, mock_patch): diff --git a/ironic_inspector/test/unit/test_plugins_standard.py b/ironic_inspector/test/unit/test_plugins_standard.py index 4f3af5c30..b177d083e 100644 --- a/ironic_inspector/test/unit/test_plugins_standard.py +++ b/ironic_inspector/test/unit/test_plugins_standard.py @@ -176,7 +176,7 @@ class TestValidateInterfacesHookBeforeProcessing(test_base.NodeTest): sorted(self.data['macs'])) self.assertEqual(self.all_interfaces, self.data['all_interfaces']) - @mock.patch.object(node_cache.NodeInfo, 'create_ports') + @mock.patch.object(node_cache.NodeInfo, 'create_ports', autospec=True) def test_disabled_bad_conf(self, mock_create_port): CONF.set_override('add_ports', 'disabled', 'processing') CONF.set_override('keep_ports', 'added', 'processing') @@ -185,7 +185,7 @@ class TestValidateInterfacesHookBeforeProcessing(test_base.NodeTest): self.hook.__init__) mock_create_port.assert_not_called() - @mock.patch.object(node_cache.NodeInfo, 'create_ports') + @mock.patch.object(node_cache.NodeInfo, 'create_ports', autospec=True) def test_disabled(self, mock_create_port): CONF.set_override('add_ports', 'disabled', 'processing') CONF.set_override('keep_ports', 'all', 'processing') diff --git a/ironic_inspector/test/unit/test_process.py b/ironic_inspector/test/unit/test_process.py index b5d884bea..edfe948de 100644 --- a/ironic_inspector/test/unit/test_process.py +++ b/ironic_inspector/test/unit/test_process.py @@ -234,7 +234,8 @@ class TestProcess(BaseProcessTest): def test_hook_unexpected_exceptions(self): for ext in plugins_base.processing_hooks_manager(): patcher = mock.patch.object(ext.obj, 'before_processing', - side_effect=RuntimeError('boom')) + side_effect=RuntimeError('boom'), + autospec=True) patcher.start() self.addCleanup(lambda p=patcher: p.stop()) @@ -252,7 +253,8 @@ class TestProcess(BaseProcessTest): self.find_mock.side_effect = utils.Error('not found') for ext in plugins_base.processing_hooks_manager(): patcher = mock.patch.object(ext.obj, 'before_processing', - side_effect=RuntimeError('boom')) + side_effect=RuntimeError('boom'), + autospec=True) patcher.start() self.addCleanup(lambda p=patcher: p.stop()) @@ -756,7 +758,8 @@ class TestReapplyNode(BaseTest): swift_mock.get_object.return_value = json.dumps(self.data) with mock.patch.object(example_plugin.ExampleProcessingHook, - 'before_processing') as before_processing_mock: + 'before_processing', + autospec=True) as before_processing_mock: before_processing_mock.side_effect = exc self.call() diff --git a/tox.ini b/tox.ini index 90813c55b..4652f9c5c 100644 --- a/tox.ini +++ b/tox.ini @@ -70,8 +70,9 @@ max-complexity=15 # [H203] Use assertIs(Not)None to check for None. # [H204] Use assert(Not)Equal to check for equality. # [H205] Use assert(Greater|Less)(Equal) for comparison. +# [H210] Require ‘autospec’, ‘spec’, or ‘spec_set’ in mock.patch/mock.patch.object calls # [H904] Delay string interpolations at logging calls. -enable-extensions=H106,H203,H204,H205,H904 +enable-extensions=H106,H203,H204,H205,H210,H904 import-order-style = pep8 application-import-names = ironic_inspector