Use assertIs(Not)None instead of assert(Not)Equal

Instead of using assert(Not)Equal(None, ***), developers should
use assertIs(Not)None(***) to have more clear messages in case of failure.

Change-Id: I20dd008badde8a3e87a367e7ab791ace4e117fb7

HACKING.rst

@@ -10,6 +10,8 @@ Octavia Specific Commandments
 -----------------------------
 - [O316] Change assertTrue(isinstance(A, B)) by optimal assert like
   assertIsInstance(A, B).
+- [O318] Change assert(Not)Equal(A, None) or assert(Not)Equal(None, A)
+  by optimal assert like assertIs(Not)None(A).
 - [O319] Validate that debug level logs are not translated.
 - [O320] Validate that LOG messages, except debug ones, have translations
 - [O321] Validate that jsonutils module is used instead of json

octavia/hacking/checks.py

@@ -67,6 +67,14 @@ assert_equal_with_true_re = re.compile(
 assert_equal_with_false_re = re.compile(
     r"assertEqual\(False,")
 mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
+assert_equal_end_with_none_re = re.compile(
+    r"(.)*assertEqual\((\w|\.|\'|\"|\[|\])+, None\)")
+assert_equal_start_with_none_re = re.compile(
+    r"(.)*assertEqual\(None, (\w|\.|\'|\"|\[|\])+\)")
+assert_not_equal_end_with_none_re = re.compile(
+    r"(.)*assertNotEqual\((\w|\.|\'|\"|\[|\])+, None\)")
+assert_not_equal_start_with_none_re = re.compile(
+    r"(.)*assertNotEqual\(None, (\w|\.|\'|\"|\[|\])+\)")
 
 
 def _directory_to_check_translation(filename):
@@ -82,6 +90,23 @@ def assert_true_instance(logical_line):
         yield (0, "O316: assertTrue(isinstance(a, b)) sentences not allowed")
 
 
+def assert_equal_or_not_none(logical_line):
+    """Check for assertEqual(A, None) or assertEqual(None, A) sentences,
+
+    assertNotEqual(A, None) or assertNotEqual(None, A) sentences
+
+    O318
+    """
+    msg = ("O318: assertEqual/assertNotEqual(A, None) or "
+           "assertEqual/assertNotEqual(None, A) sentences not allowed")
+    res = (assert_equal_start_with_none_re.match(logical_line) or
+           assert_equal_end_with_none_re.match(logical_line) or
+           assert_not_equal_start_with_none_re.match(logical_line) or
+           assert_not_equal_end_with_none_re.match(logical_line))
+    if res:
+        yield (0, msg)
+
+
 def no_translate_debug_logs(logical_line, filename):
     """Check for 'LOG.debug(_('
 
@@ -179,6 +204,7 @@ def assert_equal_in(logical_line):
 
 def factory(register):
     register(assert_true_instance)
+    register(assert_equal_or_not_none)
     register(no_translate_debug_logs)
     register(validate_log_translations)
     register(use_jsonutils)

octavia/tests/functional/api/v1/test_pool.py

@@ -343,7 +343,7 @@ class TestPool(base.BaseAPITest):
                                             self.listener.get('id'),
                                             constants.PENDING_UPDATE,
                                             constants.ONLINE)
-        self.assertNotEqual(None, api_pool.get('session_persistence'))
+        self.assertIsNotNone(api_pool.get('session_persistence'))
 
     def test_create_when_lb_pending_update(self):
         self.put(self.LB_PATH.format(lb_id=self.lb.get('id')),

octavia/tests/unit/test_hacking.py

@@ -58,6 +58,27 @@ class HackingTestCase(base.BaseTestCase):
         self.assertEqual(0, len(list(checks.assert_true_instance(
             "self.assertTrue()"))))
 
+    def test_assert_equal_or_not_none(self):
+        self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
+            "self.assertEqual(A, None)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
+            "self.assertEqual(None, A)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
+            "self.assertNotEqual(A, None)"))))
+
+        self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
+            "self.assertNotEqual(None, A)"))))
+
+        self.assertEqual(0,
+                         len(list(checks.assert_equal_or_not_none(
+                             "self.assertIsNone()"))))
+
+        self.assertEqual(0,
+                         len(list(checks.assert_equal_or_not_none(
+                             "self.assertIsNotNone()"))))
+
     def test_assert_equal_in(self):
         self.assertEqual(1, len(list(checks.assert_equal_in(
             "self.assertEqual(a in b, True)"))))