Merge "Replace 'assertEqual(True, a)' with 'assertTrue(a)' and add hacking rule"

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

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)

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):

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"))