[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [stem/master] Running PEP8 checks under a '--style' arg
commit 662a1e00746ecbfdece8136b92f9da9494007ad5
Author: Damian Johnson <atagar@xxxxxxxxxxxxxx>
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
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits