Add a hacking rule for useless assertions

Add a hacking rule for useless assertions in tests.

[N365] Misuse of assertTrue/assertIsNone

Change-Id: I3f76d57d75a266eddf7a4100c0b39fabe346e71c

HACKING.rst

@@ -75,6 +75,7 @@ Nova Specific Commandments
   calling is using priviledge escalation.
 - [N363] Disallow ``(not_a_tuple)`` because you meant ``(a_tuple_of_one,)``.
 - [N364] Check non-existent mock assertion methods and attributes.
+- [N365] Check misuse of assertTrue/assertIsNone.
 
 Creating Unit Tests
 -------------------

nova/hacking/checks.py

@@ -125,6 +125,9 @@ mock_assert_method_re = re.compile(
     r"(asser|asset|asssert|assset)_(called(_(once|with|once_with))?"
     r"|any_call|has_calls|not_called))\(")
 mock_attribute_re = re.compile(r"[\.\(](retrun_value)[,=\s]")
+# Regex for useless assertions
+useless_assertion_re = re.compile(
+    r"\.((assertIsNone)\(None|(assertTrue)\((True|\d+|'.+'|\".+\")),")
 
 
 class BaseASTChecker(ast.NodeVisitor):
@@ -942,6 +945,28 @@ def nonexistent_assertion_methods_and_attributes(logical_line, filename):
             yield (0, msg % match.group(1))
 
 
+def useless_assertion(logical_line, filename):
+    """Check useless assertions in tests.
+
+    The following assertions are useless.
+
+    - assertIsNone(None, ...)
+    - assertTrue(True, ...)
+    - assertTrue(2, ...)   # Constant number
+    - assertTrue('Constant string', ...)
+    - assertTrue("Constant string", ...)
+
+    They are usually misuses of assertIsNone or assertTrue.
+
+    N365
+    """
+    msg = "N365: Misuse of %s."
+    if 'nova/tests/' in filename:
+        match = useless_assertion_re.search(logical_line)
+        if match:
+            yield (0, msg % (match.group(2) or match.group(3)))
+
+
 def factory(register):
     register(import_no_db_in_virt)
     register(no_db_session_in_public_api)
@@ -989,3 +1014,4 @@ def factory(register):
     register(privsep_imports_not_aliased)
     register(did_you_mean_tuple)
     register(nonexistent_assertion_methods_and_attributes)
+    register(useless_assertion)

nova/tests/unit/test_hacking.py

@@ -991,3 +991,31 @@ class HackingTestCase(test.NoDBTestCase):
         self._assert_has_no_errors(
             code, checks.nonexistent_assertion_methods_and_attributes,
             filename="nova/tests/unit/test_context.py")
+
+    def test_useless_assertion(self):
+        code = """
+                   self.assertIsNone(None, status)
+                   self.assertTrue(True, flag)
+                   self.assertTrue(10, count)
+                   self.assertTrue('active', status)
+                   self.assertTrue("building", status)
+               """
+        errors = [(x + 1, 0, 'N365') for x in range(5)]
+        # Check errors in 'nova/tests' directory.
+        self._assert_has_errors(
+            code, checks.useless_assertion,
+            expected_errors=errors, filename="nova/tests/unit/test_context.py")
+        # Check no errors in other than 'nova/tests' directory.
+        self._assert_has_no_errors(
+            code, checks.nonexistent_assertion_methods_and_attributes,
+            filename="nova/compute/api.py")
+        code = """
+                   self.assertIsNone(None_test_var, "Fails")
+                   self.assertTrue(True_test_var, 'Fails')
+                   self.assertTrue(var2, "Fails")
+                   self.assertTrue(test_class.is_active('active'), 'Fails')
+                   self.assertTrue(check_status("building"), 'Fails')
+               """
+        self._assert_has_no_errors(
+            code, checks.useless_assertion,
+            filename="nova/tests/unit/test_context.py")