commit a9e1a21545a4016f865f2c97014db6e9e6ae9206 Author: Damian Johnson atagar@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.