Set Hacking rules to check some tests assert
Octavia doesn't have hacking rules to check "assert" usages like: - assertTrue(isinstance(A, class A)) - assertEqual(A, None) or assertEqual(None, A) - assertEqual(False, A) or assertEqual(True, A) - assertEqual(a in b, True) or assertEqual(a in b, False) This patch introduce hacking rules just above. Change-Id: If64402508226608b5700c8bb11e924dbc778fec3 Closes-Bug: #1511368
This commit is contained in:
Bertrand Lallau
parent
734d61c51c
commit
f7a540b156
|
|
@ -8,10 +8,10 @@ As such, we tend to follow Neutron conventions regarding coding style.
|
|||
|
||||
Octavia Specific Commandments
|
||||
-----------------------------
|
||||
- [N316] Change assertTrue(isinstance(A, B)) by optimal assert like
|
||||
- [O316] Change assertTrue(isinstance(A, B)) by optimal assert like
|
||||
assertIsInstance(A, B).
|
||||
- [N320] Validate that LOG messages, except debug ones, have translations
|
||||
- [N334] Change assertTrue/False(A in/not in B, message) to the more specific
|
||||
- [O320] Validate that LOG messages, except debug ones, have translations
|
||||
- [O334] Change assertTrue/False(A in/not in B, message) to the more specific
|
||||
assertIn/NotIn(A, B, message)
|
||||
|
||||
Creating Unit Tests
|
||||
|
|
|
|||
|
|
@ -55,6 +55,18 @@ for level, hint in six.iteritems(_all_log_levels):
|
|||
}
|
||||
log_translation_hints.append(re.compile(r))
|
||||
|
||||
assert_trueinst_re = re.compile(
|
||||
r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
|
||||
"(\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,")
|
||||
|
||||
|
||||
def _directory_to_check_translation(filename):
|
||||
return True
|
||||
|
|
@ -119,7 +131,43 @@ def no_translate_debug_logs(logical_line, filename):
|
|||
"""
|
||||
if _directory_to_check_translation(filename) and logical_line.startswith(
|
||||
"LOG.debug(_("):
|
||||
yield(0, "O319 Don't translate debug level logs")
|
||||
yield(0, "O319 Don't translate debug level logs")
|
||||
|
||||
|
||||
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")
|
||||
|
||||
|
||||
def assert_equal_true_or_false(logical_line):
|
||||
"""Check for assertEqual(True, A) or assertEqual(False, A) sentences
|
||||
|
||||
O323
|
||||
"""
|
||||
res = (assert_equal_with_true_re.search(logical_line) or
|
||||
assert_equal_with_false_re.search(logical_line))
|
||||
if res:
|
||||
yield (0, "O323: assertEqual(True, A) or assertEqual(False, A) "
|
||||
"sentences not allowed")
|
||||
|
||||
|
||||
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.")
|
||||
|
||||
|
||||
def factory(register):
|
||||
|
|
@ -127,3 +175,6 @@ def factory(register):
|
|||
register(use_jsonutils)
|
||||
register(no_author_tags)
|
||||
register(no_translate_debug_logs)
|
||||
register(assert_true_instance)
|
||||
register(assert_equal_true_or_false)
|
||||
register(assert_equal_in)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,109 @@
|
|||
# Copyright 2015
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from oslotest import base
|
||||
|
||||
from octavia.hacking import checks
|
||||
|
||||
|
||||
class HackingTestCase(base.BaseTestCase):
|
||||
"""Hacking test class.
|
||||
|
||||
This class tests the hacking checks in octavia.hacking.checks by passing
|
||||
strings to the check methods like the pep8/flake8 parser would. The parser
|
||||
loops over each line in the file and then passes the parameters to the
|
||||
check method. The parameter names in the check method dictate what type of
|
||||
object is passed to the check method. The parameter types are::
|
||||
|
||||
logical_line: A processed line with the following modifications:
|
||||
- Multi-line statements converted to a single line.
|
||||
- Stripped left and right.
|
||||
- Contents of strings replaced with "xxx" of same length.
|
||||
- Comments removed.
|
||||
physical_line: Raw line of text from the input file.
|
||||
lines: a list of the raw lines from the input file
|
||||
tokens: the tokens that contribute to this logical line
|
||||
line_number: line number in the input file
|
||||
total_lines: number of lines in the input file
|
||||
blank_lines: blank lines before this one
|
||||
indent_char: indentation character in this file (" " or "\t")
|
||||
indent_level: indentation (with tabs expanded to multiples of 8)
|
||||
previous_indent_level: indentation on previous line
|
||||
previous_logical: previous logical line
|
||||
filename: Path of the file being run through pep8
|
||||
|
||||
When running a test on a check method the return will be False/None if
|
||||
there is no violation in the sample input. If there is an error a tuple is
|
||||
returned with a position in the line, and a message. So to check the result
|
||||
just assertTrue if the check is expected to fail and assertFalse if it
|
||||
should pass.
|
||||
"""
|
||||
|
||||
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_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)"))))
|
||||
|
||||
self.assertEqual(1, len(list(checks.assert_equal_true_or_false(
|
||||
"self.assertEqual(False, A)"))))
|
||||
|
||||
self.assertEqual(0, len(list(checks.assert_equal_true_or_false(
|
||||
"self.assertTrue()"))))
|
||||
|
||||
self.assertEqual(0, len(list(checks.assert_equal_true_or_false(
|
||||
"self.assertFalse()"))))
|
||||
Loading…
Reference in New Issue