flake8: Enable some off-by-default checks

Enable the following off-by-default checks:
    * [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

Fix code that failed [H205] and [H210]

Change-Id: I7a553e7d40e328f37757fb504a098776cb9bf97c
This commit is contained in:
John L. Villalovos 2017-09-06 14:01:19 -07:00
parent 36e0ff793e
commit f94722ab44
5 changed files with 25 additions and 19 deletions

View File

@ -52,5 +52,5 @@ class DriverSanityTestIronicClient(base.FunctionalTestBase):
""" """
driver = 'fake' driver = 'fake'
available_drivers = self.get_drivers_names() available_drivers = self.get_drivers_names()
self.assertTrue(len(available_drivers) > 0) self.assertGreater(len(available_drivers), 0)
self.assertIn(driver, available_drivers) self.assertIn(driver, available_drivers)

View File

@ -78,7 +78,7 @@ class MakeClientTest(testtools.TestCase):
class BuildOptionParserTest(testtools.TestCase): class BuildOptionParserTest(testtools.TestCase):
@mock.patch.object(plugin.utils, 'env', lambda x: None) @mock.patch.object(plugin.utils, 'env', lambda x: None)
@mock.patch.object(argparse.ArgumentParser, 'add_argument') @mock.patch.object(argparse.ArgumentParser, 'add_argument', autospec=True)
def test_build_option_parser(self, mock_add_argument): def test_build_option_parser(self, mock_add_argument):
parser = argparse.ArgumentParser() parser = argparse.ArgumentParser()
mock_add_argument.reset_mock() mock_add_argument.reset_mock()
@ -86,12 +86,13 @@ class BuildOptionParserTest(testtools.TestCase):
version_list = ['1'] + ['1.%d' % i for i in range( version_list = ['1'] + ['1.%d' % i for i in range(
1, plugin.LAST_KNOWN_API_VERSION + 1)] + ['latest'] 1, plugin.LAST_KNOWN_API_VERSION + 1)] + ['latest']
mock_add_argument.assert_called_once_with( mock_add_argument.assert_called_once_with(
'--os-baremetal-api-version', action=plugin.ReplaceLatestVersion, mock.ANY, '--os-baremetal-api-version',
choices=version_list, default=http.DEFAULT_VER, help=mock.ANY, action=plugin.ReplaceLatestVersion, choices=version_list,
default=http.DEFAULT_VER, help=mock.ANY,
metavar='<baremetal-api-version>') metavar='<baremetal-api-version>')
@mock.patch.object(plugin.utils, 'env', lambda x: "latest") @mock.patch.object(plugin.utils, 'env', lambda x: "latest")
@mock.patch.object(argparse.ArgumentParser, 'add_argument') @mock.patch.object(argparse.ArgumentParser, 'add_argument', autospec=True)
def test_build_option_parser_env_latest(self, mock_add_argument): def test_build_option_parser_env_latest(self, mock_add_argument):
parser = argparse.ArgumentParser() parser = argparse.ArgumentParser()
mock_add_argument.reset_mock() mock_add_argument.reset_mock()
@ -99,11 +100,12 @@ class BuildOptionParserTest(testtools.TestCase):
version_list = ['1'] + ['1.%d' % i for i in range( version_list = ['1'] + ['1.%d' % i for i in range(
1, plugin.LAST_KNOWN_API_VERSION + 1)] + ['latest'] 1, plugin.LAST_KNOWN_API_VERSION + 1)] + ['latest']
mock_add_argument.assert_called_once_with( mock_add_argument.assert_called_once_with(
'--os-baremetal-api-version', action=plugin.ReplaceLatestVersion, mock.ANY, '--os-baremetal-api-version',
choices=version_list, default=plugin.LATEST_VERSION, help=mock.ANY, action=plugin.ReplaceLatestVersion, choices=version_list,
default=plugin.LATEST_VERSION, help=mock.ANY,
metavar='<baremetal-api-version>') metavar='<baremetal-api-version>')
@mock.patch.object(plugin.utils, 'env') @mock.patch.object(plugin.utils, 'env', autospec=True)
def test__get_environment_version(self, mock_utils_env): def test__get_environment_version(self, mock_utils_env):
mock_utils_env.return_value = 'latest' mock_utils_env.return_value = 'latest'
result = plugin._get_environment_version(None) result = plugin._get_environment_version(None)

View File

@ -1419,7 +1419,7 @@ class NodeManagerTest(testtools.TestCase):
self.assertRaises(exc.InvalidAttribute, self.mgr.vendor_passthru, self.assertRaises(exc.InvalidAttribute, self.mgr.vendor_passthru,
**kwargs) **kwargs)
@mock.patch.object(node.NodeManager, '_list') @mock.patch.object(node.NodeManager, '_list', autospec=True)
def test_vif_list(self, _list_mock): def test_vif_list(self, _list_mock):
kwargs = { kwargs = {
'node_ident': NODE1['uuid'], 'node_ident': NODE1['uuid'],
@ -1427,9 +1427,9 @@ class NodeManagerTest(testtools.TestCase):
final_path = '/v1/nodes/%s/vifs' % NODE1['uuid'] final_path = '/v1/nodes/%s/vifs' % NODE1['uuid']
self.mgr.vif_list(**kwargs) self.mgr.vif_list(**kwargs)
_list_mock.assert_called_once_with(final_path, "vifs") _list_mock.assert_called_once_with(mock.ANY, final_path, "vifs")
@mock.patch.object(node.NodeManager, 'update') @mock.patch.object(node.NodeManager, 'update', autospec=True)
def test_vif_attach(self, update_mock): def test_vif_attach(self, update_mock):
kwargs = { kwargs = {
'node_ident': NODE1['uuid'], 'node_ident': NODE1['uuid'],
@ -1438,10 +1438,10 @@ class NodeManagerTest(testtools.TestCase):
final_path = '%s/vifs' % NODE1['uuid'] final_path = '%s/vifs' % NODE1['uuid']
self.mgr.vif_attach(**kwargs) self.mgr.vif_attach(**kwargs)
update_mock.assert_called_once_with(final_path, {'id': 'vif_id'}, update_mock.assert_called_once_with(
http_method="POST") mock.ANY, final_path, {'id': 'vif_id'}, http_method="POST")
@mock.patch.object(node.NodeManager, 'update') @mock.patch.object(node.NodeManager, 'update', autospec=True)
def test_vif_attach_custom_fields(self, update_mock): def test_vif_attach_custom_fields(self, update_mock):
kwargs = { kwargs = {
'node_ident': NODE1['uuid'], 'node_ident': NODE1['uuid'],
@ -1452,10 +1452,11 @@ class NodeManagerTest(testtools.TestCase):
final_path = '%s/vifs' % NODE1['uuid'] final_path = '%s/vifs' % NODE1['uuid']
self.mgr.vif_attach(**kwargs) self.mgr.vif_attach(**kwargs)
update_mock.assert_called_once_with( update_mock.assert_called_once_with(
mock.ANY,
final_path, {'id': 'vif_id', 'foo': 'bar'}, final_path, {'id': 'vif_id', 'foo': 'bar'},
http_method="POST") http_method="POST")
@mock.patch.object(node.NodeManager, 'update') @mock.patch.object(node.NodeManager, 'update', autospec=True)
def test_vif_attach_custom_fields_id(self, update_mock): def test_vif_attach_custom_fields_id(self, update_mock):
kwargs = { kwargs = {
'node_ident': NODE1['uuid'], 'node_ident': NODE1['uuid'],
@ -1466,7 +1467,7 @@ class NodeManagerTest(testtools.TestCase):
exc.InvalidAttribute, exc.InvalidAttribute,
self.mgr.vif_attach, **kwargs) self.mgr.vif_attach, **kwargs)
@mock.patch.object(node.NodeManager, 'delete') @mock.patch.object(node.NodeManager, 'delete', autospec=True)
def test_vif_detach(self, delete_mock): def test_vif_detach(self, delete_mock):
kwargs = { kwargs = {
'node_ident': NODE1['uuid'], 'node_ident': NODE1['uuid'],
@ -1475,7 +1476,7 @@ class NodeManagerTest(testtools.TestCase):
final_path = '%s/vifs/vif_id' % NODE1['uuid'] final_path = '%s/vifs/vif_id' % NODE1['uuid']
self.mgr.vif_detach(**kwargs) self.mgr.vif_detach(**kwargs)
delete_mock.assert_called_once_with(final_path) delete_mock.assert_called_once_with(mock.ANY, final_path)
def _test_node_set_boot_device(self, boot_device, persistent=False): def _test_node_set_boot_device(self, boot_device, persistent=False):
self.mgr.set_boot_device(NODE1['uuid'], boot_device, persistent) self.mgr.set_boot_device(NODE1['uuid'], boot_device, persistent)

View File

@ -1,7 +1,7 @@
# The order of packages is significant, because pip processes them in the order # The order of packages is significant, because pip processes them in the order
# of appearance. Changing the order has an impact on the overall integration # of appearance. Changing the order has an impact on the overall integration
# process, which may cause wedges in the gate later. # process, which may cause wedges in the gate later.
hacking!=0.13.0,<0.14,>=0.12.0 # Apache-2.0 hacking>=1.0.0 # Apache-2.0
coverage!=4.4,>=4.0 # Apache-2.0 coverage!=4.4,>=4.0 # Apache-2.0
doc8 # Apache-2.0 doc8 # Apache-2.0
fixtures>=3.0.0 # Apache-2.0/BSD fixtures>=3.0.0 # Apache-2.0/BSD

View File

@ -52,8 +52,11 @@ ignore =
exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build,tools exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build,tools
# [H106] Don't put vim configuration in source files. # [H106] Don't put vim configuration in source files.
# [H203] Use assertIs(Not)None to check for None. # [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. # [H904] Delay string interpolations at logging calls.
enable-extensions=H106,H203,H904 enable-extensions=H106,H203,H204,H205,H210,H904
[hacking] [hacking]
import_exceptions = testtools.matchers, ironicclient.common.i18n import_exceptions = testtools.matchers, ironicclient.common.i18n