Merge "Remove duplicate hacking checks"
This commit is contained in:
Zuul
Gerrit Code Review
commit
3b4144b65b
@ -10,19 +10,12 @@ As such, we tend to follow Neutron conventions regarding coding style.
|
||||
|
||||
Octavia Specific Commandments
|
||||
-----------------------------
|
||||
- [O316] Change assertTrue(isinstance(A, B)) by optimal assert like
|
||||
assertIsInstance(A, B).
|
||||
- [O318] Change assert(Not)Equal(A, None) or assert(Not)Equal(None, A)
|
||||
by optimal assert like assertIs(Not)None(A).
|
||||
- [O319] Validate that debug level logs are not translated.
|
||||
- [O321] Validate that jsonutils module is used instead of json
|
||||
- [O322] Don't use author tags
|
||||
- [O323] Change assertEqual(True, A) or assertEqual(False, A) to the more
|
||||
specific assertTrue(A) or assertFalse(A)
|
||||
- [O324] Method's default argument shouldn't be mutable
|
||||
- [O338] Change assertEqual(A in B, True), assertEqual(True, A in B),
|
||||
assertEqual(A in B, False) or assertEqual(False, A in B) to the more
|
||||
specific assertIn/NotIn(A, B)
|
||||
- [O339] LOG.warn() is not allowed. Use LOG.warning()
|
||||
- [O340] Don't use xrange()
|
||||
- [O341] Don't translate logs.
|
||||
|
||||
@ -42,24 +42,11 @@ _log_translation_hint = re.compile(
|
||||
hints='|'.join(_all_hints),
|
||||
))
|
||||
|
||||
assert_trueinst_re = re.compile(
|
||||
r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
|
||||
r"(\w|\.|\'|\"|\[|\])+\)\)")
|
||||
assert_equal_in_end_with_true_or_false_re = re.compile(
|
||||
r"assertEqual\((\w|[][.'\"])+ in (\w|[][.'\", ])+, (True|False)\)")
|
||||
assert_equal_in_start_with_true_or_false_re = re.compile(
|
||||
r"assertEqual\((True|False), (\w|[][.'\"])+ in (\w|[][.'\", ])+\)")
|
||||
assert_equal_with_true_re = re.compile(
|
||||
r"assertEqual\(True,")
|
||||
assert_equal_with_false_re = re.compile(
|
||||
r"assertEqual\(False,")
|
||||
mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
|
||||
assert_equal_end_with_none_re = re.compile(r"(.)*assertEqual\(.+, None\)")
|
||||
assert_equal_start_with_none_re = re.compile(r".*assertEqual\(None, .+\)")
|
||||
assert_not_equal_end_with_none_re = re.compile(
|
||||
r"(.)*assertNotEqual\(.+, None\)")
|
||||
assert_not_equal_start_with_none_re = re.compile(
|
||||
r"(.)*assertNotEqual\(None, .+\)")
|
||||
revert_must_have_kwargs_re = re.compile(
|
||||
r'[ ]*def revert\(.+,[ ](?!\*\*kwargs)\w+\):')
|
||||
untranslated_exception_re = re.compile(r"raise (?:\w*)\((.*)\)")
|
||||
@ -73,35 +60,6 @@ def _translation_checks_not_enforced(filename):
|
||||
return any(pat in filename for pat in ["/tests/", "rally-jobs/plugins/"])
|
||||
|
||||
|
||||
@core.flake8ext
|
||||
def assert_true_instance(logical_line):
|
||||
"""Check for assertTrue(isinstance(a, b)) sentences
|
||||
|
||||
O316
|
||||
"""
|
||||
if assert_trueinst_re.match(logical_line):
|
||||
yield (0, "O316: assertTrue(isinstance(a, b)) sentences not allowed. "
|
||||
"Use assertIsInstance instead.")
|
||||
|
||||
|
||||
@core.flake8ext
|
||||
def assert_equal_or_not_none(logical_line):
|
||||
"""Check for assertEqual(A, None) or assertEqual(None, A) sentences,
|
||||
|
||||
assertNotEqual(A, None) or assertNotEqual(None, A) sentences
|
||||
|
||||
O318
|
||||
"""
|
||||
msg = ("O318: assertEqual/assertNotEqual(A, None) or "
|
||||
"assertEqual/assertNotEqual(None, A) sentences not allowed")
|
||||
res = (assert_equal_start_with_none_re.match(logical_line) or
|
||||
assert_equal_end_with_none_re.match(logical_line) or
|
||||
assert_not_equal_start_with_none_re.match(logical_line) or
|
||||
assert_not_equal_end_with_none_re.match(logical_line))
|
||||
if res:
|
||||
yield (0, msg)
|
||||
|
||||
|
||||
@core.flake8ext
|
||||
def assert_equal_true_or_false(logical_line):
|
||||
"""Check for assertEqual(True, A) or assertEqual(False, A) sentences
|
||||
@ -122,22 +80,6 @@ def no_mutable_default_args(logical_line):
|
||||
yield (0, msg)
|
||||
|
||||
|
||||
@core.flake8ext
|
||||
def assert_equal_in(logical_line):
|
||||
"""Check for assertEqual(A in B, True), assertEqual(True, A in B),
|
||||
|
||||
assertEqual(A in B, False) or assertEqual(False, A in B) sentences
|
||||
|
||||
O338
|
||||
"""
|
||||
res = (assert_equal_in_start_with_true_or_false_re.search(logical_line) or
|
||||
assert_equal_in_end_with_true_or_false_re.search(logical_line))
|
||||
if res:
|
||||
yield (0, "O338: Use assertIn/NotIn(A, B) rather than "
|
||||
"assertEqual(A in B, True/False) when checking collection "
|
||||
"contents.")
|
||||
|
||||
|
||||
@core.flake8ext
|
||||
def no_log_warn(logical_line):
|
||||
"""Disallow 'LOG.warn('
|
||||
|
||||
@ -59,35 +59,6 @@ class HackingTestCase(base.BaseTestCase):
|
||||
def assertLineFails(self, func, *args):
|
||||
self.assertIsInstance(next(func(*args)), tuple)
|
||||
|
||||
def test_assert_true_instance(self):
|
||||
self.assertEqual(1, len(list(checks.assert_true_instance(
|
||||
"self.assertTrue(isinstance(e, "
|
||||
"exception.BuildAbortException))"))))
|
||||
|
||||
self.assertEqual(0, len(list(checks.assert_true_instance(
|
||||
"self.assertTrue()"))))
|
||||
|
||||
def test_assert_equal_or_not_none(self):
|
||||
self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
|
||||
"self.assertEqual(A, None)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
|
||||
"self.assertEqual(None, A)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
|
||||
"self.assertNotEqual(A, None)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_or_not_none(
|
||||
"self.assertNotEqual(None, A)"))))
|
||||
|
||||
self.assertEqual(0,
|
||||
len(list(checks.assert_equal_or_not_none(
|
||||
"self.assertIsNone()"))))
|
||||
|
||||
self.assertEqual(0,
|
||||
len(list(checks.assert_equal_or_not_none(
|
||||
"self.assertIsNotNone()"))))
|
||||
|
||||
def test_no_mutable_default_args(self):
|
||||
self.assertEqual(0, len(list(checks.no_mutable_default_args(
|
||||
"def foo (bar):"))))
|
||||
@ -96,43 +67,6 @@ class HackingTestCase(base.BaseTestCase):
|
||||
self.assertEqual(1, len(list(checks.no_mutable_default_args(
|
||||
"def foo (bar={}):"))))
|
||||
|
||||
def test_assert_equal_in(self):
|
||||
self.assertEqual(1, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(a in b, True)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual('str' in 'string', True)"))))
|
||||
|
||||
self.assertEqual(0, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(any(a==1 for a in b), True)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(True, a in b)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(True, 'str' in 'string')"))))
|
||||
|
||||
self.assertEqual(0, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(True, any(a==1 for a in b))"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(a in b, False)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual('str' in 'string', False)"))))
|
||||
|
||||
self.assertEqual(0, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(any(a==1 for a in b), False)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(False, a in b)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(False, 'str' in 'string')"))))
|
||||
|
||||
self.assertEqual(0, len(list(checks.assert_equal_in(
|
||||
"self.assertEqual(False, any(a==1 for a in b))"))))
|
||||
|
||||
def test_assert_equal_true_or_false(self):
|
||||
self.assertEqual(1, len(list(checks.assert_equal_true_or_false(
|
||||
"self.assertEqual(True, A)"))))
|
||||
|
||||
3
tox.ini
3
tox.ini
@ -177,11 +177,8 @@ import_exceptions = octavia.i18n
|
||||
|
||||
[flake8:local-plugins]
|
||||
extension =
|
||||
O316 = checks:assert_true_instance
|
||||
O318 = checks:assert_equal_or_not_none
|
||||
O323 = checks:assert_equal_true_or_false
|
||||
O324 = checks:no_mutable_default_args
|
||||
O338 = checks:assert_equal_in
|
||||
O339 = checks:no_log_warn
|
||||
O341 = checks:no_translate_logs
|
||||
O342 = checks:check_raised_localized_exceptions
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user