From 90acdf43ea5c3d29f0ace5c297b1df914ec9282d Mon Sep 17 00:00:00 2001 From: Chris Krelle Date: Fri, 20 Mar 2015 12:25:11 -0700 Subject: [PATCH] fix invalid asserts in tests While looking at some of our tests I noticed a invalid assert, after some greping I found others, this patch corrects them. Change-Id: Ide6ad4c2b7e20ce9658ad24f229df90cb9e6a75c --- ironic/tests/api/v1/test_root.py | 4 +++- ironic/tests/conductor/test_manager.py | 13 ++++++------- ironic/tests/drivers/ilo/test_deploy.py | 2 +- ironic/tests/drivers/ilo/test_inspect.py | 6 +++--- ironic/tests/drivers/test_agent.py | 6 +++--- ironic/tests/drivers/test_agent_base_vendor.py | 2 +- ironic/tests/drivers/test_ssh.py | 2 +- ironic/tests/test_images.py | 10 +++++++--- 8 files changed, 25 insertions(+), 20 deletions(-) diff --git a/ironic/tests/api/v1/test_root.py b/ironic/tests/api/v1/test_root.py index 7228663f16..35913e60be 100644 --- a/ironic/tests/api/v1/test_root.py +++ b/ironic/tests/api/v1/test_root.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import mock from webob import exc as webob_exc from ironic.api.controllers import v1 as v1_api @@ -25,7 +26,8 @@ class TestV1Routing(api_base.FunctionalTest): def test_route_checks_version(self): self.get_json('/') - self._check_version.assert_called_once() + self._check_version.assert_called_once_with(mock.ANY, + mock.ANY) class TestCheckVersions(test_base.TestCase): diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index e2dd065681..8817668968 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/ironic/tests/conductor/test_manager.py @@ -1696,7 +1696,7 @@ class DoNodeCleanTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): node.refresh() # Assert that the node was moved to available without cleaning - mock_validate.assert_not_called() + self.assertFalse(mock_validate.called) self.assertEqual(states.AVAILABLE, node.provision_state) self.assertEqual(states.NOSTATE, node.target_provision_state) self.assertEqual({}, node.clean_step) @@ -1725,9 +1725,9 @@ class DoNodeCleanTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): self.service._worker_pool.waitall() node.refresh() - mock_validate.assert_called_once() + mock_validate.assert_called_once_with(task) mock_next_step.assert_called_once_with(mock.ANY, [], {}) - mock_steps.assert_called_once() + mock_steps.assert_called_once_with(task) # Check that state didn't change self.assertEqual(states.CLEANING, node.provision_state) @@ -1806,7 +1806,7 @@ class DoNodeCleanTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): # Cleaning should be complete without calling additional steps self.assertEqual(states.AVAILABLE, node.provision_state) self.assertEqual({}, node.clean_step) - mock_execute.assert_not_called() + self.assertFalse(mock_execute.called) @mock.patch('ironic.drivers.modules.fake.FakePower.execute_clean_step') @mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_clean_step') @@ -1869,7 +1869,7 @@ class DoNodeCleanTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): self.assertEqual({}, node.clean_step) self.assertIsNotNone(node.last_error) self.assertTrue(node.maintenance) - mock_execute.assert_not_called() + self.assertFalse(mock_execute.called) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_clean_step') def test__do_next_clean_step_fail(self, mock_execute): @@ -1897,7 +1897,6 @@ class DoNodeCleanTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): self.assertEqual({}, node.clean_step) self.assertIsNotNone(node.last_error) self.assertTrue(node.maintenance) - mock_execute.assert_not_called() mock_execute.assert_called_once_with(mock.ANY, self.clean_steps[0]) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_clean_step') @@ -1923,7 +1922,7 @@ class DoNodeCleanTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): # Cleaning should be complete without calling additional steps self.assertEqual(states.AVAILABLE, node.provision_state) self.assertEqual({}, node.clean_step) - mock_execute.assert_not_called() + self.assertFalse(mock_execute.called) @mock.patch('ironic.drivers.modules.fake.FakePower.execute_clean_step') @mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_clean_step') diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py index c0c15ebefd..c7f3fbe5fc 100644 --- a/ironic/tests/drivers/ilo/test_deploy.py +++ b/ironic/tests/drivers/ilo/test_deploy.py @@ -143,7 +143,7 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): image_props_mock.assert_called_once_with( task.context, 'image-uuid', ['boot_iso', 'kernel_id', 'ramdisk_id']) - get_node_cap_mock.assert_not_called(task.node, 'boot_mode') + self.assertFalse(get_node_cap_mock.called) self.assertIsNone(boot_iso_result) @mock.patch.object(tempfile, 'NamedTemporaryFile') diff --git a/ironic/tests/drivers/ilo/test_inspect.py b/ironic/tests/drivers/ilo/test_inspect.py index 129f52b0fc..05997cd1f3 100644 --- a/ironic/tests/drivers/ilo/test_inspect.py +++ b/ironic/tests/drivers/ilo/test_inspect.py @@ -327,7 +327,7 @@ class IloInspectTestCase(db_base.DbTestCase): get_essential_mock.assert_called_once_with(task.node, ilo_object_mock) self.assertEqual(task.node.properties, result['properties']) - create_port_mock.assert_not_called() + self.assertFalse(create_port_mock.called) class TestInspectPrivateMethods(db_base.DbTestCase): @@ -347,7 +347,7 @@ class TestInspectPrivateMethods(db_base.DbTestCase): port_dict1 = {'address': 'aa:aa:aa:aa:aa:aa', 'node_id': node_id} port_dict2 = {'address': 'bb:bb:bb:bb:bb:bb', 'node_id': node_id} ilo_inspect._create_ports_if_not_exist(self.node, macs) - instance_mock.assert_called_once() + instance_mock.assert_called_once_with() self.assertTrue(log_mock.called) db_obj.create_port.assert_any_call(port_dict1) db_obj.create_port.assert_any_call(port_dict2) @@ -361,7 +361,7 @@ class TestInspectPrivateMethods(db_base.DbTestCase): dbapi_mock.create_port.side_effect = exception.MACAlreadyExists('f') macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'} ilo_inspect._create_ports_if_not_exist(self.node, macs) - instance_mock.assert_called_once() + instance_mock.assert_called_once_with() self.assertTrue(log_mock.called) def test__get_essential_properties_ok(self): diff --git a/ironic/tests/drivers/test_agent.py b/ironic/tests/drivers/test_agent.py index 7e5ed3133a..c72998a7f1 100644 --- a/ironic/tests/drivers/test_agent.py +++ b/ironic/tests/drivers/test_agent.py @@ -226,8 +226,8 @@ class TestAgentDeploy(db_base.DbTestCase): self.driver.prepare_cleaning(task)) prepare_mock.assert_called_once_with(task) boot_mock.assert_called_once_with(task, ports) - create_mock.assert_called_once() - delete_mock.assert_called_once() + create_mock.assert_called_once_with(task) + delete_mock.assert_called_once_with(task) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports') @mock.patch('ironic.drivers.modules.agent._clean_up_pxe') @@ -238,7 +238,7 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertIsNone(self.driver.tear_down_cleaning(task)) power_mock.assert_called_once_with(task, states.POWER_OFF) cleanup_mock.assert_called_once_with(task) - neutron_mock.assert_called_once() + neutron_mock.assert_called_once_with(task) @mock.patch('ironic.drivers.modules.deploy_utils.agent_get_clean_steps') def test_get_clean_steps(self, mock_get_clean_steps): diff --git a/ironic/tests/drivers/test_agent_base_vendor.py b/ironic/tests/drivers/test_agent_base_vendor.py index 31596abc13..d1727fa496 100644 --- a/ironic/tests/drivers/test_agent_base_vendor.py +++ b/ironic/tests/drivers/test_agent_base_vendor.py @@ -424,7 +424,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: self.passthru.continue_cleaning(task) - notify_mock.assert_not_called() + self.assertFalse(notify_mock.called) @mock.patch('ironic.conductor.manager.cleaning_error_handler') @mock.patch.object(agent_client.AgentClient, 'get_commands_status') diff --git a/ironic/tests/drivers/test_ssh.py b/ironic/tests/drivers/test_ssh.py index f70e213093..76e0236750 100644 --- a/ironic/tests/drivers/test_ssh.py +++ b/ironic/tests/drivers/test_ssh.py @@ -283,7 +283,7 @@ class SSHPrivateMethodsTestCase(db_base.DbTestCase): info) get_hosts_name_mock.assert_called_once_with(self.sshclient, info) - exec_ssh_mock.assert_not_called() + self.assertFalse(exec_ssh_mock.called) @mock.patch.object(processutils, 'ssh_execute') def test__get_power_status_exception(self, exec_ssh_mock): diff --git a/ironic/tests/test_images.py b/ironic/tests/test_images.py index 9ebd2d4b81..bd189ad447 100644 --- a/ironic/tests/test_images.py +++ b/ironic/tests/test_images.py @@ -679,7 +679,7 @@ class FsImageTestCase(base.TestCase): 'tgt_file', 'path/to/deployiso', 'path/to/kernel', 'path/to/ramdisk') - umount_mock.assert_called_once_wth('mountdir') + umount_mock.assert_called_once_with('mountdir') @mock.patch.object(images, '_create_root_fs') @mock.patch.object(utils, 'write_to_file') @@ -744,8 +744,12 @@ class FsImageTestCase(base.TestCase): 'tmpdir/kernel-uuid') fetch_images_mock.assert_any_call('ctx', 'ramdisk-uuid', 'tmpdir/ramdisk-uuid') - fetch_images_mock.assert_not_called_with('ctx', 'deploy_iso-uuid', - 'tmpdir/deploy_iso-uuid') + # Note (NobodyCam): the orginal assert asserted that fetch_images + # was not called with parameters, this did not + # work, So I instead assert that there were only + # Two calls to the mock validating the above + # asserts. + self.assertEqual(2, fetch_images_mock.call_count) params = ['root=UUID=root-uuid', 'kernel-params'] create_isolinux_mock.assert_called_once_with('output_file',