Added style enfore checks for assert statements

Added following style checks:
 * Enforce use of assertTrue/assertFalse
 * Enforce use of assertIs/assertIsNot

Change-Id: Ic4bc964fef9ea78934937dc74017569c2a55ba44
Implements: blueprint mistral-hacking

HACKING.rst

@@ -6,8 +6,11 @@ Read the OpenStack Style Commandments https://docs.openstack.org/developer/hacki
 Mistral Specific Commandments
 -----------------------------
 
+- [M001] Use LOG.warning(). LOG.warn() is deprecated.
 - [M318] Change assertEqual(A, None) or assertEqual(None, A) by optimal assert
   like assertIsNone(A)
+- [M319] Enforce use of assertTrue/assertFalse
+- [M320] Enforce use of assertIs/assertIsNot
 - [M327] Do not use xrange(). xrange() is not compatible with Python 3. Use
   range() or six.moves.range() instead.
 - [M328] Python 3: do not use dict.iteritems.

mistral/hacking/checks.py

@@ -49,6 +49,34 @@ def assert_equal_none(logical_line):
         yield (0, msg)
 
 
+def no_assert_equal_true_false(logical_line):
+    """Check for assertTrue/assertFalse sentences
+
+    M319
+    """
+    _start_re = re.compile(r'assert(Not)?Equal\((True|False),')
+    _end_re = re.compile(r'assert(Not)?Equal\(.*,\s+(True|False)\)$')
+
+    if _start_re.search(logical_line) or _end_re.search(logical_line):
+        yield (0, "M319: assertEqual(A, True|False), "
+               "assertEqual(True|False, A), assertNotEqual(A, True|False), "
+               "or assertEqual(True|False, A) sentences must not be used. "
+               "Use assertTrue(A) or assertFalse(A) instead")
+
+
+def no_assert_true_false_is_not(logical_line):
+    """Check for assertIs/assertIsNot sentences
+
+    M320
+    """
+    _re = re.compile(r'assert(True|False)\(.+\s+is\s+(not\s+)?.+\)$')
+
+    if _re.search(logical_line):
+        yield (0, "M320: assertTrue(A is|is not B) or "
+               "assertFalse(A is|is not B) sentences must not be used. "
+               "Use assertIs(A, B) or assertIsNot(A, B) instead")
+
+
 def check_oslo_namespace_imports(logical_line):
     if re.match(oslo_namespace_imports_from_dot, logical_line):
         msg = ("O323: '%s' must be used instead of '%s'.") % (
@@ -255,6 +283,8 @@ class CheckForLoggingIssues(BaseASTChecker):
 
 def factory(register):
     register(assert_equal_none)
+    register(no_assert_equal_true_false)
+    register(no_assert_true_false_is_not)
     register(check_oslo_namespace_imports)
     register(CheckForLoggingIssues)
     register(check_python3_no_iteritems)

mistral/tests/unit/hacking/test_checks.py

@@ -38,27 +38,27 @@ class BaseLoggingCheckTest(base.BaseTest):
     # installed.
     @mock.patch('pep8._checks',
                 {'physical_line': {}, 'logical_line': {}, 'tree': {}})
-    def run_check(self, code):
-        pep8.register_check(self.get_checker())
+    def run_check(self, code, checker, filename=None):
+        pep8.register_check(checker)
         lines = textwrap.dedent(code).strip().splitlines(True)
-        checker = pep8.Checker(lines=lines)
-        checker.check_all()
+        checker = pep8.Checker(filename=filename, lines=lines)
+        with mock.patch('pep8.StandardReport.get_file_results'):
+            checker.check_all()
         checker.report._deferred_print.sort()
 
         return checker.report._deferred_print
 
-    def assert_has_errors(self, code, expected_errors=None):
+    def _assert_has_errors(self, code, checker, expected_errors=None,
+                           filename=None):
 
         # Pull out the parts of the error that we'll match against.
-        actual_errors = (e[:3] for e in self.run_check(code))
-
-        # Adjust line numbers to make the fixture data more readable.
-        import_lines = len(self.code_ex.shared_imports.split('\n')) - 1
-        actual_errors = [(e[0] - import_lines, e[1], e[2])
-                         for e in actual_errors]
-
+        actual_errors = [e[:3] for e in
+                         self.run_check(code, checker, filename)]
         self.assertEqual(expected_errors or [], actual_errors)
 
+    def _assert_has_no_errors(self, code, checker, filename=None):
+        self._assert_has_errors(code, checker, filename=filename)
+
     def test_assert_equal_none(self):
         self.assertEqual(len(list(checks.assert_equal_none(
             "self.assertEqual(A, None)"))), 1)
@@ -69,6 +69,40 @@ class BaseLoggingCheckTest(base.BaseTest):
         self.assertEqual(
             len(list(checks.assert_equal_none("self.assertIsNone()"))), 0)
 
+    def test_no_assert_equal_true_false(self):
+        code = """
+                  self.assertEqual(context_is_admin, True)
+                  self.assertEqual(context_is_admin, False)
+                  self.assertEqual(True, context_is_admin)
+                  self.assertEqual(False, context_is_admin)
+                  self.assertNotEqual(context_is_admin, True)
+                  self.assertNotEqual(context_is_admin, False)
+                  self.assertNotEqual(True, context_is_admin)
+                  self.assertNotEqual(False, context_is_admin)
+               """
+        errors = [(1, 0, 'M319'), (2, 0, 'M319'), (3, 0, 'M319'),
+                  (4, 0, 'M319'), (5, 0, 'M319'), (6, 0, 'M319'),
+                  (7, 0, 'M319'), (8, 0, 'M319')]
+        self._assert_has_errors(code, checks.no_assert_equal_true_false,
+                                expected_errors=errors)
+        code = """
+                  self.assertEqual(context_is_admin, stuff)
+                  self.assertNotEqual(context_is_admin, stuff)
+               """
+        self._assert_has_no_errors(code, checks.no_assert_equal_true_false)
+
+    def test_no_assert_true_false_is_not(self):
+        code = """
+                  self.assertTrue(test is None)
+                  self.assertTrue(False is my_variable)
+                  self.assertFalse(None is test)
+                  self.assertFalse(my_variable is False)
+               """
+        errors = [(1, 0, 'M320'), (2, 0, 'M320'), (3, 0, 'M320'),
+                  (4, 0, 'M320')]
+        self._assert_has_errors(code, checks.no_assert_true_false_is_not,
+                                expected_errors=errors)
+
     def test_check_python3_xrange(self):
         func = checks.check_python3_xrange
         self.assertEqual(1, len(list(func('for i in xrange(10)'))))
@@ -105,4 +139,5 @@ class TestLoggingWithWarn(BaseLoggingCheckTest):
         code = self.code_ex.shared_imports + data['code']
         errors = data['expected_errors']
 
-        self.assert_has_errors(code, expected_errors=errors)
+        self._assert_has_errors(code, checks.CheckForLoggingIssues,
+                                expected_errors=errors)

mistral/tests/unit/mstrlfixtures/hacking.py

@@ -35,6 +35,6 @@ class HackingLogging(fixtures.Fixture):
                 LOG.warn('text')
         """,
         'expected_errors': [
-            (4, 9, 'M001'),
+            (8, 9, 'M001'),
         ],
     }