From 637740a4f1aa64e192c05638b76ad2355909e346 Mon Sep 17 00:00:00 2001 From: Steven Dake Date: Wed, 27 Feb 2013 10:26:46 -0700 Subject: [PATCH] Update infrastructure to more closely match other OpenStack projects This patch imports nova's HACKING.rst guidelines altered for Heat. This patch imports nova's run_pep8.sh with some minor alterations. This patch imports nova's hacking.py extensions to pep8. This patch places the run_pep8 logic in a separate file rather than having it spread between tox.ini and run_tests.sh. The HACKING.rst requirements as well as general pep8 requirements are then enforced with run_pep8.sh. Avoid running once the N802 rule until dependency changelogs are less then 50 characters. Change-Id: If46c14668eae3cf11b8fbff46d766cc589d701aa Fixes: Bug #1131275 --- HACKING.rst | 162 ++++++++++--- run_tests.sh | 6 +- tools/hacking.py | 596 ++++++++++++++++++++++++++++++++++++++++++++++ tools/run_pep8.sh | 17 ++ tox.ini | 2 +- 5 files changed, 746 insertions(+), 37 deletions(-) create mode 100755 tools/hacking.py create mode 100755 tools/run_pep8.sh diff --git a/HACKING.rst b/HACKING.rst index a7e691113f..2692e7af5f 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -1,4 +1,4 @@ -Glance Style Commandments +Heat Style Commandments ======================= - Step 1: Read http://www.python.org/dev/peps/pep-0008/ @@ -9,18 +9,62 @@ Glance Style Commandments General ------- - Put two newlines between top-level code (funcs, classes, etc) +- Use only UNIX style newlines ("\n"), not Windows style ("\r\n") - Put one newline between methods in classes and anywhere else +- Long lines should be wrapped in parentheses + in preference to using a backslash for line continuation. - Do not write "except:", use "except Exception:" at the very least - Include your name with TODOs as in "#TODO(termie)" -- Do not name anything the same name as a built-in or reserved word -- Do declare variable names that conflict with the debugger, such as the letter 'c' +- Do not shadow a built-in or reserved word. Example:: + + def list(): + return [1, 2, 3] + + mylist = list() # BAD, shadows `list` built-in + + class Foo(object): + def list(self): + return [1, 2, 3] + + mylist = Foo().list() # OKAY, does not shadow built-in + +- Use the "is not" operator when testing for unequal identities. Example:: + + if not X is Y: # BAD, intended behavior is ambiguous + pass + + if X is not Y: # OKAY, intuitive + pass + +- Use the "not in" operator for evaluating membership in a collection. Example:: + + if not X in Y: # BAD, intended behavior is ambiguous + pass + + if X not in Y: # OKAY, intuitive + pass + + if not (X in Y or X in Z): # OKAY, still better than all those 'not's + pass + Imports ------- +- Do not import objects, only modules (*) +- Do not import more than one module per line (*) +- Do not use wildcard ``*`` import (*) - Do not make relative imports +- Do not make new heat.db imports in heat/virt/* - Order your imports by the full module path - Organize your imports according to the following template +(*) exceptions are: + +- imports from ``migrate`` package +- imports from ``sqlalchemy`` package +- imports from ``heat.db.sqlalchemy.session`` module +- imports from ``heat.db.sqlalchemy.migration.versioning_api`` package + Example:: # vim: tabstop=4 shiftwidth=4 softtabstop=4 @@ -28,7 +72,7 @@ Example:: \n {{third-party lib imports in human alphabetical order}} \n - {{glance imports in human alphabetical order}} + {{heat imports in human alphabetical order}} \n \n {{begin your code}} @@ -48,38 +92,30 @@ Example:: import eventlet import webob.exc - import glance.api.middleware - from glance.api import images - from glance.auth import users - import glance.common - from glance.endpoint import cloud - from glance import test + import heat.api.ec2 + from heat.api import openstack + from heat.auth import users + from heat.endpoint import cloud + import heat.flags + from heat import test Docstrings ---------- - -Docstrings are required for all functions and methods. - -Docstrings should ONLY use triple-double-quotes (``"""``) - -Single-line docstrings should NEVER have extraneous whitespace -between enclosing triple-double-quotes. - -**INCORRECT** :: - - """ There is some whitespace between the enclosing quotes :( """ - -**CORRECT** :: - - """There is no whitespace between the enclosing quotes :)""" - -Docstrings that span more than one line should look like this: - Example:: - """ - Start the docstring on the line following the opening triple-double-quote + """A one line docstring looks like this and ends in a period.""" + + + """A multi line docstring has a one-line summary, less than 80 characters. + + Then a new paragraph after a newline that explains in more detail any + general information about the function, class or method. Example usages + are also great to have here if it is a complex class for function. + + When writing the docstring for a class, an extra line should be placed + after the closing quotations. For more in-depth explanations for these + decisions see http://www.python.org/dev/peps/pep-0257/ If you are going to describe parameters and return values, use Sphinx, the appropriate syntax is as follows. @@ -91,8 +127,6 @@ Example:: :raises: AttributeError, KeyError """ -**DO NOT** leave an extra newline before the closing triple-double-quote. - Dictionaries/Lists ------------------ @@ -185,6 +219,29 @@ bug that had no unit test, a new passing unit test should be added. If a submitted bug fix does have a unit test, be sure to add a new one that fails without the patch and passes with the patch. +For more information on creating unit tests and utilizing the testing +infrastructure in OpenStack Heat, please read heat/testing/README.rst. + + +Running Tests +------------- +The testing system is based on a combination of tox and testr. The canonical +approach to running tests is to simply run the command `tox`. This will +create virtual environments, populate them with depenedencies and run all of +the tests that OpenStack CI systems run. Behind the scenes, tox is running +`testr run --parallel`, but is set up such that you can supply any additional +testr arguments that are needed to tox. For example, you can run: +`tox -- --analyze-isolation` to cause tox to tell testr to add +--analyze-isolation to its argument list. + +It is also possible to run the tests inside of a virtual environment +you have created, or it is possible that you have all of the dependencies +installed locally already. In this case, you can interact with the testr +command directly. Running `testr run` will run the entire test suite. `testr +run --parallel` will run it in parallel (this is the default incantation tox +uses.) More information about testr can be found at: +http://wiki.openstack.org/testr + openstack-common ---------------- @@ -199,3 +256,44 @@ with the help of openstack-common's update.py script. See: The copy of the code should never be directly modified here. Please always update openstack-common first and then run the script to copy the changes across. + +OpenStack Trademark +------------------- + +OpenStack is a registered trademark of the OpenStack Foundation, and uses the +following capitalization: + + OpenStack + + +Commit Messages +--------------- +Using a common format for commit messages will help keep our git history +readable. Follow these guidelines: + + First, provide a brief summary (it is recommended to keep the commit title + under 50 chars). + + The first line of the commit message should provide an accurate + description of the change, not just a reference to a bug or + blueprint. It must be followed by a single blank line. + + If the change relates to a specific driver (libvirt, xenapi, qpid, etc...), + begin the first line of the commit message with the driver name, lowercased, + followed by a colon. + + Following your brief summary, provide a more detailed description of + the patch, manually wrapping the text at 72 characters. This + description should provide enough detail that one does not have to + refer to external resources to determine its high-level functionality. + + Once you use 'git review', two lines will be appended to the commit + message: a blank line followed by a 'Change-Id'. This is important + to correlate this commit with a specific review in Gerrit, and it + should not be modified. + +For further information on constructing high quality commit messages, +and how to split up commits into a series of changes, consult the +project wiki: + + http://wiki.openstack.org/GitCommitMessages diff --git a/run_tests.sh b/run_tests.sh index feda1934d0..8a5dbc9816 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -46,10 +46,8 @@ function run_tests { } function run_pep8 { - echo "Running pep8..." - PEP8_OPTIONS="--exclude=$PEP8_EXCLUDE --repeat" - PEP8_INCLUDE="bin/heat-cfn bin/heat-boto bin/heat-api-cfn bin/heat-api bin/heat-engine heat tools setup.py heat/testing/runner.py" - ${wrapper} pep8 $PEP8_OPTIONS $PEP8_INCLUDE + echo "Running PEP8 and HACKING compliance check..." + bash -c "${wrapper} tools/run_pep8.sh" } # run unit tests with pep8 when no arguments are specified diff --git a/tools/hacking.py b/tools/hacking.py new file mode 100755 index 0000000000..04a412e98a --- /dev/null +++ b/tools/hacking.py @@ -0,0 +1,596 @@ +#!/usr/bin/env python +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright (c) 2012, Cloudscaling +# 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. + +"""heat HACKING file compliance testing + +Built on top of pep8.py +""" + +import imp +import inspect +import logging +import os +import re +import subprocess +import sys +import tokenize + +import pep8 + +# Don't need this for testing +logging.disable('LOG') + +#N1xx comments +#N2xx except +#N3xx imports +#N4xx docstrings +#N5xx dictionaries/lists +#N6xx calling methods +#N7xx localization +#N8xx git commit messages +#N9xx other + +IMPORT_EXCEPTIONS = ['sqlalchemy', 'migrate', 'heat.db.sqlalchemy.session', + 'heat.db.sqlalchemy.migration.versioning_api'] +# Paste is missing a __init__ in top level directory +START_DOCSTRING_TRIPLE = ['u"""', 'r"""', '"""', "u'''", "r'''", "'''"] +END_DOCSTRING_TRIPLE = ['"""', "'''"] +VERBOSE_MISSING_IMPORT = os.getenv('HACKING_VERBOSE_MISSING_IMPORT', 'False') + +_missingImport = set([]) + + +# Monkey patch broken excluded filter in pep8 +# See https://github.com/jcrocholl/pep8/pull/111 +def excluded(self, filename): + """Check if options.exclude contains a pattern that matches filename.""" + basename = os.path.basename(filename) + return any((pep8.filename_match(filename, self.options.exclude, + default=False), + pep8.filename_match(basename, self.options.exclude, + default=False))) + + +def input_dir(self, dirname): + """Check all files in this directory and all subdirectories.""" + dirname = dirname.rstrip('/') + if self.excluded(dirname): + return 0 + counters = self.options.report.counters + verbose = self.options.verbose + filepatterns = self.options.filename + runner = self.runner + for root, dirs, files in os.walk(dirname): + if verbose: + print('directory ' + root) + counters['directories'] += 1 + for subdir in sorted(dirs): + if self.excluded(os.path.join(root, subdir)): + dirs.remove(subdir) + for filename in sorted(files): + # contain a pattern that matches? + if ((pep8.filename_match(filename, filepatterns) and + not self.excluded(filename))): + runner(os.path.join(root, filename)) + + +def is_import_exception(mod): + return (mod in IMPORT_EXCEPTIONS or + any(mod.startswith(m + '.') for m in IMPORT_EXCEPTIONS)) + + +def import_normalize(line): + # convert "from x import y" to "import x.y" + # handle "from x import y as z" to "import x.y as z" + split_line = line.split() + if ("import" in line and line.startswith("from ") and "," not in line and + split_line[2] == "import" and split_line[3] != "*" and + split_line[1] != "__future__" and + (len(split_line) == 4 or + (len(split_line) == 6 and split_line[4] == "as"))): + return "import %s.%s" % (split_line[1], split_line[3]) + else: + return line + + +def nova_todo_format(physical_line, tokens): + """Check for 'TODO()'. + + nova HACKING guide recommendation for TODO: + Include your name with TODOs as in "#TODO(termie)" + + Okay: #TODO(sdague) + N101: #TODO fail + N101: #TODO (jogo) fail + """ + # TODO(sdague): TODO check shouldn't fail inside of space + pos = physical_line.find('TODO') + pos1 = physical_line.find('TODO(') + pos2 = physical_line.find('#') # make sure it's a comment + if (pos != pos1 and pos2 >= 0 and pos2 < pos and len(tokens) == 0): + return pos, "N101: Use TODO(NAME)" + + +def nova_except_format(logical_line): + r"""Check for 'except:'. + + nova HACKING guide recommends not using except: + Do not write "except:", use "except Exception:" at the very least + + Okay: except Exception: + N201: except: + """ + if logical_line.startswith("except:"): + yield 6, "N201: no 'except:' at least use 'except Exception:'" + + +def nova_except_format_assert(logical_line): + r"""Check for 'assertRaises(Exception'. + + nova HACKING guide recommends not using assertRaises(Exception...): + Do not use overly broad Exception type + + Okay: self.assertRaises(NovaException) + N202: self.assertRaises(Exception) + """ + if logical_line.startswith("self.assertRaises(Exception"): + yield 1, "N202: assertRaises Exception too broad" + + +modules_cache = dict((mod, True) for mod in tuple(sys.modules.keys()) + + sys.builtin_module_names) + +RE_RELATIVE_IMPORT = re.compile('^from\s*[.]') + + +def nova_import_rules(logical_line): + r"""Check for imports. + + nova HACKING guide recommends one import per line: + Do not import more than one module per line + + Examples: + Okay: from nova.compute import api + N301: from nova.compute import api, utils + + + Imports should usually be on separate lines. + + nova HACKING guide recommends importing only modules: + Do not import objects, only modules + + Examples: + Okay: from os import path + Okay: from os import path as p + Okay: from os import (path as p) + Okay: import os.path + Okay: from nova.compute import rpcapi + N302: from os.path import dirname as dirname2 + N302: from os.path import (dirname as dirname2) + N303: from os.path import * + N304: from .compute import rpcapi + """ + #NOTE(afazekas): An old style relative import example will not be able to + # pass the doctest, since the relativity depends on the file's locality + + def is_module_for_sure(mod, search_path=sys.path): + mod = mod.replace('(', '') # Ignore parentheses + try: + mod_name = mod + while '.' in mod_name: + pack_name, _sep, mod_name = mod.partition('.') + f, p, d = imp.find_module(pack_name, search_path) + search_path = [p] + imp.find_module(mod_name, search_path) + except ImportError: + try: + # NOTE(vish): handle namespace modules + module = __import__(mod) + except ImportError, exc: + # NOTE(vish): the import error might be due + # to a missing dependency + missing = str(exc).split()[-1] + if (missing != mod.split('.')[-1] or + "cannot import" in str(exc)): + _missingImport.add(missing) + return True + return False + return True + + def is_module(mod): + """Checks for non module imports.""" + if mod in modules_cache: + return modules_cache[mod] + res = is_module_for_sure(mod) + modules_cache[mod] = res + return res + + current_path = os.path.dirname(pep8.current_file) + current_mod = os.path.basename(pep8.current_file) + if current_mod[-3:] == ".py": + current_mod = current_mod[:-3] + + split_line = logical_line.split() + split_line_len = len(split_line) + if (split_line[0] in ('import', 'from') and split_line_len > 1 and + not is_import_exception(split_line[1])): + pos = logical_line.find(',') + if pos != -1: + if split_line[0] == 'from': + yield pos, "N301: one import per line" + return # ',' is not supported by the N302 checker yet + pos = logical_line.find('*') + if pos != -1: + yield pos, "N303: No wildcard (*) import." + return + + if split_line_len in (2, 4, 6) and split_line[1] != "__future__": + if 'from' == split_line[0] and split_line_len > 3: + mod = '.'.join((split_line[1], split_line[3])) + if is_import_exception(mod): + return + if RE_RELATIVE_IMPORT.search(logical_line): + yield logical_line.find('.'), ("N304: No " + "relative imports. '%s' is a relative import" + % logical_line) + return + + if not is_module(mod): + yield 0, ("N302: import only modules." + "'%s' does not import a module" % logical_line) + return + + #NOTE(afazekas): import searches first in the package + # The import keyword just imports modules + # The guestfs module now imports guestfs + mod = split_line[1] + if (current_mod != mod and + not is_module(mod) and + is_module_for_sure(mod, [current_path])): + yield 0, ("N304: No relative imports." + " '%s' is a relative import" + % logical_line) + + +#TODO(jogo): import template: N305 + + +def nova_import_alphabetical(logical_line, blank_lines, previous_logical, + indent_level, previous_indent_level): + r"""Check for imports in alphabetical order. + + nova HACKING guide recommendation for imports: + imports in human alphabetical order + + Okay: import os\nimport sys\n\nimport nova\nfrom nova import test + N306: import sys\nimport os + """ + # handle import x + # use .lower since capitalization shouldn't dictate order + split_line = import_normalize(logical_line.strip()).lower().split() + split_previous = import_normalize(previous_logical.strip()).lower().split() + + if blank_lines < 1 and indent_level == previous_indent_level: + length = [2, 4] + if (len(split_line) in length and len(split_previous) in length and + split_line[0] == "import" and split_previous[0] == "import"): + if split_line[1] < split_previous[1]: + yield (0, "N306: imports not in alphabetical order (%s, %s)" + % (split_previous[1], split_line[1])) + + +def nova_import_no_db_in_virt(logical_line, filename): + """Check for db calls from nova/virt + + As of grizzly-2 all the database calls have been removed from + nova/virt, and we want to keep it that way. + + N307 + """ + if "nova/virt" in filename and not filename.endswith("fake.py"): + if logical_line.startswith("from nova import db"): + yield (0, "N307: nova.db import not allowed in nova/virt/*") + + +def in_docstring_position(previous_logical): + return (previous_logical.startswith("def ") or + previous_logical.startswith("class ")) + + +def nova_docstring_start_space(physical_line, previous_logical): + r"""Check for docstring not start with space. + + nova HACKING guide recommendation for docstring: + Docstring should not start with space + + Okay: def foo():\n '''This is good.''' + N401: def foo():\n ''' This is not.''' + """ + # short circuit so that we don't fail on our own fail test + # when running under external pep8 + if physical_line.find("N401: def foo()") != -1: + return + + # it's important that we determine this is actually a docstring, + # and not a doc block used somewhere after the first line of a + # function def + if in_docstring_position(previous_logical): + pos = max([physical_line.find(i) for i in START_DOCSTRING_TRIPLE]) + if pos != -1 and len(physical_line) > pos + 4: + if physical_line[pos + 3] == ' ': + return (pos, "N401: docstring should not start with" + " a space") + + +def nova_docstring_one_line(physical_line): + r"""Check one line docstring end. + + nova HACKING guide recommendation for one line docstring: + A one line docstring looks like this and ends in punctuation. + + Okay: '''This is good.''' + Okay: '''This is good too!''' + Okay: '''How about this?''' + N402: '''This is not''' + N402: '''Bad punctuation,''' + """ + #TODO(jogo) make this apply to multi line docstrings as well + line = physical_line.lstrip() + + if line.startswith('"') or line.startswith("'"): + pos = max([line.find(i) for i in START_DOCSTRING_TRIPLE]) # start + end = max([line[-4:-1] == i for i in END_DOCSTRING_TRIPLE]) # end + + if pos != -1 and end and len(line) > pos + 4: + if line[-5] not in ['.', '?', '!']: + return pos, "N402: one line docstring needs punctuation." + + +def nova_docstring_multiline_end(physical_line, previous_logical): + r"""Check multi line docstring end. + + nova HACKING guide recommendation for docstring: + Docstring should end on a new line + + Okay: '''foobar\nfoo\nbar\n''' + N403: def foo():\n'''foobar\nfoo\nbar\n d'''\n\n + """ + if in_docstring_position(previous_logical): + pos = max(physical_line.find(i) for i in END_DOCSTRING_TRIPLE) + if pos != -1 and len(physical_line) == pos + 4: + if physical_line.strip() not in START_DOCSTRING_TRIPLE: + return (pos, "N403: multi line docstring end on new line") + + +def nova_docstring_multiline_start(physical_line, previous_logical, tokens): + r"""Check multi line docstring start with summary. + + nova HACKING guide recommendation for docstring: + Docstring should start with A multi line docstring has a one-line summary + + Okay: '''foobar\nfoo\nbar\n''' + N404: def foo():\n'''\nfoo\nbar\n''' \n\n + """ + if in_docstring_position(previous_logical): + pos = max([physical_line.find(i) for i in START_DOCSTRING_TRIPLE]) + # start of docstring when len(tokens)==0 + if len(tokens) == 0 and pos != -1 and len(physical_line) == pos + 4: + if physical_line.strip() in START_DOCSTRING_TRIPLE: + return (pos, "N404: multi line docstring " + "should start with a summary") + + +def nova_no_cr(physical_line): + r"""Check that we only use newlines not cariage returns. + + Okay: import os\nimport sys + # pep8 doesn't yet replace \r in strings, will work on an + # upstream fix + N901 import os\r\nimport sys + """ + pos = physical_line.find('\r') + if pos != -1 and pos == (len(physical_line) - 2): + return (pos, "N901: Windows style line endings not allowed in code") + + +FORMAT_RE = re.compile("%(?:" + "%|" # Ignore plain percents + "(\(\w+\))?" # mapping key + "([#0 +-]?" # flag + "(?:\d+|\*)?" # width + "(?:\.\d+)?" # precision + "[hlL]?" # length mod + "\w))") # type + + +class LocalizationError(Exception): + pass + + +def check_i18n(): + """Generator that checks token stream for localization errors. + + Expects tokens to be ``send``ed one by one. + Raises LocalizationError if some error is found. + """ + while True: + try: + token_type, text, _, _, line = yield + except GeneratorExit: + return + + if (token_type == tokenize.NAME and text == "_" and + not line.startswith('def _(msg):')): + + while True: + token_type, text, start, _, _ = yield + if token_type != tokenize.NL: + break + if token_type != tokenize.OP or text != "(": + continue # not a localization call + + format_string = '' + while True: + token_type, text, start, _, _ = yield + if token_type == tokenize.STRING: + format_string += eval(text) + elif token_type == tokenize.NL: + pass + else: + break + + if not format_string: + raise LocalizationError(start, + "N701: Empty localization string") + if token_type != tokenize.OP: + raise LocalizationError(start, + "N701: Invalid localization call") + if text != ")": + if text == "%": + raise LocalizationError(start, + "N702: Formatting operation should be outside" + " of localization method call") + elif text == "+": + raise LocalizationError(start, + "N702: Use bare string concatenation instead" + " of +") + else: + raise LocalizationError(start, + "N702: Argument to _ must be just a string") + + format_specs = FORMAT_RE.findall(format_string) + positional_specs = [(key, spec) for key, spec in format_specs + if not key and spec] + # not spec means %%, key means %(smth)s + if len(positional_specs) > 1: + raise LocalizationError(start, + "N703: Multiple positional placeholders") + + +def nova_localization_strings(logical_line, tokens): + r"""Check localization in line. + + Okay: _("This is fine") + Okay: _("This is also fine %s") + N701: _('') + N702: _("Bob" + " foo") + N702: _("Bob %s" % foo) + # N703 check is not quite right, disabled by removing colon + N703 _("%s %s" % (foo, bar)) + """ + # TODO(sdague) actually get these tests working + gen = check_i18n() + next(gen) + try: + map(gen.send, tokens) + gen.close() + except LocalizationError as e: + yield e.args + +#TODO(jogo) Dict and list objects + + +def nova_is_not(logical_line): + r"""Check localization in line. + + Okay: if x is not y + N901: if not X is Y + N901: if not X.B is Y + """ + split_line = logical_line.split() + if (len(split_line) == 5 and split_line[0] == 'if' and + split_line[1] == 'not' and split_line[3] == 'is'): + yield (logical_line.find('not'), "N901: Use the 'is not' " + "operator for when testing for unequal identities") + + +def nova_not_in(logical_line): + r"""Check localization in line. + + Okay: if x not in y + Okay: if not (X in Y or X is Z) + Okay: if not (X in Y) + N902: if not X in Y + N902: if not X.B in Y + """ + split_line = logical_line.split() + if (len(split_line) == 5 and split_line[0] == 'if' and + split_line[1] == 'not' and split_line[3] == 'in' and not + split_line[2].startswith('(')): + yield (logical_line.find('not'), "N902: Use the 'not in' " + "operator for collection membership evaluation") + +current_file = "" + + +def readlines(filename): + """Record the current file being tested.""" + pep8.current_file = filename + return open(filename).readlines() + + +def add_nova(): + """Monkey patch in nova guidelines. + + Look for functions that start with nova_ and have arguments + and add them to pep8 module + Assumes you know how to write pep8.py checks + """ + for name, function in globals().items(): + if not inspect.isfunction(function): + continue + args = inspect.getargspec(function)[0] + if args and name.startswith("nova"): + exec("pep8.%s = %s" % (name, name)) + + +imports_on_separate_lines_N301_compliant = r""" + Imports should usually be on separate lines. + + Okay: import os\nimport sys + E401: import sys, os + + N301: from subprocess import Popen, PIPE + Okay: from myclas import MyClass + Okay: from foo.bar.yourclass import YourClass + Okay: import myclass + Okay: import foo.bar.yourclass + """ + +if __name__ == "__main__": + #include nova path + sys.path.append(os.getcwd()) + #Run once tests (not per line) + #NOVA error codes start with an N + pep8.SELFTEST_REGEX = re.compile(r'(Okay|[EWN]\d{3}):\s(.*)') + pep8.ERRORCODE_REGEX = re.compile(r'[EWN]\d{3}') + add_nova() + pep8.current_file = current_file + pep8.readlines = readlines + pep8.StyleGuide.excluded = excluded + pep8.StyleGuide.input_dir = input_dir + # we need to kill this doctring otherwise the self tests fail + pep8.imports_on_separate_lines.__doc__ = \ + imports_on_separate_lines_N301_compliant + + try: + pep8._main() + finally: + if len(_missingImport) > 0: + print >> sys.stderr, ("%i imports missing in this test environment" + % len(_missingImport)) diff --git a/tools/run_pep8.sh b/tools/run_pep8.sh new file mode 100755 index 0000000000..f75db12aa8 --- /dev/null +++ b/tools/run_pep8.sh @@ -0,0 +1,17 @@ +#!/bin/bash + +set -e +# This is used by run_tests.sh and tox.ini +python tools/hacking.py --doctest + +# Until all these issues get fixed, ignore. +PEP8='python tools/hacking.py --ignore=N101,N201,N202,N301,N302,N303,N304,N305,N306,N401,N402,N403,N404,N702,N703,N801,N902' + +EXCLUDE='--exclude=.venv,.git,.tox,dist,doc,*lib/python*' +EXCLUDE+=',*egg,build,*tools*' + +# Check all .py files +${PEP8} ${EXCLUDE} . + +# Check binaries without py extension +${PEP8} bin/heat-api bin/heat-api-cfn bin/heat-api-cloudwatch bin/heat-cfn bin/heat-engine diff --git a/tox.ini b/tox.ini index 48abda5a30..d877253a0c 100644 --- a/tox.ini +++ b/tox.ini @@ -8,7 +8,7 @@ deps = -r{toxinidir}/tools/pip-requires commands = nosetests -a tag='unit' [testenv:pep8] -commands = pep8 --repeat --show-source --exclude openstack heat setup.py +commands = bash tools/run_pep8.sh [testenv:venv] commands = {posargs}