diff --git a/tests/hacking/README.rst b/tests/hacking/README.rst index 0b2ed91f..f8f6d1b0 100644 --- a/tests/hacking/README.rst +++ b/tests/hacking/README.rst @@ -23,6 +23,7 @@ Rally Specific Commandments * [N323] - Ensure that ``assertTrue/assertFalse(A in/not in B)`` are not used with collection contents * [N324] - Ensure that ``assertEqual(A in/not in B, True/False)`` and ``assertEqual(True/False, A in/not in B)`` are not used with collection contents * [N325] - Ensure that ``assertNotEqual(A, None)`` and ``assertNotEqual(None, A)`` are not used + * [N326] - Ensure that ``assertEqual(A, True/False)`` and ``assertEqual(True/False, A)`` are not used * [N340] - Ensure that we are importing always ``from rally import objects`` * [N341] - Ensure that we are importing oslo_xyz packages instead of deprecated oslo.xyz ones * [N342] - Ensure that we load opts from correct paths only diff --git a/tests/hacking/checks.py b/tests/hacking/checks.py index 0db7bdf2..f97c5e92 100644 --- a/tests/hacking/checks.py +++ b/tests/hacking/checks.py @@ -28,6 +28,11 @@ import functools import re import tokenize + +re_assert_equal_end_with_true_or_false = re.compile( + r"assertEqual\(.*?, \s+(True|False)\)$") +re_assert_equal_start_with_true_or_false = re.compile( + r"assertEqual\((True|False),") re_assert_true_instance = re.compile( r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, " r"(\w|\.|\'|\"|\[|\])+\)\)") @@ -296,6 +301,23 @@ def assert_not_equal_none(logical_line, physical_line, filename): "instead.") +@skip_ignored_lines +def assert_equal_true_or_false(logical_line, physical_line, filename): + """Check for assertEqual(A, True/False) sentences + + Check for assertEqual(A, True/False) sentences or + assertEqual(True/False, A) + + N326 + """ + res = (re_assert_equal_end_with_true_or_false.search(logical_line) or + re_assert_equal_start_with_true_or_false.search(logical_line)) + if res: + yield (0, "N326 assertEqual(A, True/False) or " + "assertEqual(True/False, A) sentences not allowed," + "you should use assertTrue(A) or assertFalse(A) instead.") + + @skip_ignored_lines def check_no_direct_rally_objects_import(logical_line, physical_line, filename): @@ -591,6 +613,7 @@ def factory(register): register(assert_equal_none) register(assert_true_or_false_with_in) register(assert_equal_in) + register(assert_equal_true_or_false) register(check_no_direct_rally_objects_import) register(check_no_oslo_deprecated_import) register(check_quotes) diff --git a/tests/unit/plugins/openstack/cleanup/test_resources.py b/tests/unit/plugins/openstack/cleanup/test_resources.py index d070a96d..94d146db 100755 --- a/tests/unit/plugins/openstack/cleanup/test_resources.py +++ b/tests/unit/plugins/openstack/cleanup/test_resources.py @@ -121,7 +121,7 @@ class NovaFlavorsTestCase(test.TestCase): mock_resource_manager__manager().get.side_effect = exc flavor = resources.NovaFlavors() flavor.raw_resource = mock.MagicMock() - self.assertEqual(True, flavor.is_deleted()) + self.assertTrue(flavor.is_deleted()) @mock.patch("%s.base.ResourceManager._manager" % BASE) def test_is_deleted_fail(self, mock_resource_manager__manager): diff --git a/tests/unit/plugins/openstack/context/manila/test_manila_share_networks.py b/tests/unit/plugins/openstack/context/manila/test_manila_share_networks.py index 88ae8d80..c5ecba62 100644 --- a/tests/unit/plugins/openstack/context/manila/test_manila_share_networks.py +++ b/tests/unit/plugins/openstack/context/manila/test_manila_share_networks.py @@ -214,8 +214,7 @@ class ShareNetworksTestCase(test.TestCase): self.assertEqual(expected_ctxt["task"], inst.context.get("task")) self.assertEqual(expected_ctxt["config"], inst.context.get("config")) self.assertEqual(expected_ctxt["users"], inst.context.get("users")) - self.assertEqual( - False, + self.assertFalse( inst.context.get(consts.SHARE_NETWORKS_CONTEXT_NAME, {}).get( "delete_share_networks")) self.assertEqual(expected_ctxt["tenants"], inst.context.get("tenants"))