From 071d30718940e625570736375d27b252942cf518 Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Sun, 19 Mar 2017 10:56:13 -0700 Subject: [PATCH] Add missing 'autospec' statements to unit test mocks Add missing 'autospec' statements to unit test mocks. This helps to make sure we don't accidentally call invalid assert methods. Change-Id: I54cf353a2ebd63cfc41cfaa0379a96515f39edd7 --- ironicclient/tests/unit/common/test_utils.py | 8 ++++---- ironicclient/tests/unit/test_client.py | 6 +++--- ironicclient/tests/unit/test_exc.py | 2 +- ironicclient/tests/unit/test_shell.py | 16 ++++++++-------- .../tests/unit/v1/test_create_resources.py | 2 +- .../tests/unit/v1/test_create_resources_shell.py | 2 +- ironicclient/tests/unit/v1/test_driver.py | 14 +++++++------- ironicclient/tests/unit/v1/test_driver_shell.py | 6 +++--- ironicclient/tests/unit/v1/test_node.py | 16 ++++++++-------- 9 files changed, 36 insertions(+), 36 deletions(-) diff --git a/ironicclient/tests/unit/common/test_utils.py b/ironicclient/tests/unit/common/test_utils.py index 2fc07e5c9..6052470ca 100644 --- a/ironicclient/tests/unit/common/test_utils.py +++ b/ironicclient/tests/unit/common/test_utils.py @@ -230,7 +230,7 @@ class CommonFiltersTest(test_utils.BaseTestCase): self.assertEqual(['fields=a,b,c'], result) -@mock.patch.object(subprocess, 'Popen') +@mock.patch.object(subprocess, 'Popen', autospec=True) class MakeConfigDriveTest(test_utils.BaseTestCase): def setUp(self): @@ -256,14 +256,14 @@ class MakeConfigDriveTest(test_utils.BaseTestCase): stdout=subprocess.PIPE) fake_process.communicate.assert_called_once_with() - @mock.patch.object(os, 'access') + @mock.patch.object(os, 'access', autospec=True) def test_make_configdrive_non_readable_dir(self, mock_access, mock_popen): mock_access.return_value = False self.assertRaises(exc.CommandError, utils.make_configdrive, 'fake-dir') mock_access.assert_called_once_with('fake-dir', os.R_OK) self.assertFalse(mock_popen.called) - @mock.patch.object(os, 'access') + @mock.patch.object(os, 'access', autospec=True) def test_make_configdrive_oserror(self, mock_access, mock_popen): mock_access.return_value = True mock_popen.side_effect = OSError('boom') @@ -274,7 +274,7 @@ class MakeConfigDriveTest(test_utils.BaseTestCase): stderr=subprocess.PIPE, stdout=subprocess.PIPE) - @mock.patch.object(os, 'access') + @mock.patch.object(os, 'access', autospec=True) def test_make_configdrive_non_zero_returncode(self, mock_access, mock_popen): fake_process = mock.Mock(returncode=123) diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index b601037be..8716a81fd 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/ironicclient/tests/unit/test_client.py @@ -281,7 +281,7 @@ class ClientTest(utils.BaseTestCase): self._test_loader_arguments_passed_correctly( passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs) - @mock.patch.object(iroclient, 'Client') + @mock.patch.object(iroclient, 'Client', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) def test_correct_arguments_passed_to_client_constructor_noauth_mode( self, mock_ks_session, mock_client): @@ -312,7 +312,7 @@ class ClientTest(utils.BaseTestCase): ) self.assertFalse(mock_ks_session.called) - @mock.patch.object(iroclient, 'Client') + @mock.patch.object(iroclient, 'Client', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) def test_correct_arguments_passed_to_client_constructor_session_created( self, mock_ks_session, mock_client): @@ -337,7 +337,7 @@ class ClientTest(utils.BaseTestCase): } ) - @mock.patch.object(iroclient, 'Client') + @mock.patch.object(iroclient, 'Client', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) def test_correct_arguments_passed_to_client_constructor_session_passed( self, mock_ks_session, mock_client): diff --git a/ironicclient/tests/unit/test_exc.py b/ironicclient/tests/unit/test_exc.py index 02a1a549a..d7c838263 100644 --- a/ironicclient/tests/unit/test_exc.py +++ b/ironicclient/tests/unit/test_exc.py @@ -21,7 +21,7 @@ from ironicclient import exc from ironicclient.tests.unit import utils as test_utils -@mock.patch.object(exceptions, 'from_response') +@mock.patch.object(exceptions, 'from_response', autospec=True) class ExcTest(test_utils.BaseTestCase): def setUp(self): diff --git a/ironicclient/tests/unit/test_shell.py b/ironicclient/tests/unit/test_shell.py index 42baaba05..05b55513f 100644 --- a/ironicclient/tests/unit/test_shell.py +++ b/ironicclient/tests/unit/test_shell.py @@ -152,10 +152,10 @@ class ShellTest(utils.BaseTestCase): self.make_env(exclude='OS_USERNAME') self.test_help() - @mock.patch.object(client, 'get_client', + @mock.patch.object(client, 'get_client', autospec=True, side_effect=keystone_exc.ConnectFailure) - @mock.patch('sys.stdin', side_effect=mock.MagicMock) - @mock.patch('getpass.getpass', return_value='password') + @mock.patch('sys.stdin', side_effect=mock.MagicMock, autospec=True) + @mock.patch('getpass.getpass', return_value='password', autospec=True) def test_password_prompted(self, mock_getpass, mock_stdin, mock_client): self.make_env(exclude='OS_PASSWORD') # We will get a ConnectFailure because there is no keystone. @@ -179,9 +179,9 @@ class ShellTest(utils.BaseTestCase): # Make sure we are actually prompted. mock_getpass.assert_called_with('OpenStack Password: ') - @mock.patch.object(client, 'get_client', + @mock.patch.object(client, 'get_client', autospec=True, side_effect=keystone_exc.ConnectFailure) - @mock.patch('getpass.getpass', return_value='password') + @mock.patch('getpass.getpass', return_value='password', autospec=True) def test_token_auth(self, mock_getpass, mock_client): self.make_env(environ_dict=FAKE_ENV_KEYSTONE_V2_TOKEN) # We will get a ConnectFailure because there is no keystone. @@ -206,8 +206,8 @@ class ShellTest(utils.BaseTestCase): mock_client.assert_called_once_with(1, **expected_kwargs) self.assertFalse(mock_getpass.called) - @mock.patch('sys.stdin', side_effect=mock.MagicMock) - @mock.patch('getpass.getpass', side_effect=EOFError) + @mock.patch('sys.stdin', side_effect=mock.MagicMock, autospec=True) + @mock.patch('getpass.getpass', side_effect=EOFError, autospec=True) def test_password_prompted_ctrlD(self, mock_getpass, mock_stdin): self.make_env(exclude='OS_PASSWORD') # We should get Command Error because we mock Ctl-D. @@ -216,7 +216,7 @@ class ShellTest(utils.BaseTestCase): # Make sure we are actually prompted. mock_getpass.assert_called_with('OpenStack Password: ') - @mock.patch('sys.stdin') + @mock.patch('sys.stdin', autospec=True) def test_no_password_no_tty(self, mock_stdin): # delete the isatty attribute so that we do not get # prompted when manually running the tests diff --git a/ironicclient/tests/unit/v1/test_create_resources.py b/ironicclient/tests/unit/v1/test_create_resources.py index 7bd92d964..c27cd5f8b 100644 --- a/ironicclient/tests/unit/v1/test_create_resources.py +++ b/ironicclient/tests/unit/v1/test_create_resources.py @@ -200,7 +200,7 @@ class LoadFromFileTest(utils.BaseTestCase): 'must have .json or .yaml extension', create_resources.load_from_file, fname) - @mock.patch.object(__builtin__, 'open') + @mock.patch.object(__builtin__, 'open', autospec=True) def test_load_ioerror(self, mock_open): mock_open.side_effect = IOError('file does not exist') fname = 'abc.json' diff --git a/ironicclient/tests/unit/v1/test_create_resources_shell.py b/ironicclient/tests/unit/v1/test_create_resources_shell.py index 24a5b9fa8..87a93206f 100644 --- a/ironicclient/tests/unit/v1/test_create_resources_shell.py +++ b/ironicclient/tests/unit/v1/test_create_resources_shell.py @@ -22,7 +22,7 @@ class TestCreateResourcesShell(utils.BaseTestCase): super(TestCreateResourcesShell, self).setUp() self.client = mock.MagicMock(autospec=True) - @mock.patch.object(create_resources, 'create_resources') + @mock.patch.object(create_resources, 'create_resources', autospec=True) def test_create_shell(self, mock_create_resources): args = mock.MagicMock() files = ['file1', 'file2', 'file3'] diff --git a/ironicclient/tests/unit/v1/test_driver.py b/ironicclient/tests/unit/v1/test_driver.py index 995f49db8..17cb1763f 100644 --- a/ironicclient/tests/unit/v1/test_driver.py +++ b/ironicclient/tests/unit/v1/test_driver.py @@ -131,7 +131,7 @@ class DriverManagerTest(testtools.TestCase): '/v1/drivers/%s/raid/logical_disk_properties' % DRIVER2['name']) self.assertEqual({}, properties) - @mock.patch.object(driver.DriverManager, 'update') + @mock.patch.object(driver.DriverManager, 'update', autospec=True) def test_vendor_passthru_update(self, update_mock): # For now just mock the tests because vendor-passthru doesn't return # anything to verify. @@ -146,12 +146,12 @@ class DriverManagerTest(testtools.TestCase): for http_method in ('POST', 'PUT', 'PATCH'): kwargs['http_method'] = http_method self.mgr.vendor_passthru(**kwargs) - update_mock.assert_called_once_with(final_path, + update_mock.assert_called_once_with(mock.ANY, final_path, vendor_passthru_args, http_method=http_method) update_mock.reset_mock() - @mock.patch.object(driver.DriverManager, 'get') + @mock.patch.object(driver.DriverManager, 'get', autospec=True) def test_vendor_passthru_get(self, get_mock): kwargs = { 'driver_name': 'driver_name', @@ -161,9 +161,9 @@ class DriverManagerTest(testtools.TestCase): final_path = 'driver_name/vendor_passthru/method' self.mgr.vendor_passthru(**kwargs) - get_mock.assert_called_once_with(final_path) + get_mock.assert_called_once_with(mock.ANY, final_path) - @mock.patch.object(driver.DriverManager, 'delete') + @mock.patch.object(driver.DriverManager, 'delete', autospec=True) def test_vendor_passthru_delete(self, delete_mock): kwargs = { 'driver_name': 'driver_name', @@ -173,9 +173,9 @@ class DriverManagerTest(testtools.TestCase): final_path = 'driver_name/vendor_passthru/method' self.mgr.vendor_passthru(**kwargs) - delete_mock.assert_called_once_with(final_path) + delete_mock.assert_called_once_with(mock.ANY, final_path) - @mock.patch.object(driver.DriverManager, 'delete') + @mock.patch.object(driver.DriverManager, 'delete', autospec=True) def test_vendor_passthru_unknown_http_method(self, delete_mock): kwargs = { 'driver_name': 'driver_name', diff --git a/ironicclient/tests/unit/v1/test_driver_shell.py b/ironicclient/tests/unit/v1/test_driver_shell.py index 5f9a0f5f6..e64beb294 100644 --- a/ironicclient/tests/unit/v1/test_driver_shell.py +++ b/ironicclient/tests/unit/v1/test_driver_shell.py @@ -73,7 +73,7 @@ class DriverShellTest(utils.BaseTestCase): d_shell.do_driver_properties(client_mock, args) client_mock.driver.properties.assert_called_once_with("driver_name") - @mock.patch('ironicclient.common.cliutils.print_dict') + @mock.patch('ironicclient.common.cliutils.print_dict', autospec=True) def test_do_driver_properties_with_wrap_default(self, mock_print_dict): client_mock = self.client_mock client_mock.driver.properties.return_value = { @@ -91,7 +91,7 @@ class DriverShellTest(utils.BaseTestCase): json_flag=False, wrap=0) - @mock.patch('ironicclient.common.cliutils.print_dict') + @mock.patch('ironicclient.common.cliutils.print_dict', autospec=True) def test_do_driver_properties_with_wrap(self, mock_print_dict): client_mock = self.client_mock client_mock.driver.properties.return_value = { @@ -109,7 +109,7 @@ class DriverShellTest(utils.BaseTestCase): json_flag=False, wrap=80) - @mock.patch('ironicclient.common.cliutils.print_dict') + @mock.patch('ironicclient.common.cliutils.print_dict', autospec=True) def _test_do_driver_raid_logical_disk(self, print_dict_mock, wrap=0): cli_mock = self.client_mock cli_mock.driver.raid_logical_disk_properties.return_value = { diff --git a/ironicclient/tests/unit/v1/test_node.py b/ironicclient/tests/unit/v1/test_node.py index 5f4583d97..b84189d47 100644 --- a/ironicclient/tests/unit/v1/test_node.py +++ b/ironicclient/tests/unit/v1/test_node.py @@ -954,7 +954,7 @@ class NodeManagerTest(testtools.TestCase): ] self.assertEqual(expect, self.api.calls) - @mock.patch.object(common_utils, 'make_configdrive') + @mock.patch.object(common_utils, 'make_configdrive', autospec=True) def test_node_set_provision_state_with_configdrive_dir(self, mock_configdrive): mock_configdrive.return_value = 'fake-configdrive' @@ -1022,7 +1022,7 @@ class NodeManagerTest(testtools.TestCase): self.assertEqual(expect, self.api.calls) self.assertEqual(CONSOLE_DATA_DISABLED, info) - @mock.patch.object(node.NodeManager, 'update') + @mock.patch.object(node.NodeManager, 'update', autospec=True) def test_vendor_passthru_update(self, update_mock): # For now just mock the tests because vendor-passthru doesn't return # anything to verify. @@ -1037,12 +1037,12 @@ class NodeManagerTest(testtools.TestCase): for http_method in ('POST', 'PUT', 'PATCH'): kwargs['http_method'] = http_method self.mgr.vendor_passthru(**kwargs) - update_mock.assert_called_once_with(final_path, + update_mock.assert_called_once_with(mock.ANY, final_path, vendor_passthru_args, http_method=http_method) update_mock.reset_mock() - @mock.patch.object(node.NodeManager, 'get') + @mock.patch.object(node.NodeManager, 'get', autospec=True) def test_vendor_passthru_get(self, get_mock): kwargs = { 'node_id': 'node_uuid', @@ -1052,9 +1052,9 @@ class NodeManagerTest(testtools.TestCase): final_path = 'node_uuid/vendor_passthru/method' self.mgr.vendor_passthru(**kwargs) - get_mock.assert_called_once_with(final_path) + get_mock.assert_called_once_with(mock.ANY, final_path) - @mock.patch.object(node.NodeManager, 'delete') + @mock.patch.object(node.NodeManager, 'delete', autospec=True) def test_vendor_passthru_delete(self, delete_mock): kwargs = { 'node_id': 'node_uuid', @@ -1064,9 +1064,9 @@ class NodeManagerTest(testtools.TestCase): final_path = 'node_uuid/vendor_passthru/method' self.mgr.vendor_passthru(**kwargs) - delete_mock.assert_called_once_with(final_path) + delete_mock.assert_called_once_with(mock.ANY, final_path) - @mock.patch.object(node.NodeManager, 'delete') + @mock.patch.object(node.NodeManager, 'delete', autospec=True) def test_vendor_passthru_unknown_http_method(self, delete_mock): kwargs = { 'node_id': 'node_uuid',