Use assertIsNone instead of assertEqual(None, ***)
Instead of using assertEqual(None, ***), developers should use assertIsNone(***) to have more clear messages in case of failure. Closes-Bug: #1510006 Change-Id: Ib3d09ed651877569a9b940da97662489885f18e9
This commit is contained in:
parent
23a5d63a22
commit
e97f70788c
|
@ -9,4 +9,6 @@ Magnum Specific Commandments
|
||||||
----------------------------
|
----------------------------
|
||||||
|
|
||||||
- [M301] policy.enforce_wsgi decorator must be the first decorator on a method.
|
- [M301] policy.enforce_wsgi decorator must be the first decorator on a method.
|
||||||
|
- [M318] Change assertEqual(A, None) or assertEqual(None, A) by optimal assert
|
||||||
|
like assertIsNone(A)
|
||||||
- [M322] Method's default argument shouldn't be mutable.
|
- [M322] Method's default argument shouldn't be mutable.
|
||||||
|
|
|
@ -33,6 +33,10 @@ Guidelines for writing new hacking checks
|
||||||
enforce_re = re.compile(r"@policy.enforce_wsgi*")
|
enforce_re = re.compile(r"@policy.enforce_wsgi*")
|
||||||
decorator_re = re.compile(r"@.*")
|
decorator_re = re.compile(r"@.*")
|
||||||
mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
|
mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
|
||||||
|
asse_equal_end_with_none_re = re.compile(
|
||||||
|
r"(.)*assertEqual\((\w|\.|\'|\"|\[|\])+, None\)")
|
||||||
|
asse_equal_start_with_none_re = re.compile(
|
||||||
|
r"(.)*assertEqual\(None, (\w|\.|\'|\"|\[|\])+\)")
|
||||||
|
|
||||||
|
|
||||||
def check_policy_enforce_decorator(logical_line,
|
def check_policy_enforce_decorator(logical_line,
|
||||||
|
@ -45,6 +49,19 @@ def check_policy_enforce_decorator(logical_line,
|
||||||
yield(0, msg)
|
yield(0, msg)
|
||||||
|
|
||||||
|
|
||||||
|
def assert_equal_none(logical_line):
|
||||||
|
"""Check for assertEqual(A, None) or assertEqual(None, A) sentences
|
||||||
|
|
||||||
|
M318
|
||||||
|
"""
|
||||||
|
msg = ("M318: assertEqual(A, None) or assertEqual(None, A) "
|
||||||
|
"sentences not allowed")
|
||||||
|
res = (asse_equal_start_with_none_re.match(logical_line) or
|
||||||
|
asse_equal_end_with_none_re.match(logical_line))
|
||||||
|
if res:
|
||||||
|
yield (0, msg)
|
||||||
|
|
||||||
|
|
||||||
def no_mutable_default_args(logical_line):
|
def no_mutable_default_args(logical_line):
|
||||||
msg = "M322: Method's default argument shouldn't be mutable!"
|
msg = "M322: Method's default argument shouldn't be mutable!"
|
||||||
if mutable_default_args.match(logical_line):
|
if mutable_default_args.match(logical_line):
|
||||||
|
@ -54,3 +71,4 @@ def no_mutable_default_args(logical_line):
|
||||||
def factory(register):
|
def factory(register):
|
||||||
register(check_policy_enforce_decorator)
|
register(check_policy_enforce_decorator)
|
||||||
register(no_mutable_default_args)
|
register(no_mutable_default_args)
|
||||||
|
register(assert_equal_none)
|
||||||
|
|
|
@ -118,8 +118,8 @@ class UtilsTestCase(base.TestCase):
|
||||||
utils.get_ip_version, 'x.x.x.x')
|
utils.get_ip_version, 'x.x.x.x')
|
||||||
|
|
||||||
def test_convert_to_list_dict(self):
|
def test_convert_to_list_dict(self):
|
||||||
self.assertEqual(None, utils.convert_to_list_dict(None, 'fred'))
|
self.assertIsNone(utils.convert_to_list_dict(None, 'fred'))
|
||||||
self.assertEqual(None, utils.convert_to_list_dict('', 'fred'))
|
self.assertIsNone(utils.convert_to_list_dict('', 'fred'))
|
||||||
self.assertEqual([{'fred': 'list'}],
|
self.assertEqual([{'fred': 'list'}],
|
||||||
utils.convert_to_list_dict('list', 'fred'))
|
utils.convert_to_list_dict('list', 'fred'))
|
||||||
self.assertEqual([{'fred': 'first'}, {'fred': 'second'}],
|
self.assertEqual([{'fred': 'first'}, {'fred': 'second'}],
|
||||||
|
|
|
@ -86,6 +86,16 @@ class HackingTestCase(base.TestCase):
|
||||||
self._assert_has_errors(code, checks.check_policy_enforce_decorator,
|
self._assert_has_errors(code, checks.check_policy_enforce_decorator,
|
||||||
expected_errors=[(2, 0, "M301")])
|
expected_errors=[(2, 0, "M301")])
|
||||||
|
|
||||||
|
def test_assert_equal_none(self):
|
||||||
|
self.assertEqual(len(list(checks.assert_equal_none(
|
||||||
|
"self.assertEqual(A, None)"))), 1)
|
||||||
|
|
||||||
|
self.assertEqual(len(list(checks.assert_equal_none(
|
||||||
|
"self.assertEqual(None, A)"))), 1)
|
||||||
|
|
||||||
|
self.assertEqual(
|
||||||
|
len(list(checks.assert_equal_none("self.assertIsNone()"))), 0)
|
||||||
|
|
||||||
def test_no_mutable_default_args(self):
|
def test_no_mutable_default_args(self):
|
||||||
self.assertEqual(1, len(list(checks.no_mutable_default_args(
|
self.assertEqual(1, len(list(checks.no_mutable_default_args(
|
||||||
"def get_info_from_bdm(virt_type, bdm, mapping=[])"))))
|
"def get_info_from_bdm(virt_type, bdm, mapping=[])"))))
|
||||||
|
|
Loading…
Reference in New Issue