Using assertFalse(A) instead of assertEqual(False, A)
This patch is to replace assertEqual(False, A) with assertFalse(A), which the latter is more straightforward and easier to understand. Also the relevant hacking rule is added. Change-Id: Idd09acc6820e54c4388e183963d405b8990bc533
This commit is contained in:
parent
ad6d14b6d4
commit
bcad8a84c0
@ -26,6 +26,7 @@ Cinder Specific Commandments
|
||||
- [C311] Check for proper naming and usage in option registration.
|
||||
- [C312] Check that assertIsNone(value) is used and not assertEqual(None, value).
|
||||
- [C313] Check that assertTrue(value) is used and not assertEqual(True, value).
|
||||
- [C314] Check that assertFalse(value) is used and not assertEqual(False, value).
|
||||
|
||||
General
|
||||
-------
|
||||
|
@ -61,6 +61,8 @@ assert_None = re.compile(
|
||||
r".*assertEqual\(None, .*\)")
|
||||
assert_True = re.compile(
|
||||
r".*assertEqual\(True, .*\)")
|
||||
assert_False = re.compile(
|
||||
r".*assertEqual\(False, .*\)")
|
||||
|
||||
|
||||
class BaseASTChecker(ast.NodeVisitor):
|
||||
@ -462,6 +464,13 @@ def validate_assertTrue(logical_line):
|
||||
yield(0, msg)
|
||||
|
||||
|
||||
def validate_assertFalse(logical_line):
|
||||
if re.match(assert_False, logical_line):
|
||||
msg = ("C314: Unit tests should use assertFalse(value) instead"
|
||||
" of using assertEqual(False, value).")
|
||||
yield(0, msg)
|
||||
|
||||
|
||||
def factory(register):
|
||||
register(no_vi_headers)
|
||||
register(no_translate_debug_logs)
|
||||
|
@ -994,34 +994,24 @@ class BackupsAPITestCase(test.TestCase):
|
||||
volume = self.volume_api.get(context.get_admin_context(), volume_id)
|
||||
|
||||
# test empty service
|
||||
self.assertEqual(False,
|
||||
self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'],
|
||||
testhost))
|
||||
self.assertFalse(self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'], testhost))
|
||||
|
||||
# test host not match service
|
||||
self.assertEqual(False,
|
||||
self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'],
|
||||
testhost))
|
||||
self.assertFalse(self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'], testhost))
|
||||
|
||||
# test az not match service
|
||||
self.assertEqual(False,
|
||||
self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'],
|
||||
testhost))
|
||||
self.assertFalse(self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'], testhost))
|
||||
|
||||
# test disabled service
|
||||
self.assertEqual(False,
|
||||
self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'],
|
||||
testhost))
|
||||
self.assertFalse(self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'], testhost))
|
||||
|
||||
# test dead service
|
||||
self.assertEqual(False,
|
||||
self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'],
|
||||
testhost))
|
||||
self.assertFalse(self.backup_api._is_backup_service_enabled(
|
||||
volume['availability_zone'], testhost))
|
||||
|
||||
# test multi services and the last service matches
|
||||
self.assertTrue(self.backup_api._is_backup_service_enabled(
|
||||
|
@ -232,10 +232,10 @@ class SnapshotManageTest(test.TestCase):
|
||||
def test_get_manageable_snapshots_non_admin(self, mock_api_manageable):
|
||||
res = self._get_resp_get('fakehost', False, False, admin=False)
|
||||
self.assertEqual(http_client.FORBIDDEN, res.status_int)
|
||||
self.assertEqual(False, mock_api_manageable.called)
|
||||
self.assertFalse(mock_api_manageable.called)
|
||||
res = self._get_resp_get('fakehost', True, False, admin=False)
|
||||
self.assertEqual(http_client.FORBIDDEN, res.status_int)
|
||||
self.assertEqual(False, mock_api_manageable.called)
|
||||
self.assertFalse(mock_api_manageable.called)
|
||||
|
||||
@mock.patch('cinder.volume.api.API.get_manageable_snapshots',
|
||||
wraps=api_get_manageable_snapshots)
|
||||
|
@ -725,7 +725,7 @@ class VolumeApiTest(test.TestCase):
|
||||
|
||||
req = fakes.HTTPRequest.blank('/v1/volumes/%s' % fake.VOLUME_ID)
|
||||
res_dict = self.controller.show(req, fake.VOLUME_ID)
|
||||
self.assertEqual(False, res_dict['volume']['encrypted'])
|
||||
self.assertFalse(res_dict['volume']['encrypted'])
|
||||
|
||||
@mock.patch.object(volume_api.API, 'delete', v2_fakes.fake_volume_delete)
|
||||
@mock.patch.object(volume_api.API, 'get', v2_fakes.fake_volume_get)
|
||||
|
@ -1426,7 +1426,7 @@ class VolumeApiTest(test.TestCase):
|
||||
|
||||
req = fakes.HTTPRequest.blank('/v2/volumes/%s' % fake.VOLUME_ID)
|
||||
res_dict = self.controller.show(req, fake.VOLUME_ID)
|
||||
self.assertEqual(False, res_dict['volume']['encrypted'])
|
||||
self.assertFalse(res_dict['volume']['encrypted'])
|
||||
|
||||
def test_volume_show_with_error_managing_deleting(self):
|
||||
def fake_volume_get(self, context, volume_id, **kwargs):
|
||||
|
@ -190,5 +190,5 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
|
||||
body['consistencygroup']['description'],
|
||||
body['consistencygroup']['add_volumes'],
|
||||
body['consistencygroup']['remove_volumes'])
|
||||
self.assertEqual(False, allow_empty)
|
||||
self.assertFalse(allow_empty)
|
||||
consistencygroup.destroy()
|
||||
|
@ -375,6 +375,13 @@ class HackingTestCase(test.TestCase):
|
||||
self.assertEqual(1, len(list(checks.validate_assertTrue(
|
||||
"assertEqual(True, %s)" % test_value))))
|
||||
|
||||
def test_validate_assertFalse(self):
|
||||
test_value = False
|
||||
self.assertEqual(0, len(list(checks.validate_assertFalse(
|
||||
"assertFalse(False)"))))
|
||||
self.assertEqual(1, len(list(checks.validate_assertFalse(
|
||||
"assertEqual(False, %s)" % test_value))))
|
||||
|
||||
@ddt.unpack
|
||||
@ddt.data(
|
||||
(1, 'LOG.info', "cinder/tests/unit/fake.py", False),
|
||||
|
@ -137,7 +137,7 @@ class UnityDriverTest(unittest.TestCase):
|
||||
self.assertEqual('', config.san_private_key)
|
||||
self.assertEqual('', config.san_clustername)
|
||||
self.assertEqual(22, config.san_ssh_port)
|
||||
self.assertEqual(False, config.san_is_local)
|
||||
self.assertFalse(config.san_is_local)
|
||||
self.assertEqual(30, config.ssh_conn_timeout)
|
||||
self.assertEqual(1, config.ssh_min_pool_conn)
|
||||
self.assertEqual(5, config.ssh_max_pool_conn)
|
||||
|
@ -1048,7 +1048,7 @@ class FlashSystemDriverTestCase(test.TestCase):
|
||||
self.assertEqual('IBM', stats['vendor_name'])
|
||||
self.assertEqual('FC', stats['storage_protocol'])
|
||||
self.assertEqual(backend_name, stats['volume_backend_name'])
|
||||
self.assertEqual(False, stats['multiattach'])
|
||||
self.assertFalse(stats['multiattach'])
|
||||
|
||||
self._reset_flags()
|
||||
|
||||
|
@ -718,7 +718,7 @@ class NetAppBlockStorage7modeLibraryTestCase(test.TestCase):
|
||||
retval = self.library._refresh_volume_info()
|
||||
|
||||
self.assertIsNone(retval)
|
||||
self.assertEqual(False, self.library.vol_refresh_voluntary)
|
||||
self.assertFalse(self.library.vol_refresh_voluntary)
|
||||
self.assertEqual(['vol1', 'vol2'], self.library.volume_list)
|
||||
self.assertIsNotNone(self.library.vol_refresh_time)
|
||||
na_utils.set_safe_attr.assert_has_calls([
|
||||
|
@ -2309,7 +2309,7 @@ class VolumeTestCase(base.BaseVolumeTestCase):
|
||||
volume_api.check_volume_filters(filters)
|
||||
|
||||
# Confirming converted filter value against False
|
||||
self.assertEqual(False, filters['bootable'])
|
||||
self.assertFalse(filters['bootable'])
|
||||
|
||||
def test_check_volume_filters_invalid(self):
|
||||
"""Test bootable as filter"""
|
||||
|
Loading…
Reference in New Issue
Block a user