Add hacking rule framework for magnum
This patch adds hacking rule check framework for magnum, and adds first rule: policy.enforce_wsgi decorator must be the first decorator on a method. refer this link for why we need this rule. `https://review.openstack.org/#/c/190140/` Closes-Bugs: #1465895 Change-Id: If98e47426b391b75755ca0b559aee1baa93b8503
This commit is contained in:
parent
e8a77b6e1b
commit
de7da996b5
@ -1,4 +1,11 @@
|
||||
magnum Style Commandments
|
||||
=========================
|
||||
|
||||
Read the OpenStack Style Commandments http://docs.openstack.org/developer/hacking/
|
||||
- Step 1: Read the OpenStack Style Commandments
|
||||
http://docs.openstack.org/developer/hacking/
|
||||
- Step 2: Read on
|
||||
|
||||
magnum Specific Commandments
|
||||
---------------------------
|
||||
|
||||
- [N301] policy.enforce_wsgi decorator must be the first decorator on a method.
|
0
magnum/hacking/__init__.py
Normal file
0
magnum/hacking/__init__.py
Normal file
47
magnum/hacking/checks.py
Normal file
47
magnum/hacking/checks.py
Normal file
@ -0,0 +1,47 @@
|
||||
# Copyright (c) 2015 Intel, Inc.
|
||||
# All Rights Reserved.
|
||||
#
|
||||
# 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.
|
||||
|
||||
import re
|
||||
|
||||
"""
|
||||
Guidelines for writing new hacking checks
|
||||
|
||||
- Use only for Magnum specific tests. OpenStack general tests
|
||||
should be submitted to the common 'hacking' module.
|
||||
- Pick numbers in the range N3xx. Find the current test with
|
||||
the highest allocated number and then pick the next value.
|
||||
- Keep the test method code in the source file ordered based
|
||||
on the N3xx value.
|
||||
- List the new rule in the top level HACKING.rst file
|
||||
- Add test cases for each new rule to magnum/tests/unit/test_hacking.py
|
||||
|
||||
"""
|
||||
|
||||
enforce_re = re.compile(r"@policy.enforce_wsgi*")
|
||||
decorator_re = re.compile(r"@.*")
|
||||
|
||||
|
||||
def check_policy_enforce_decorator(logical_line,
|
||||
previous_logical, blank_before,
|
||||
filename):
|
||||
msg = ("N301: the policy.enforce_wsgi decorator must be the "
|
||||
"first decorator on a method.")
|
||||
if (blank_before == 0 and re.match(enforce_re, logical_line)
|
||||
and re.match(decorator_re, previous_logical)):
|
||||
yield(0, msg)
|
||||
|
||||
|
||||
def factory(register):
|
||||
register(check_policy_enforce_decorator)
|
85
magnum/tests/unit/test_hacking.py
Normal file
85
magnum/tests/unit/test_hacking.py
Normal file
@ -0,0 +1,85 @@
|
||||
# Copyright 2915 Intel, Inc.
|
||||
#
|
||||
# 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.
|
||||
|
||||
import mock
|
||||
import pep8
|
||||
import textwrap
|
||||
|
||||
from magnum.hacking import checks
|
||||
from magnum.tests import base
|
||||
|
||||
|
||||
class HackingTestCase(base.TestCase):
|
||||
"""This class tests the hacking checks in magnum.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.
|
||||
"""
|
||||
# We are patching pep8 so that only the check under test is actually
|
||||
# installed.
|
||||
|
||||
@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_policy_enforce_decorator(self):
|
||||
code = """
|
||||
@some_other_decorator
|
||||
@policy.enforce_wsgi("bay", "create")
|
||||
def my_method():
|
||||
pass
|
||||
"""
|
||||
self._assert_has_errors(code, checks.check_policy_enforce_decorator,
|
||||
expected_errors=[(2, 0, "N301")])
|
3
tox.ini
3
tox.ini
@ -56,6 +56,9 @@ commands =
|
||||
ignore = E131,E251,H302,H405,H803,H904,E711
|
||||
exclude = .venv,.git,.tox,dist,doc,*openstack/common*,*lib/python*,*egg,build,tools,magnum/common/pythonk8sclient
|
||||
|
||||
[hacking]
|
||||
local-check-factory = magnum.hacking.checks.factory
|
||||
|
||||
[testenv:pip-missing-reqs]
|
||||
# do not install test-requirements as that will pollute the virtualenv for
|
||||
# determining missing packages
|
||||
|
Loading…
Reference in New Issue
Block a user