Add hacking check to ensure LOG.warn is not used
LOG.warn is replaced with LOG.warning in review: https://review.openstack.org/#/c/343214/. But use of LOG.warn should not happen again. So added hacking check to ensure LOG.warn is not used Change-Id: Ice650ba14386b136e72d1b91c0411b390ebfd429 Closes-Bug: #1605711
This commit is contained in:
parent
bf257d153d
commit
91341e6f74
@ -14,6 +14,7 @@ Senlin Specific Commandments
|
||||
directly.
|
||||
- [S320] Default arguments of a method should not be mutable.
|
||||
- [S321] The api_version decorator has to be the first decorator on a method.
|
||||
- [S322] LOG.warn is deprecated. Enforce use of LOG.warning.
|
||||
|
||||
Working on APIs
|
||||
---------------
|
||||
|
@ -48,6 +48,20 @@ def no_mutable_default_args(logical_line):
|
||||
yield (0, msg)
|
||||
|
||||
|
||||
def no_log_warn(logical_line):
|
||||
"""Disallow 'LOG.warn('
|
||||
|
||||
Deprecated LOG.warn(), instead use LOG.warning
|
||||
https://bugs.launchpad.net/senlin/+bug/1508442
|
||||
|
||||
N352
|
||||
"""
|
||||
|
||||
msg = ("S322: LOG.warn is deprecated, please use LOG.warning!")
|
||||
if "LOG.warn(" in logical_line:
|
||||
yield (0, msg)
|
||||
|
||||
|
||||
def check_api_version_decorator(logical_line, previous_logical, blank_before,
|
||||
filename):
|
||||
msg = ("S321: The api_version decorator must be the first decorator on "
|
||||
@ -61,4 +75,5 @@ def factory(register):
|
||||
register(assert_equal_none)
|
||||
register(use_jsonutils)
|
||||
register(no_mutable_default_args)
|
||||
register(no_log_warn)
|
||||
register(check_api_version_decorator)
|
||||
|
@ -19,6 +19,27 @@ from senlin.tests.unit.common import base
|
||||
|
||||
|
||||
class HackingTestCase(base.SenlinTestCase):
|
||||
@mock.patch('pep8._checks',
|
||||
{'physical_line': {}, 'logical_line': {}, 'tree': {}})
|
||||
def _run_check(self, code, checker, filename=None):
|
||||
pep8.register_check(checker)
|
||||
|
||||
lines = textwrap.dedent(code).strip().splitlines(True)
|
||||
|
||||
checker = pep8.Checker(filename=filename, lines=lines)
|
||||
checker.check_all()
|
||||
checker.report._deferred_print.sort()
|
||||
return checker.report._deferred_print
|
||||
|
||||
def _assert_has_errors(self, code, checker, expected_errors=None,
|
||||
filename=None):
|
||||
actual_errors = [e[:3] for e in
|
||||
self._run_check(code, checker, filename)]
|
||||
self.assertEqual(expected_errors or [], actual_errors)
|
||||
|
||||
def _assert_has_no_errors(self, code, checker, filename=None):
|
||||
self._assert_has_errors(code, checker, filename=filename)
|
||||
|
||||
def test_assert_equal_none(self):
|
||||
self.assertEqual(1, len(list(checks.assert_equal_none(
|
||||
"self.assertEqual(A, None)"))))
|
||||
@ -53,8 +74,6 @@ class HackingTestCase(base.SenlinTestCase):
|
||||
self.assertEqual(0, len(list(checks.no_mutable_default_args(
|
||||
"defined, undefined = [], {}"))))
|
||||
|
||||
@mock.patch("pep8._checks",
|
||||
{'physical_line': {}, 'logical_line': {}, 'tree': {}})
|
||||
def test_api_version_decorator(self):
|
||||
code = """
|
||||
@some_other_decorator
|
||||
@ -63,14 +82,8 @@ class HackingTestCase(base.SenlinTestCase):
|
||||
pass
|
||||
"""
|
||||
|
||||
lines = textwrap.dedent(code).strip().splitlines(True)
|
||||
|
||||
pep8.register_check(checks.check_api_version_decorator)
|
||||
checker = pep8.Checker(filename=None, lines=lines)
|
||||
checker.check_all()
|
||||
checker.report._deferred_print.sort()
|
||||
|
||||
actual_error = checker.report._deferred_print[0]
|
||||
actual_error = self._run_check(code,
|
||||
checks.check_api_version_decorator)[0]
|
||||
|
||||
self.assertEqual(2, actual_error[0])
|
||||
self.assertEqual(0, actual_error[1])
|
||||
@ -79,8 +92,6 @@ class HackingTestCase(base.SenlinTestCase):
|
||||
'decorator on a method.',
|
||||
actual_error[3])
|
||||
|
||||
@mock.patch("pep8._checks",
|
||||
{'physical_line': {}, 'logical_line': {}, 'tree': {}})
|
||||
def test_api_version_decorator_good(self):
|
||||
code = """
|
||||
class SomeController():
|
||||
@ -89,12 +100,19 @@ class HackingTestCase(base.SenlinTestCase):
|
||||
pass
|
||||
|
||||
"""
|
||||
lines = textwrap.dedent(code).strip().splitlines(True)
|
||||
|
||||
pep8.register_check(checks.check_api_version_decorator)
|
||||
checker = pep8.Checker(filename=None, lines=lines)
|
||||
checker.check_all()
|
||||
checker.report._deferred_print.sort()
|
||||
|
||||
actual_error = checker.report._deferred_print
|
||||
actual_error = self._run_check(code,
|
||||
checks.check_api_version_decorator)
|
||||
self.assertEqual(0, len(actual_error))
|
||||
|
||||
def test_no_log_warn(self):
|
||||
code = """
|
||||
LOG.warn("LOG.warn is deprecated")
|
||||
"""
|
||||
errors = [(1, 0, 'S322')]
|
||||
self._assert_has_errors(code, checks.no_log_warn,
|
||||
expected_errors=errors)
|
||||
code = """
|
||||
LOG.warning("LOG.warn is deprecated")
|
||||
"""
|
||||
self._assert_has_no_errors(code, checks.no_log_warn)
|
||||
|
Loading…
Reference in New Issue
Block a user