commit 662a1e00746ecbfdece8136b92f9da9494007ad5 Author: Damian Johnson atagar@torproject.org Date: Sun Jan 6 19:26:00 2013 -0800
Running PEP8 checks under a '--style' arg
Well... damn. I had hoped that we would be able to wrap the PEP8 style checks into our normal testing runs (the reason check_whitespace.py works so well is that it's trivial to do with every test run).
However, pep8 takes on the order of ten seconds to run over our codebase. This is twice as long as all of our unit tests and hence too long to include in tests without opting in.
I'm keeping my fingers crossed that they're doing something stupid with ignored issues so we'll get better performance when we've made it happier. We'll see.
On the up side PEP8 is wonderfully configurable with respect to what it accepts and ignores so we can start using it right away. Also, the output is well formed so it was trivial to wrap it into our present style checker (which means consolidated and colorized output - yay!). --- run_tests.py | 26 +++++++++++++++++--------- test/check_whitespace.py | 38 ++++++++++++++++++++++++++++++++++++++ test/settings.cfg | 1 + 3 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/run_tests.py b/run_tests.py index 55948bd..9675706 100755 --- a/run_tests.py +++ b/run_tests.py @@ -69,13 +69,14 @@ import test.integ.util.proc import test.integ.util.system import test.integ.version
-OPT = "uit:l:c:h" -OPT_EXPANDED = ["unit", "integ", "targets=", "test=", "log=", "tor=", "config=", "help"] +OPT = "uist:l:c:h" +OPT_EXPANDED = ["unit", "integ", "style", "targets=", "test=", "log=", "tor=", "config=", "help"] DIVIDER = "=" * 70
CONFIG = stem.util.conf.config_dict("test", { "argument.unit": False, "argument.integ": False, + "argument.style": False, "argument.test": "", "argument.log": None, "argument.tor": "tor", @@ -189,6 +190,8 @@ def load_user_configuration(test_config): arg_overrides["argument.unit"] = "true" elif opt in ("-i", "--integ"): arg_overrides["argument.integ"] = "true" + elif opt in ("-s", "--style"): + arg_overrides["argument.style"] = "true" elif opt in ("-c", "--config"): config_path = os.path.abspath(arg) elif opt in ("-t", "--targets"): @@ -458,17 +461,22 @@ if __name__ == '__main__': # TODO: note unused config options afterward?
base_path = os.path.sep.join(__file__.split(os.path.sep)[:-1]) - whitespace_issues = test.check_whitespace.get_issues(os.path.join(base_path, "stem")) - whitespace_issues.update(test.check_whitespace.get_issues(os.path.join(base_path, "test"))) - whitespace_issues.update(test.check_whitespace.get_issues(os.path.join(base_path, "run_tests.py"))) + style_issues = test.check_whitespace.get_issues(os.path.join(base_path, "stem")) + style_issues.update(test.check_whitespace.get_issues(os.path.join(base_path, "test"))) + style_issues.update(test.check_whitespace.get_issues(os.path.join(base_path, "run_tests.py")))
- if whitespace_issues: - test.output.print_line("WHITESPACE ISSUES", term.Color.BLUE, term.Attr.BOLD) + if CONFIG["argument.style"]: + style_issues.update(test.check_whitespace.pep8_issues(os.path.join(base_path, "stem"))) + style_issues.update(test.check_whitespace.pep8_issues(os.path.join(base_path, "test"))) + style_issues.update(test.check_whitespace.pep8_issues(os.path.join(base_path, "run_tests.py"))) + + if style_issues: + test.output.print_line("STYLE ISSUES", term.Color.BLUE, term.Attr.BOLD)
- for file_path in whitespace_issues: + for file_path in style_issues: test.output.print_line("* %s" % file_path, term.Color.BLUE, term.Attr.BOLD)
- for line_number, msg in whitespace_issues[file_path]: + for line_number, msg in style_issues[file_path]: line_count = "%-4s" % line_number test.output.print_line(" line %s - %s" % (line_count, msg))
diff --git a/test/check_whitespace.py b/test/check_whitespace.py index 7f7b46e..f9008f5 100644 --- a/test/check_whitespace.py +++ b/test/check_whitespace.py @@ -20,9 +20,47 @@ from __future__ import with_statement import re import os
+from stem.util import system + # if ran directly then run over everything one level up DEFAULT_TARGET = os.path.sep.join(__file__.split(os.path.sep)[:-1])
+def pep8_issues(base_path = DEFAULT_TARGET): + """ + Checks for stylistic issues that are an issue according to the parts of PEP8 + we conform to. + + :param str base_path: directory to be iterated over + + :returns: dict of the form ``path => [(line_number, message)...]`` + """ + + # pep8 give output of the form... + # + # FILE:LINE:CHARACTER ISSUE + # + # ... for instance... + # + # ./test/mocking.py:868:31: E225 missing whitespace around operator + + # TODO: Presently this is a list of all issues pep8 complains about in stem. + # We're gonna trim these down by cateogry but include the pep8 checks to + # prevent regression. + + ignored_issues = "E111,E121,W293,E501,E302,E701,E251,E261,W391,E127,E241,E128,E226,E231,E202,E201,E203,E124,E211,E222,E225,E221,E126,E262,E271,E502,E303,E711" + + issues = {} + pep8_output = system.call("pep8 --ignore %s %s" % (ignored_issues, base_path)) + + for line in pep8_output: + line_match = re.match("^(.*):(\d+):(\d+): (.*)$", line) + + if line_match: + path, line, _, issue = line_match.groups() + issues.setdefault(path, []).append((int(line), issue)) + + return issues + def get_issues(base_path = DEFAULT_TARGET): """ Checks python source code in the given directory for whitespace issues. diff --git a/test/settings.cfg b/test/settings.cfg index 9aadcda..cf827cd 100644 --- a/test/settings.cfg +++ b/test/settings.cfg @@ -38,6 +38,7 @@
argument.unit false argument.integ false +argument.style false argument.test argument.log argument.tor tor