diff --git a/HACKING.rst b/HACKING.rst index 284fe3479d..ca3a79b347 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -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. \ No newline at end of file diff --git a/magnum/hacking/__init__.py b/magnum/hacking/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/magnum/hacking/checks.py b/magnum/hacking/checks.py new file mode 100644 index 0000000000..65d9dbd613 --- /dev/null +++ b/magnum/hacking/checks.py @@ -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) diff --git a/magnum/tests/unit/test_hacking.py b/magnum/tests/unit/test_hacking.py new file mode 100644 index 0000000000..bcd65e61e0 --- /dev/null +++ b/magnum/tests/unit/test_hacking.py @@ -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")]) diff --git a/tox.ini b/tox.ini index 7369782070..fb360910af 100644 --- a/tox.ini +++ b/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