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>
This commit is contained in:
parent
03b9937884
commit
09e2bcf3f5
@ -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
|
||||
-------------------
|
||||
|
@ -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)
|
||||
|
@ -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)
|
||||
|
Loading…
Reference in New Issue
Block a user