diff --git a/HACKING.rst b/HACKING.rst index edf37a2d34a..3763afdda57 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -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 ------- diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py index 0ae042f9bcd..90779c73793 100644 --- a/cinder/hacking/checks.py +++ b/cinder/hacking/checks.py @@ -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) diff --git a/cinder/tests/unit/api/contrib/test_backups.py b/cinder/tests/unit/api/contrib/test_backups.py index 521ce3526ca..30efb301271 100644 --- a/cinder/tests/unit/api/contrib/test_backups.py +++ b/cinder/tests/unit/api/contrib/test_backups.py @@ -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( diff --git a/cinder/tests/unit/api/contrib/test_snapshot_manage.py b/cinder/tests/unit/api/contrib/test_snapshot_manage.py index 152db592606..6bf9b448fcb 100644 --- a/cinder/tests/unit/api/contrib/test_snapshot_manage.py +++ b/cinder/tests/unit/api/contrib/test_snapshot_manage.py @@ -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) diff --git a/cinder/tests/unit/api/v1/test_volumes.py b/cinder/tests/unit/api/v1/test_volumes.py index 2e83e0af63c..10f29867434 100644 --- a/cinder/tests/unit/api/v1/test_volumes.py +++ b/cinder/tests/unit/api/v1/test_volumes.py @@ -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) diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py index 6a38889756c..f3320e21d0d 100644 --- a/cinder/tests/unit/api/v2/test_volumes.py +++ b/cinder/tests/unit/api/v2/test_volumes.py @@ -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): diff --git a/cinder/tests/unit/api/v3/test_consistencygroups.py b/cinder/tests/unit/api/v3/test_consistencygroups.py index ece7524ab04..89346971899 100644 --- a/cinder/tests/unit/api/v3/test_consistencygroups.py +++ b/cinder/tests/unit/api/v3/test_consistencygroups.py @@ -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() diff --git a/cinder/tests/unit/test_hacking.py b/cinder/tests/unit/test_hacking.py index b63a66fb05c..6ff2acdcb11 100644 --- a/cinder/tests/unit/test_hacking.py +++ b/cinder/tests/unit/test_hacking.py @@ -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), diff --git a/cinder/tests/unit/volume/drivers/dell_emc/unity/test_driver.py b/cinder/tests/unit/volume/drivers/dell_emc/unity/test_driver.py index b1ce7721bd1..75d29af5c1c 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/unity/test_driver.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/unity/test_driver.py @@ -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) diff --git a/cinder/tests/unit/volume/drivers/ibm/test_ibm_flashsystem.py b/cinder/tests/unit/volume/drivers/ibm/test_ibm_flashsystem.py index 63bec428f27..80e970b25e3 100644 --- a/cinder/tests/unit/volume/drivers/ibm/test_ibm_flashsystem.py +++ b/cinder/tests/unit/volume/drivers/ibm/test_ibm_flashsystem.py @@ -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() diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_7mode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_7mode.py index c1a1f93114a..530672c528a 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_7mode.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_7mode.py @@ -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([ diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index a1efa5fd018..d1a23881bc4 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -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"""