commit 984b77b4f19592f3bc155f1fba895111ed9cba99 Author: Damian Johnson <atagar@torproject.org> Date: Wed Jan 1 16:30:44 2014 -0800 Use pyflakes' api rather than shelling out We've been shelling out to pyflakes and parsing its output. This is all well and good except that it's... well, silly. Pyflakes is a python library, and provides us with well structured data faster if we use it like so. This halves the runtime of our unit tests (from 6 to 3 seconds on my system)! --- run_tests.py | 2 +- test/settings.cfg | 1 + test/util.py | 101 +++++++++++++++++++++++++++++++++-------------------- 3 files changed, 65 insertions(+), 39 deletions(-) diff --git a/run_tests.py b/run_tests.py index 08143c1..0711b98 100755 --- a/run_tests.py +++ b/run_tests.py @@ -366,7 +366,7 @@ def _print_static_issues(args): # much overhead in including it with all tests. if args.run_unit or args.run_integ: - if stem.util.system.is_available("pyflakes"): + if test.util.is_pyflakes_available(): static_check_issues.update(test.util.get_pyflakes_issues(SRC_PATHS)) else: println("Static error checking requires pyflakes. Please install it from ...\n http://pypi.python.org/pypi/pyflakes\n", ERROR) diff --git a/test/settings.cfg b/test/settings.cfg index 1f40ef7..311d02e 100644 --- a/test/settings.cfg +++ b/test/settings.cfg @@ -142,6 +142,7 @@ pyflakes.ignore stem/util/str_tools.py => redefinition of function '_to_unicode_ pyflakes.ignore test/mocking.py => undefined name 'builtins' pyflakes.ignore test/mocking.py => undefined name 'test' pyflakes.ignore test/unit/response/events.py => 'from stem import *' used; unable to detect undefined names +pyflakes.ignore test/util.py => 'pyflakes' imported but unused # Test modules we want to run. Modules are roughly ordered by the dependencies # so the lowest level tests come first. This is because a problem in say, diff --git a/test/util.py b/test/util.py index c10a88e..706a5b2 100644 --- a/test/util.py +++ b/test/util.py @@ -9,6 +9,7 @@ Helper functions for our test framework. get_unit_tests - provides our unit tests get_integ_tests - provides our integration tests + is_pyflakes_available - checks if pyflakes is available get_prereq - provides the tor version required to run the given target get_torrc_entries - provides the torrc entries for a given target get_help_message - provides usage information for running our tests @@ -155,6 +156,20 @@ def get_help_message(): return help_msg +def is_pyflakes_available(): + """ + Checks if pyflakes is availalbe. + + :returns: **True** if we can use pyflakes and **False** otherwise + """ + + try: + import pyflakes + return True + except ImportError: + return False + + def get_prereq(target): """ Provides the tor version required to run the given target. If the target @@ -307,49 +322,19 @@ def get_pyflakes_issues(paths): :returns: dict of the form ``path => [(line_number, message)...]`` """ - pyflakes_ignore = {} + if not is_pyflakes_available(): + return {} - for line in CONFIG["pyflakes.ignore"]: - path, issue = line.split("=>") - pyflakes_ignore.setdefault(path.strip(), []).append(issue.strip()) + import pyflakes.api - def is_ignored(path, issue): - # Paths in pyflakes_ignore are relative, so we need to check to see if our - # path ends with any of them. - - for ignore_path in pyflakes_ignore: - if path.endswith(ignore_path) and issue in pyflakes_ignore[ignore_path]: - return True - - return False - - # Pyflakes issues are of the form... - # - # FILE:LINE: ISSUE - # - # ... for instance... - # - # stem/prereq.py:73: 'long_to_bytes' imported but unused - # stem/control.py:957: undefined name 'entry' - - issues = {} + reporter = PyflakesReporter() for path in paths: - pyflakes_output = stem.util.system.call( - "pyflakes %s" % path, - ignore_exit_status = True, - ) - - for line in pyflakes_output: - line_match = re.match("^(.*):(\d+): (.*)$", line) - - if line_match: - path, line, issue = line_match.groups() - - if not _is_test_data(path) and not is_ignored(path, issue): - issues.setdefault(path, []).append((int(line), issue)) + for file_path in _get_files_with_suffix(path): + if not _is_test_data(file_path): + pyflakes.api.checkPath(file_path, reporter) - return issues + return reporter.issues def check_stem_version(): @@ -566,6 +551,46 @@ def run_tasks(category, *tasks): println() +class PyflakesReporter(object): + """ + Implementation of the pyflakes.reporter.Reporter interface. This populates + our **issues** with a dictionary of the form... + + {path: [(line_number, issue)...], ...} + """ + + def __init__(self): + self.issues = {} + self.ignored_issues = {} + + for line in CONFIG["pyflakes.ignore"]: + path, issue = line.split("=>") + self.ignored_issues.setdefault(path.strip(), []).append(issue.strip()) + + def is_ignored(self, path, issue): + # Paths in pyflakes_ignore are relative, so we need to check to see if our + # path ends with any of them. + + for ignore_path in self.ignored_issues: + if path.endswith(ignore_path) and issue in self.ignored_issues[ignore_path]: + return True + + return False + + def unexpectedError(self, filename, msg): + self.register_issue(filename, None, msg) + + def syntaxError(self, filename, msg, lineno, offset, text): + self.register_issue(filename, lineno, msg) + + def flake(self, msg): + self.register_issue(msg.filename, msg.lineno, msg.message % msg.message_args) + + def register_issue(self, path, line_number, issue): + if not self.is_ignored(path, issue): + self.issues.setdefault(path, []).append((line_number, issue)) + + class Task(object): """ Task we can process while running our tests. The runner can return either a