From b85248bce686309395e92f8b7e31f8a4e8fe7fd4 Mon Sep 17 00:00:00 2001 From: Tom Barron Date: Mon, 23 May 2016 16:35:56 -0400 Subject: [PATCH] Use assertTrue rather than assertEqual(True, ...) Manila unit tests contain several occurences of assertEqual(True, ...) instead of assertTrue(...). Fix these occurences and add hacking check so new occurences don't creep into the codebase. Closes-Bug: #1584948 Change-Id: Ib586590b9efcb5253e8c09ee735a8ffb62b44d74 --- HACKING.rst | 2 +- manila/hacking/checks.py | 10 ++++++++++ manila/tests/share/drivers/test_lvm.py | 2 +- manila/tests/share/drivers/test_service_instance.py | 9 +++------ manila/tests/share/test_driver.py | 3 +-- manila/tests/test_hacking.py | 7 +++++++ 6 files changed, 23 insertions(+), 10 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 01b1a32b48..d152ac948a 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -8,7 +8,7 @@ Manila Style Commandments Manila Specific Commandments ---------------------------- - +- [M313] Use assertTrue(...) rather than assertEqual(True, ...). - [M319] Validate that debug level logs are not translated. - [M323] Ensure that the _() function is explicitly imported to ensure proper translations. - [M325] str() cannot be used on an exception. Remove use or use six.text_type() diff --git a/manila/hacking/checks.py b/manila/hacking/checks.py index cce8fcec6b..daf8900de3 100644 --- a/manila/hacking/checks.py +++ b/manila/hacking/checks.py @@ -56,6 +56,8 @@ custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*") oslo_namespace_imports = re.compile(r"from[\s]*oslo[.](.*)") dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") assert_no_xrange_re = re.compile(r"\s*xrange\s*\(") +assert_True = re.compile( + r".*assertEqual\(True, .*\)") class BaseASTChecker(ast.NodeVisitor): @@ -249,6 +251,13 @@ def no_xrange(logical_line): yield(0, "M337: Do not use xrange().") +def validate_assertTrue(logical_line): + if re.match(assert_True, logical_line): + msg = ("M313: Unit tests should use assertTrue(value) instead" + " of using assertEqual(True, value).") + yield(0, msg) + + def factory(register): register(validate_log_translations) register(check_explicit_underscore_import) @@ -258,3 +267,4 @@ def factory(register): register(check_oslo_namespace_imports) register(dict_constructor_with_list_copy) register(no_xrange) + register(validate_assertTrue) diff --git a/manila/tests/share/drivers/test_lvm.py b/manila/tests/share/drivers/test_lvm.py index 4bcd59e224..f01fbf4441 100644 --- a/manila/tests/share/drivers/test_lvm.py +++ b/manila/tests/share/drivers/test_lvm.py @@ -514,6 +514,6 @@ class LVMShareDriverTestCase(test.TestCase): self.assertEqual('NFS_CIFS', self._driver._stats['storage_protocol']) self.assertEqual(50, self._driver._stats['reserved_percentage']) self.assertIsNone(self._driver._stats['consistency_group_support']) - self.assertEqual(True, self._driver._stats['snapshot_support']) + self.assertTrue(self._driver._stats['snapshot_support']) self.assertEqual('LVMShareDriver', self._driver._stats['driver_name']) self.assertEqual('test-pool', self._driver._stats['pools']) diff --git a/manila/tests/share/drivers/test_service_instance.py b/manila/tests/share/drivers/test_service_instance.py index 86498d1a78..3e6c4b0d51 100644 --- a/manila/tests/share/drivers/test_service_instance.py +++ b/manila/tests/share/drivers/test_service_instance.py @@ -222,8 +222,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): self.config = configuration.Configuration(opts, 'CUSTOM') self._manager = service_instance.ServiceInstanceManager( self.config) - self.assertEqual( - True, + self.assertTrue( self._manager.get_config_option("driver_handles_share_servers")) self.assertIsNotNone(self._manager.driver_config) self.assertTrue(hasattr(self._manager, 'network_helper')) @@ -255,8 +254,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): service_instance_network_helper_type=service_instance.NOVA_NAME)) with test_utils.create_temp_config_with_opts(config_data): self._manager = service_instance.ServiceInstanceManager() - self.assertEqual( - True, + self.assertTrue( self._manager.get_config_option("driver_handles_share_servers")) self.assertIsNone(self._manager.driver_config) self.assertTrue(hasattr(self._manager, 'network_helper')) @@ -309,8 +307,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): "service_instance_name_template") % "%s_%s" % ( self._manager.driver_config.config_group, fake_server_id), result) - self.assertEqual( - True, + self.assertTrue( self._manager.get_config_option("driver_handles_share_servers")) self.assertTrue(hasattr(self._manager, 'network_helper')) self.assertTrue(service_instance.NovaNetworkHelper.called) diff --git a/manila/tests/share/test_driver.py b/manila/tests/share/test_driver.py index 26dd5713db..104887288e 100644 --- a/manila/tests/share/test_driver.py +++ b/manila/tests/share/test_driver.py @@ -265,8 +265,7 @@ class ShareDriverTestCase(test.TestCase): child_class_instance._update_share_stats() - self.assertEqual( - True, child_class_instance._stats["snapshot_support"]) + self.assertTrue(child_class_instance._stats["snapshot_support"]) self.assertTrue(child_class_instance.configuration.safe_get.called) @ddt.data( diff --git a/manila/tests/test_hacking.py b/manila/tests/test_hacking.py index 327925c8a8..22f2ff57c4 100644 --- a/manila/tests/test_hacking.py +++ b/manila/tests/test_hacking.py @@ -243,3 +243,10 @@ class HackingTestCase(test.TestCase): self.assertEqual(1, len(list(checks.no_xrange("xrange(45)")))) self.assertEqual(0, len(list(checks.no_xrange("range(45)")))) + + def test_validate_assertTrue(self): + test_value = True + self.assertEqual(0, len(list(checks.validate_assertTrue( + "assertTrue(True)")))) + self.assertEqual(1, len(list(checks.validate_assertTrue( + "assertEqual(True, %s)" % test_value))))