[tor-commits] [arm/master] Checking pep8 as part of our tests
atagar at torproject.org
atagar at torproject.org
Wed Jan 1 22:19:15 UTC 2014
commit a9e1a21545a4016f865f2c97014db6e9e6ae9206
Author: Damian Johnson <atagar at torproject.org>
Date: Wed Jan 1 13:08:20 2014 -0800
Checking pep8 as part of our tests
This presently blacklists all of the files we haven't rewritten yet (which is
most of them). Pep8 works on a directory level so unfortunately we need to skip
after the actual check (so this still adds a substantial bit to our test
runtime).
---
run_tests.py | 152 ++++++++++++++++++++++++++++++++++++++++++++---------
test/settings.cfg | 47 +++++++++++++++++
2 files changed, 173 insertions(+), 26 deletions(-)
diff --git a/run_tests.py b/run_tests.py
index 0135708..3ed5e25 100755
--- a/run_tests.py
+++ b/run_tests.py
@@ -18,6 +18,7 @@ from arm.util import load_settings
CONFIG = stem.util.conf.config_dict("test", {
"pep8.ignore": [],
+ "pep8.blacklist": [],
"pyflakes.ignore": [],
})
@@ -29,6 +30,41 @@ SRC_PATHS = [os.path.join(ARM_BASE, path) for path in (
)]
+def main():
+ load_settings()
+
+ test_config = stem.util.conf.get_config("test")
+ test_config.load(os.path.join(ARM_BASE, "test", "settings.cfg"))
+
+ clean_orphaned_pyc()
+
+ tests = unittest.defaultTestLoader.discover('test', pattern='*.py')
+ test_runner = unittest.TextTestRunner()
+ test_runner.run(tests)
+
+ print
+
+ static_check_issues = {}
+
+ if stem.util.system.is_available("pyflakes"):
+ static_check_issues.update(get_pyflakes_issues(SRC_PATHS))
+
+ if stem.util.system.is_available("pep8"):
+ static_check_issues.update(get_stylistic_issues(SRC_PATHS))
+
+ if static_check_issues:
+ print "STATIC CHECKS"
+
+ for file_path in static_check_issues:
+ print "* %s" % file_path
+
+ for line_number, msg in static_check_issues[file_path]:
+ line_count = "%-4s" % line_number
+ print " line %s - %s" % (line_count, msg)
+
+ print
+
+
def clean_orphaned_pyc():
for root, _, files in os.walk(os.path.dirname(__file__)):
for filename in files:
@@ -43,6 +79,80 @@ def clean_orphaned_pyc():
os.remove(pyc_path)
+def get_stylistic_issues(paths):
+ """
+ Checks for stylistic issues that are an issue according to the parts of PEP8
+ we conform to. This alsochecks a few other stylistic issues:
+
+ * two space indentations
+ * tabs are the root of all evil and should be shot on sight
+ * standard newlines (\\n), not windows (\\r\\n) nor classic mac (\\r)
+ * checks that we're using 'as' for exceptions rather than a comma
+
+ :param list paths: paths to search for stylistic issues
+
+ :returns: **dict** of the form ``path => [(line_number, message)...]``
+ """
+
+ ignored_issues = ','.join(CONFIG["pep8.ignore"])
+ issues = {}
+
+ for path in paths:
+ pep8_output = stem.util.system.call(
+ "pep8 --ignore %s %s" % (ignored_issues, path),
+ ignore_exit_status = True,
+ )
+
+ for line in pep8_output:
+ line_match = re.match("^(.*):(\d+):(\d+): (.*)$", line)
+
+ if line_match:
+ path, line, _, issue = line_match.groups()
+
+ if path in CONFIG["pep8.blacklist"]:
+ continue
+
+ issues.setdefault(path, []).append((int(line), issue))
+
+ for file_path in _get_files_with_suffix(path):
+ if file_path in CONFIG["pep8.blacklist"]:
+ continue
+
+ with open(file_path) as f:
+ file_contents = f.read()
+
+ lines, file_issues, prev_indent = file_contents.split("\n"), [], 0
+ is_block_comment = False
+
+ for index, line in enumerate(lines):
+ whitespace, content = re.match("^(\s*)(.*)$", line).groups()
+
+ if '"""' in content:
+ is_block_comment = not is_block_comment
+
+ if "\t" in whitespace:
+ file_issues.append((index + 1, "indentation has a tab"))
+ elif "\r" in content:
+ file_issues.append((index + 1, "contains a windows newline"))
+ elif content != content.rstrip():
+ file_issues.append((index + 1, "line has trailing whitespace"))
+ elif content.lstrip().startswith("except") and content.endswith(", exc:"):
+ # Python 2.6 - 2.7 supports two forms for exceptions...
+ #
+ # except ValueError, exc:
+ # except ValueError as exc:
+ #
+ # The former is the old method and no longer supported in python 3
+ # going forward.
+
+ file_issues.append((index + 1, "except clause should use 'as', not comma"))
+
+ if file_issues:
+ issues[file_path] = file_issues
+
+ return issues
+
+
def get_pyflakes_issues(paths):
"""
Performs static checks via pyflakes.
@@ -97,36 +207,26 @@ def get_pyflakes_issues(paths):
return issues
-def main():
- load_settings()
-
- test_config = stem.util.conf.get_config("test")
- test_config.load(os.path.join(ARM_BASE, "test", "settings.cfg"))
-
- clean_orphaned_pyc()
-
- tests = unittest.defaultTestLoader.discover('test', pattern='*.py')
- test_runner = unittest.TextTestRunner()
- test_runner.run(tests)
-
- print
-
- static_check_issues = {}
-
- if stem.util.system.is_available("pyflakes"):
- static_check_issues.update(get_pyflakes_issues(SRC_PATHS))
+def _get_files_with_suffix(base_path, suffix = ".py"):
+ """
+ Iterates over files in a given directory, providing filenames with a certain
+ suffix.
- if static_check_issues:
- print "STATIC CHECKS"
+ :param str base_path: directory to be iterated over
+ :param str suffix: filename suffix to look for
- for file_path in static_check_issues:
- print "* %s" % file_path
+ :returns: iterator that yields the absolute path for files with the given suffix
+ """
- for line_number, msg in static_check_issues[file_path]:
- line_count = "%-4s" % line_number
- print " line %s - %s" % (line_count, msg)
+ if os.path.isfile(base_path):
+ if base_path.endswith(suffix):
+ yield base_path
+ else:
+ for root, _, files in os.walk(base_path):
+ for filename in files:
+ if filename.endswith(suffix):
+ yield os.path.join(root, filename)
- print
if __name__ == '__main__':
main()
diff --git a/test/settings.cfg b/test/settings.cfg
index cf0049c..169a016 100644
--- a/test/settings.cfg
+++ b/test/settings.cfg
@@ -1,3 +1,50 @@
+# PEP8 compliance issues that we're ignoreing.
+
+pep8.ignore E111
+pep8.ignore E121
+pep8.ignore E501
+pep8.ignore E251
+pep8.ignore E127
+
+# Files we haven't gotten around to revisiing yet. This list will be going away
+# completely before our next release.
+
+pep8.blacklist ./arm/connections/__init__.py
+pep8.blacklist ./arm/connections/circEntry.py
+pep8.blacklist ./arm/connections/connEntry.py
+pep8.blacklist ./arm/connections/connPanel.py
+pep8.blacklist ./arm/connections/countPopup.py
+pep8.blacklist ./arm/connections/descriptorPopup.py
+pep8.blacklist ./arm/connections/entries.py
+
+pep8.blacklist ./arm/graphing/__init__.py
+pep8.blacklist ./arm/graphing/bandwidthStats.py
+pep8.blacklist ./arm/graphing/connStats.py
+pep8.blacklist ./arm/graphing/graphPanel.py
+pep8.blacklist ./arm/graphing/resourceStats.py
+
+pep8.blacklist ./arm/menu/__init__.py
+pep8.blacklist ./arm/menu/actions.py
+pep8.blacklist ./arm/menu/item.py
+pep8.blacklist ./arm/menu/menu.py
+
+pep8.blacklist ./arm/util/connections.py
+pep8.blacklist ./arm/util/panel.py
+pep8.blacklist ./arm/util/sysTools.py
+pep8.blacklist ./arm/util/textInput.py
+pep8.blacklist ./arm/util/torConfig.py
+pep8.blacklist ./arm/util/torTools.py
+pep8.blacklist ./arm/util/uiTools.py
+
+pep8.blacklist ./arm/__init__.py
+pep8.blacklist ./arm/configPanel.py
+pep8.blacklist ./arm/controller.py
+pep8.blacklist ./arm/headerPanel.py
+pep8.blacklist ./arm/logPanel.py
+pep8.blacklist ./arm/popups.py
+pep8.blacklist ./arm/prereq.py
+pep8.blacklist ./arm/torrcPanel.py
+
# False positives from pyflakes. These are mappings between the path and the
# issue.
More information about the tor-commits
mailing list