hacking: Use assertIs(Not), assert(True|False)

This is per the OpenStack style guidelines.

Change-Id: Iec102872e2d5b004255ce897cc22c4d1a41c6f9e
Co-authored-by: Gabor Antal <antal@inf.u-szeged.hu>

diff --git a/HACKING.rst b/HACKING.rst
index 35e754d708b4e27729823bf7910586d91e6c5a74..ed417c9b468e4937b5372a8360fcc68cbc307197 100644 (file)
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -65,6 +65,8 @@ Nova Specific Commandments
 - [N352] LOG.warn is deprecated. Enforce use of LOG.warning.
 - [N353] Validate that context objects is not passed in logging calls.
 - [N354] String interpolation should be delayed at logging calls.
+- [N355] Enforce use of assertTrue/assertFalse
+- [N356] Enforce use of assertIs/assertIsNot
 
 Creating Unit Tests
 -------------------

diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py
index 55b5005d7e835a1b6f3c5005e4bf63382b5bfa2c..15e3d48ae0a0e62bf7e47ce9ef37a807c85c8a1b 100644 (file)
--- a/nova/hacking/checks.py
+++ b/nova/hacking/checks.py
@@ -823,6 +823,39 @@ def check_delayed_string_interpolation(logical_line, physical_line, filename):
               "Use ',' instead of '%'.")
 
 
+def no_assert_equal_true_false(logical_line):
+    """Enforce use of assertTrue/assertFalse.
+
+    Prevent use of assertEqual(A, True|False), assertEqual(True|False, A),
+    assertNotEqual(A, True|False), and assertNotEqual(True|False, A).
+
+    N355
+    """
+    _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, "N355: 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):
+    """Enforce use of assertIs/assertIsNot.
+
+    Prevent use of assertTrue(A is|is not B) and assertFalse(A is|is not B).
+
+    N356
+    """
+    _re = re.compile(r'assert(True|False)\(.+\s+is\s+(not\s+)?.+\)$')
+
+    if _re.search(logical_line):
+        yield (0, "N356: 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 factory(register):
     register(import_no_db_in_virt)
     register(no_db_session_in_public_api)
@@ -864,3 +897,5 @@ def factory(register):
     register(CheckForUncalledTestClosure)
     register(check_context_log)
     register(check_delayed_string_interpolation)
+    register(no_assert_equal_true_false)
+    register(no_assert_true_false_is_not)

diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py
index ced94f016180a360fe774135eccfe52c55115335..96a4616032cb5fb3bdfeb3dae2f3fc007d0dd2e9 100644 (file)
--- a/nova/tests/unit/test_hacking.py
+++ b/nova/tests/unit/test_hacking.py
@@ -843,3 +843,37 @@ class HackingTestCase(test.NoDBTestCase):
                               for name, value in test.items()])})
                """
         self._assert_has_no_errors(code, checker)
+
+    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, 'N355'), (2, 0, 'N355'), (3, 0, 'N355'),
+                  (4, 0, 'N355'), (5, 0, 'N355'), (6, 0, 'N355'),
+                  (7, 0, 'N355'), (8, 0, 'N355')]
+        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, 'N356'), (2, 0, 'N356'), (3, 0, 'N356'),
+                  (4, 0, 'N356')]
+        self._assert_has_errors(code, checks.no_assert_true_false_is_not,
+                                expected_errors=errors)