From bcad8a84c00f10d7c8245a866f63700fcccac290 Mon Sep 17 00:00:00 2001 From: "jeremy.zhang" Date: Tue, 6 Jun 2017 11:00:40 +0800 Subject: [PATCH] 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 --- HACKING.rst | 1 + cinder/hacking/checks.py | 9 ++++++ cinder/tests/unit/api/contrib/test_backups.py | 30 +++++++------------ .../unit/api/contrib/test_snapshot_manage.py | 4 +-- cinder/tests/unit/api/v1/test_volumes.py | 2 +- cinder/tests/unit/api/v2/test_volumes.py | 2 +- .../unit/api/v3/test_consistencygroups.py | 2 +- cinder/tests/unit/test_hacking.py | 7 +++++ .../drivers/dell_emc/unity/test_driver.py | 2 +- .../drivers/ibm/test_ibm_flashsystem.py | 2 +- .../netapp/dataontap/test_block_7mode.py | 2 +- cinder/tests/unit/volume/test_volume.py | 2 +- 12 files changed, 36 insertions(+), 29 deletions(-) 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"""