From fd179c33ccd57eb82eba3db18043bceb765d70d3 Mon Sep 17 00:00:00 2001 From: esberglu Date: Thu, 18 Jan 2018 11:33:19 -0600 Subject: [PATCH] Autospeccing: Tasks This adds autospeccing throughout the tasks tests. This will help catch errors in argument lists, object attributes, and other possible issues. This is the first of a series of patches for autospeccing throughout the project. Change-Id: I93719a3c9569e59f9b7443b7407f4c2ab026a919 --- .../tests/virt/powervm/tasks/test_image.py | 6 +- .../tests/virt/powervm/tasks/test_network.py | 56 ++++++++++--------- .../tests/virt/powervm/tasks/test_storage.py | 13 +++-- .../tests/virt/powervm/tasks/test_vm.py | 38 +++++++------ 4 files changed, 62 insertions(+), 51 deletions(-) diff --git a/nova_powervm/tests/virt/powervm/tasks/test_image.py b/nova_powervm/tests/virt/powervm/tasks/test_image.py index 8e74922e..a0511fbf 100644 --- a/nova_powervm/tests/virt/powervm/tasks/test_image.py +++ b/nova_powervm/tests/virt/powervm/tasks/test_image.py @@ -37,8 +37,10 @@ class TestImage(test.NoDBTestCase): expected_state='expected_state') tf.execute() - @mock.patch('nova_powervm.virt.powervm.image.stream_blockdev_to_glance') - @mock.patch('nova_powervm.virt.powervm.image.snapshot_metadata') + @mock.patch('nova_powervm.virt.powervm.image.stream_blockdev_to_glance', + autospec=True) + @mock.patch('nova_powervm.virt.powervm.image.snapshot_metadata', + autospec=True) def test_stream_to_glance(self, mock_metadata, mock_stream): mock_metadata.return_value = 'metadata' mock_inst = mock.Mock() diff --git a/nova_powervm/tests/virt/powervm/tasks/test_network.py b/nova_powervm/tests/virt/powervm/tasks/test_network.py index b58f9203..c9ad76db 100644 --- a/nova_powervm/tests/virt/powervm/tasks/test_network.py +++ b/nova_powervm/tests/virt/powervm/tasks/test_network.py @@ -46,8 +46,8 @@ class TestNetwork(test.NoDBTestCase): self.mock_lpar_wrap = mock.MagicMock() self.mock_lpar_wrap.can_modify_io.return_value = True, None - @mock.patch('nova_powervm.virt.powervm.vif.unplug') - @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') + @mock.patch('nova_powervm.virt.powervm.vif.unplug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas', autospec=True) def test_unplug_vifs(self, mock_vm_get, mock_unplug): """Tests that a delete of the vif can be done.""" inst = objects.Instance(**powervm.TEST_INSTANCE) @@ -97,9 +97,9 @@ class TestNetwork(test.NoDBTestCase): self.assertRaises(exception.VirtualInterfaceUnplugException, p_vifs.execute, self.mock_lpar_wrap) - @mock.patch('nova_powervm.virt.powervm.vif.plug') - @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') - @mock.patch('nova_powervm.virt.powervm.vm.get_vnics') + @mock.patch('nova_powervm.virt.powervm.vif.plug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_vnics', autospec=True) def test_plug_vifs_rmc(self, mock_vnic_get, mock_cna_get, mock_plug): """Tests that a crt vif can be done with secure RMC.""" inst = objects.Instance(**powervm.TEST_INSTANCE) @@ -147,8 +147,8 @@ class TestNetwork(test.NoDBTestCase): # created. self.assertEqual(pre_cnas + [mock_new_cna], all_cnas) - @mock.patch('nova_powervm.virt.powervm.vif.plug') - @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') + @mock.patch('nova_powervm.virt.powervm.vif.plug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas', autospec=True) def test_plug_vifs_rmc_no_create(self, mock_vm_get, mock_plug): """Verifies if no creates are needed, none are done.""" inst = objects.Instance(**powervm.TEST_INSTANCE) @@ -174,8 +174,8 @@ class TestNetwork(test.NoDBTestCase): self.apt, 'host_uuid', inst, net_info[1], 'slot_mgr', new_vif=False) - @mock.patch('nova_powervm.virt.powervm.vif.plug') - @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') + @mock.patch('nova_powervm.virt.powervm.vif.plug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas', autospec=True) def test_plug_vifs_invalid_state(self, mock_vm_get, mock_plug): """Tests that a crt_vif fails when the LPAR state is bad.""" inst = objects.Instance(**powervm.TEST_INSTANCE) @@ -196,8 +196,8 @@ class TestNetwork(test.NoDBTestCase): # The create should not have been invoked self.assertEqual(0, mock_plug.call_count) - @mock.patch('nova_powervm.virt.powervm.vif.plug') - @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') + @mock.patch('nova_powervm.virt.powervm.vif.plug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas', autospec=True) def test_plug_vifs_timeout(self, mock_vm_get, mock_plug): """Tests that crt vif failure via loss of neutron callback.""" inst = objects.Instance(**powervm.TEST_INSTANCE) @@ -220,8 +220,8 @@ class TestNetwork(test.NoDBTestCase): # The create should have only been called once. self.assertEqual(1, mock_plug.call_count) - @mock.patch('nova_powervm.virt.powervm.vif.plug') - @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') + @mock.patch('nova_powervm.virt.powervm.vif.plug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas', autospec=True) def test_plug_vifs_diff_host(self, mock_vm_get, mock_plug): """Tests that crt vif handles bad inst.host value.""" inst = powervm.TEST_INST1 @@ -247,8 +247,8 @@ class TestNetwork(test.NoDBTestCase): self.assertEqual(2, mock_inst_save.call_count) self.assertEqual('host1', inst.host) - @mock.patch('nova_powervm.virt.powervm.vif.plug') - @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') + @mock.patch('nova_powervm.virt.powervm.vif.plug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas', autospec=True) def test_plug_vifs_diff_host_except(self, mock_vm_get, mock_plug): """Tests that crt vif handles bad inst.host value. @@ -282,9 +282,9 @@ class TestNetwork(test.NoDBTestCase): self.assertEqual(2, mock_inst_save.call_count) self.assertEqual('host1', inst.host) - @mock.patch('nova_powervm.virt.powervm.vif.unplug') - @mock.patch('nova_powervm.virt.powervm.vif.plug') - @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') + @mock.patch('nova_powervm.virt.powervm.vif.unplug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vif.plug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas', autospec=True) def test_plug_vifs_revert(self, mock_vm_get, mock_plug, mock_unplug): """Tests that the revert flow works properly.""" inst = objects.Instance(**powervm.TEST_INSTANCE) @@ -321,10 +321,12 @@ class TestNetwork(test.NoDBTestCase): 'slot_mgr', cna_w_list=cna_list) mock_unplug.assert_has_calls([c2, c3]) - @mock.patch('nova_powervm.virt.powervm.vif.plug_secure_rmc_vif') - @mock.patch('nova_powervm.virt.powervm.vif.get_secure_rmc_vswitch') - @mock.patch('nova_powervm.virt.powervm.vif.plug') - @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') + @mock.patch('nova_powervm.virt.powervm.vif.plug_secure_rmc_vif', + autospec=True) + @mock.patch('nova_powervm.virt.powervm.vif.get_secure_rmc_vswitch', + autospec=True) + @mock.patch('nova_powervm.virt.powervm.vif.plug', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas', autospec=True) def test_plug_mgmt_vif(self, mock_vm_get, mock_plug, mock_get_rmc_vswitch, mock_plug_rmc_vif): """Tests that a mgmt vif can be created.""" @@ -341,7 +343,7 @@ class TestNetwork(test.NoDBTestCase): # With the default get_cnas mock (which returns a Mock()), we think we # found an existing management CNA. - mock_plug_rmc_vif.assert_not_called() + self.assertEqual(0, mock_plug_rmc_vif.call_count) mock_vm_get.assert_called_once_with( self.apt, inst, vswitch_uri='fake_mgmt_uri') @@ -363,8 +365,8 @@ class TestNetwork(test.NoDBTestCase): # Get wasn't called, since the CNAs were passed "from PlugVifs"; but # since the mgmt vif wasn't included, plug was called. - mock_vm_get.assert_not_called() - mock_plug_rmc_vif.assert_called() + self.assertEqual(0, mock_vm_get.call_count) + self.assertEqual(1, mock_plug_rmc_vif.call_count) # Finally, pass CNAs including the mgmt. cnas.append(mock.Mock(vswitch_uri='fake_mgmt_uri')) @@ -372,8 +374,8 @@ class TestNetwork(test.NoDBTestCase): p_vifs.execute(cnas) # Neither get nor plug was called. - mock_vm_get.assert_not_called() - mock_plug_rmc_vif.assert_not_called() + self.assertEqual(0, mock_vm_get.call_count) + self.assertEqual(0, mock_plug_rmc_vif.call_count) def test_get_vif_events(self): # Set up common mocks. diff --git a/nova_powervm/tests/virt/powervm/tasks/test_storage.py b/nova_powervm/tests/virt/powervm/tasks/test_storage.py index 9712ef6c..ebc2ff5e 100644 --- a/nova_powervm/tests/virt/powervm/tasks/test_storage.py +++ b/nova_powervm/tests/virt/powervm/tasks/test_storage.py @@ -80,7 +80,7 @@ class TestStorage(test.NoDBTestCase): task.revert(lpar_w, 'mgmt_cna', 'result', 'flow_failures') self.mock_mb.assert_not_called() - @mock.patch('nova_powervm.virt.powervm.vm.get_pvm_uuid') + @mock.patch('nova_powervm.virt.powervm.vm.get_pvm_uuid', autospec=True) def test_delete_vopt(self, mock_pvm_uuid): # Test with no FeedTask mock_pvm_uuid.return_value = 'pvm_uuid' @@ -146,9 +146,11 @@ class TestStorage(test.NoDBTestCase): task.revert('result', 'flow failures') self.disk_dvr.delete_disks.assert_called_once_with(['result']) - @mock.patch('pypowervm.tasks.scsi_mapper.find_maps') - @mock.patch('nova_powervm.virt.powervm.mgmt.discover_vscsi_disk') - @mock.patch('nova_powervm.virt.powervm.mgmt.remove_block_dev') + @mock.patch('pypowervm.tasks.scsi_mapper.find_maps', autospec=True) + @mock.patch('nova_powervm.virt.powervm.mgmt.discover_vscsi_disk', + autospec=True) + @mock.patch('nova_powervm.virt.powervm.mgmt.remove_block_dev', + autospec=True) def test_instance_disk_to_mgmt(self, mock_rm, mock_discover, mock_find): mock_discover.return_value = '/dev/disk' mock_instance = mock.Mock() @@ -252,7 +254,8 @@ class TestStorage(test.NoDBTestCase): self.assertEqual(0, disk_dvr.disconnect_disk_from_mgmt.call_count) self.assertEqual(0, mock_rm.call_count) - @mock.patch('nova_powervm.virt.powervm.mgmt.remove_block_dev') + @mock.patch('nova_powervm.virt.powervm.mgmt.remove_block_dev', + autospec=True) def test_remove_instance_disk_from_mgmt(self, mock_rm): disk_dvr = mock.MagicMock() mock_instance = mock.Mock() diff --git a/nova_powervm/tests/virt/powervm/tasks/test_vm.py b/nova_powervm/tests/virt/powervm/tasks/test_vm.py index 9edaf0a6..89c3c2e8 100644 --- a/nova_powervm/tests/virt/powervm/tasks/test_vm.py +++ b/nova_powervm/tests/virt/powervm/tasks/test_vm.py @@ -34,13 +34,16 @@ class TestVMTasks(test.NoDBTestCase): self.apt = mock.Mock() self.instance = mock.Mock(uuid='fake-uuid') - @mock.patch('pypowervm.utils.transaction.FeedTask') - @mock.patch('pypowervm.tasks.partition.build_active_vio_feed_task') - @mock.patch('pypowervm.tasks.storage.add_lpar_storage_scrub_tasks') - @mock.patch('nova_powervm.virt.powervm.vm.create_lpar') + @mock.patch('pypowervm.utils.transaction.FeedTask', autospec=True) + @mock.patch('pypowervm.tasks.partition.build_active_vio_feed_task', + autospec=True) + @mock.patch('pypowervm.tasks.storage.add_lpar_storage_scrub_tasks', + autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.create_lpar', autospec=True) def test_create(self, mock_vm_crt, mock_stg, mock_bld, mock_ftsk): nvram_mgr = mock.Mock() nvram_mgr.fetch.return_value = 'data' + mock_ftsk.name = 'vio_feed_task' lpar_entry = mock.Mock() # Test create with normal (non-recreate) ftsk @@ -69,9 +72,10 @@ class TestVMTasks(test.NoDBTestCase): rcrt.execute() mock_ftsk.execute.assert_called_once_with() - @mock.patch('nova_powervm.virt.powervm.vm.get_pvm_uuid') - @mock.patch('nova_powervm.virt.powervm.tasks.vm.Create.execute') - @mock.patch('nova_powervm.virt.powervm.vm.delete_lpar') + @mock.patch('nova_powervm.virt.powervm.vm.get_pvm_uuid', autospec=True) + @mock.patch('nova_powervm.virt.powervm.tasks.vm.Create.execute', + autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.delete_lpar', autospec=True) def test_create_revert(self, mock_vm_dlt, mock_crt_exc, mock_get_pvm_uuid): @@ -84,23 +88,23 @@ class TestVMTasks(test.NoDBTestCase): flow_test = tf_lf.Flow("test_revert") flow_test.add(crt) self.assertRaises(exception.NovaException, tf_eng.run, flow_test) - mock_vm_dlt.assert_not_called() + self.assertEqual(0, mock_vm_dlt.call_count) # Assert that a failure when rebuild results in revert crt.instance.task_state = task_states.REBUILD_SPAWNING flow_test = tf_lf.Flow("test_revert") flow_test.add(crt) self.assertRaises(exception.NovaException, tf_eng.run, flow_test) - mock_vm_dlt.assert_called() + self.assertEqual(1, mock_vm_dlt.call_count) - @mock.patch('nova_powervm.virt.powervm.vm.power_on') + @mock.patch('nova_powervm.virt.powervm.vm.power_on', autospec=True) def test_power_on(self, mock_pwron): pwron = tf_vm.PowerOn(self.apt, self.instance, pwr_opts='opt') pwron.execute() mock_pwron.assert_called_once_with(self.apt, self.instance, opts='opt') - @mock.patch('nova_powervm.virt.powervm.vm.power_on') - @mock.patch('nova_powervm.virt.powervm.vm.power_off') + @mock.patch('nova_powervm.virt.powervm.vm.power_on', autospec=True) + @mock.patch('nova_powervm.virt.powervm.vm.power_off', autospec=True) def test_power_on_revert(self, mock_pwroff, mock_pwron): flow = tf_lf.Flow('revert_power_on') pwron = tf_vm.PowerOn(self.apt, self.instance, pwr_opts='opt') @@ -124,9 +128,9 @@ class TestVMTasks(test.NoDBTestCase): mock_pwron.side_effect = exception.NovaException() self.assertRaises(exception.NovaException, tf_eng.run, flow) mock_pwron.assert_called_once_with(self.apt, self.instance, opts='opt') - mock_pwroff.assert_not_called() + self.assertEqual(0, mock_pwroff.call_count) - @mock.patch('nova_powervm.virt.powervm.vm.power_off') + @mock.patch('nova_powervm.virt.powervm.vm.power_off', autospec=True) def test_power_off(self, mock_pwroff): # Default force_immediate pwroff = tf_vm.PowerOff(self.apt, self.instance) @@ -142,13 +146,13 @@ class TestVMTasks(test.NoDBTestCase): mock_pwroff.assert_called_once_with(self.apt, self.instance, force_immediate=True) - @mock.patch('nova_powervm.virt.powervm.vm.delete_lpar') + @mock.patch('nova_powervm.virt.powervm.vm.delete_lpar', autospec=True) def test_delete(self, mock_dlt): delete = tf_vm.Delete(self.apt, self.instance) delete.execute() mock_dlt.assert_called_once_with(self.apt, self.instance) - @mock.patch('nova_powervm.virt.powervm.vm.update') + @mock.patch('nova_powervm.virt.powervm.vm.update', autospec=True) def test_resize(self, mock_vm_update): resize = tf_vm.Resize(self.apt, 'host_wrapper', self.instance, @@ -160,7 +164,7 @@ class TestVMTasks(test.NoDBTestCase): name='new_name') self.assertEqual('resized_entry', resized_entry) - @mock.patch('nova_powervm.virt.powervm.vm.rename') + @mock.patch('nova_powervm.virt.powervm.vm.rename', autospec=True) def test_rename(self, mock_vm_rename): mock_vm_rename.return_value = 'new_entry' rename = tf_vm.Rename(self.apt, self.instance, 'new_name')