commit a254e5ff8529db70756758782a9a2646a71a1d94 Author: Damian Johnson atagar@torproject.org Date: Thu Feb 13 09:06:50 2014 -0800
Moving pep8 and pyflakes helpers to stem.util.test_tools
Twice now I've copied the part of stem's test/util.py for performing pyflakes and pep8 checks (for both arm and ExitMap). I don't want to vend this functionality but it's silly for us to keep copying this functionality.
Generalizing the helper functions and moving them to our util. Like the log module, this has a note that it is not something we vend and that users should not rely on it to have a backward compatible API. --- run_tests.py | 15 +-- stem/util/__init__.py | 1 + stem/util/test_tools.py | 288 +++++++++++++++++++++++++++++++++++++++++++++++ test/settings.cfg | 5 +- test/util.py | 204 +-------------------------------- 5 files changed, 303 insertions(+), 210 deletions(-)
diff --git a/run_tests.py b/run_tests.py index ae10df9..2524756 100755 --- a/run_tests.py +++ b/run_tests.py @@ -20,6 +20,7 @@ import stem.util.conf import stem.util.enum import stem.util.log import stem.util.system +import stem.util.test_tools
import test.output import test.runner @@ -90,7 +91,7 @@ https://pypi.python.org/pypi/mock/
PYFLAKES_TASK = Task( "running pyflakes", - test.util.get_pyflakes_issues, + stem.util.test_tools.get_pyflakes_issues, args = (SRC_PATHS,), is_required = False, print_result = False, @@ -98,8 +99,8 @@ PYFLAKES_TASK = Task(
PEP8_TASK = Task( "running pep8", - test.util.get_stylistic_issues, - args = (SRC_PATHS,), + stem.util.test_tools.get_stylistic_issues, + args = (SRC_PATHS, True, True, True, True), is_required = False, print_result = False, ) @@ -154,10 +155,10 @@ def main(): pyflakes_task, pep8_task = None, None
if not stem.prereq.is_python_3() and not args.specific_test: - if test.util.is_pyflakes_available(): + if stem.util.test_tools.is_pyflakes_available(): pyflakes_task = PYFLAKES_TASK
- if test.util.is_pep8_available(): + if stem.util.test_tools.is_pep8_available(): pep8_task = PEP8_TASK
test.util.run_tasks( @@ -296,14 +297,14 @@ def main(): for path, issues in pyflakes_task.result.items(): for issue in issues: static_check_issues.setdefault(path, []).append(issue) - elif not test.util.is_pyflakes_available(): + elif not stem.util.test_tools.is_pyflakes_available(): println("Static error checking requires pyflakes version 0.7.3 or later. Please install it from ...\n http://pypi.python.org/pypi/pyflakes%5Cn", ERROR)
if pep8_task and pep8_task.is_successful: for path, issues in pep8_task.result.items(): for issue in issues: static_check_issues.setdefault(path, []).append(issue) - elif not test.util.is_pep8_available(): + elif not stem.util.test_tools.is_pep8_available(): println("Style checks require pep8 version 1.4.2 or later. Please install it from...\n http://pypi.python.org/pypi/pep8%5Cn", ERROR)
_print_static_issues(static_check_issues) diff --git a/stem/util/__init__.py b/stem/util/__init__.py index 339a62a..5885e47 100644 --- a/stem/util/__init__.py +++ b/stem/util/__init__.py @@ -15,5 +15,6 @@ __all__ = [ "proc", "system", "term", + "test_tools", "tor_tools", ] diff --git a/stem/util/test_tools.py b/stem/util/test_tools.py new file mode 100644 index 0000000..f97189a --- /dev/null +++ b/stem/util/test_tools.py @@ -0,0 +1,288 @@ +# Copyright 2014, Damian Johnson and The Tor Project +# See LICENSE for licensing information + +""" +Helper functions for testing. + +**Stem users are more than welcome to use these for their own test runs, but +these functions are not being vended to our users. They may change in the +future, use them at your own risk.** + +:: + + clean_orphaned_pyc - delete *.pyc files without corresponding *.py + + is_pyflakes_available - checks if pyflakes is available + is_pep8_available - checks if pep8 is available + + get_stylistic_issues - checks for PEP8 and other stylistic issues + get_pyflakes_issues - static checks for problems via pyflakes +""" + +import os +import re + +import stem.util.conf +import stem.util.system + +CONFIG = stem.util.conf.config_dict("test", { + "pep8.ignore": [], + "pyflakes.ignore": [], + "exclude_paths": [], +}) + + +def clean_orphaned_pyc(paths): + """ + Deletes any file with a *.pyc extention without a corresponding *.py. This + helps to address a common gotcha when deleting python files... + + * You delete module 'foo.py' and run the tests to ensure that you haven't + broken anything. They pass, however there *are* still some 'import foo' + statements that still work because the bytecode (foo.pyc) is still around. + + * You push your change. + + * Another developer clones our repository and is confused because we have a + bunch of ImportErrors. + + :param list paths: paths to search for orphaned pyc files + + :returns: list of absolute paths that were deleted + """ + + orphaned_pyc = [] + + for path in paths: + for pyc_path in stem.util.system.files_with_suffix(path, '.pyc'): + # If we're running python 3 then the *.pyc files are no longer bundled + # with the *.py. Rather, they're in a __pycache__ directory. + + # TODO: At the moment there's no point in checking for orphaned bytecode + # with python 3 because it's an exported copy of the python 2 codebase, + # so skipping. However, we might want to address this for other callers. + + if "__pycache__" in pyc_path: + continue + + if not os.path.exists(pyc_path[:-1]): + orphaned_pyc.append(pyc_path) + os.remove(pyc_path) + + return orphaned_pyc + + +def is_pyflakes_available(): + """ + Checks if pyflakes is availalbe. + + :returns: **True** if we can use pyflakes and **False** otherwise + """ + + try: + import pyflakes.api + import pyflakes.reporter + return True + except ImportError: + return False + + +def is_pep8_available(): + """ + Checks if pep8 is availalbe. + + :returns: **True** if we can use pep8 and **False** otherwise + """ + + try: + import pep8 + + if not hasattr(pep8, 'BaseReport'): + raise ImportError() + + return True + except ImportError: + return False + + +def get_stylistic_issues(paths, check_two_space_indents = False, check_newlines = False, check_trailing_whitespace = False, check_exception_keyword = False): + """ + Checks for stylistic issues that are an issue according to the parts of PEP8 + we conform to. You can suppress PEP8 issues by making a 'test' configuration + that sets 'pep8.ignore'. + + For example, with a 'test/settings.cfg' of... + + :: + + # PEP8 compliance issues that we're ignoreing... + # + # * E111 and E121 four space indentations + # * E501 line is over 79 characters + + pep8.ignore E111 + pep8.ignore E121 + pep8.ignore E501 + + ... you can then run tests with... + + :: + + import stem.util.conf + + test_config = stem.util.conf.get_config('test') + test_config.load('test/settings.cfg') + + issues = get_stylistic_issues('my_project') + + If a 'exclude_paths' was set in our test config then we exclude any absolute + paths matching those regexes. + + :param list paths: paths to search for stylistic issues + :param bool check_two_space_indents: check for two space indentations and + that no tabs snuck in + :param bool check_newlines: check that we have standard newlines (\n), not + windows (\r\n) nor classic mac (\r) + :param bool check_trailing_whitespace: check that our lines don't end with + trailing whitespace + :param bool check_exception_keyword: checks that we're using 'as' for + exceptions rather than a comma + + :returns: **dict** of the form ``path => [(line_number, message)...]`` + """ + + issues = {} + + if is_pep8_available(): + import pep8 + + class StyleReport(pep8.BaseReport): + def __init__(self, options): + super(StyleReport, self).__init__(options) + + def error(self, line_number, offset, text, check): + code = super(StyleReport, self).error(line_number, offset, text, check) + + if code: + issues.setdefault(self.filename, []).append((offset + line_number, "%s %s" % (code, text))) + + style_checker = pep8.StyleGuide(ignore = CONFIG['pep8.ignore'], reporter = StyleReport) + style_checker.check_files(list(_python_files(paths))) + + if check_two_space_indents or check_newlines or check_trailing_whitespace or check_exception_keyword: + for path in _python_files(paths): + with open(path) as f: + file_contents = f.read() + + lines, prev_indent = file_contents.split("\n"), 0 + is_block_comment = False + + for index, line in enumerate(lines): + whitespace, content = re.match("^(\s*)(.*)$", line).groups() + + # TODO: This does not check that block indentations are two spaces + # because differentiating source from string blocks ("""foo""") is more + # of a pita than I want to deal with right now. + + if '"""' in content: + is_block_comment = not is_block_comment + + if check_two_space_indents and "\t" in whitespace: + issues.setdefault(path, []).append((index + 1, "indentation has a tab")) + elif check_newlines and "\r" in content: + issues.setdefault(path, []).append((index + 1, "contains a windows newline")) + elif check_trailing_whitespace and content != content.rstrip(): + issues.setdefault(path, []).append((index + 1, "line has trailing whitespace")) + elif check_exception_keyword and 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. + + # TODO: This check only works if the exception variable is called + # 'exc'. We should generalize this via a regex so other names work + # too. + + issues.setdefault(path, []).append((index + 1, "except clause should use 'as', not comma")) + + return issues + + +def get_pyflakes_issues(paths): + """ + Performs static checks via pyflakes. False positives can be ignored via + 'pyflakes.ignore' entries in our 'test' config. For instance... + + :: + + pyflakes.ignore stem/util/test_tools.py => 'pyflakes' imported but unused + pyflakes.ignore stem/util/test_tools.py => 'pep8' imported but unused + + If a 'exclude_paths' was set in our test config then we exclude any absolute + paths matching those regexes. + + :param list paths: paths to search for problems + + :returns: dict of the form ``path => [(line_number, message)...]`` + """ + + issues = {} + + if is_pyflakes_available(): + import pyflakes.api + import pyflakes.reporter + + class Reporter(pyflakes.reporter.Reporter): + def __init__(self): + self._ignored_issues = {} + + for line in CONFIG["pyflakes.ignore"]: + path, issue = line.split("=>") + self._ignored_issues.setdefault(path.strip(), []).append(issue.strip()) + + 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 _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 ignored_path, ignored_issues in self._ignored_issues.items(): + if path.endswith(ignored_path) and issue in ignored_issues: + return True + + return False + + def _register_issue(self, path, line_number, issue): + if not self._is_ignored(path, issue): + issues.setdefault(path, []).append((line_number, issue)) + + reporter = Reporter() + + for path in _python_files(paths): + pyflakes.api.checkPath(path, reporter) + + return issues + + +def _python_files(paths): + for path in paths: + for file_path in stem.util.system.files_with_suffix(path, '.py'): + skip = False + + for exclude_path in CONFIG["exclude_paths"]: + if re.match(exclude_path, file_path): + skip = True + break + + if not skip: + yield file_path diff --git a/test/settings.cfg b/test/settings.cfg index fd960a1..f12311d 100644 --- a/test/settings.cfg +++ b/test/settings.cfg @@ -16,6 +16,7 @@ # 'run_tests.py'. Logging is disabled if set ot an empty value.
integ.test_directory ./test/data +exclude_paths .*/stem/test/data/.* integ.log ./test/data/log
# The following are less testing framework attributes that aren't as commonly @@ -135,10 +136,10 @@ pyflakes.ignore stem/prereq.py => 'RSA' imported but unused pyflakes.ignore stem/prereq.py => 'asn1' imported but unused pyflakes.ignore stem/prereq.py => 'unittest' imported but unused pyflakes.ignore stem/prereq.py => 'long_to_bytes' imported but unused +pyflakes.ignore stem/util/test_tools.py => 'pyflakes' imported but unused +pyflakes.ignore stem/util/test_tools.py => 'pep8' imported but unused 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 -pyflakes.ignore test/util.py => 'pep8' 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 384f9b6..52ea8f8 100644 --- a/test/util.py +++ b/test/util.py @@ -9,15 +9,10 @@ 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 - is_pep8_available - checks if pep8 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 get_python3_destination - location where a python3 copy of stem is exported to - get_stylistic_issues - checks for PEP8 and other stylistic issues - get_pyflakes_issues - static checks for problems via pyflakes
Sets of :class:`~test.util.Task` instances can be ran with :func:`~test.util.run_tasks`. Functions that are intended for easy use with @@ -50,6 +45,7 @@ import stem import stem.prereq import stem.util.conf import stem.util.system +import stem.util.test_tools import stem.version
import test.output @@ -61,8 +57,6 @@ CONFIG = stem.util.conf.config_dict("test", { "target.description": {}, "target.prereq": {}, "target.torrc": {}, - "pep8.ignore": [], - "pyflakes.ignore": [], "integ.test_directory": "./test/data", "test.unit_tests": "", "test.integ_tests": "", @@ -158,39 +152,6 @@ 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.api - import pyflakes.reporter - return True - except ImportError: - return False - - -def is_pep8_available(): - """ - Checks if pep8 is availalbe. - - :returns: **True** if we can use pep8 and **False** otherwise - """ - - try: - import pep8 - - if not hasattr(pep8, 'BaseReport'): - raise ImportError() - - return True - except ImportError: - return False - - def get_prereq(target): """ Provides the tor version required to run the given target. If the target @@ -250,130 +211,6 @@ def get_python3_destination(): return os.path.join(CONFIG["integ.test_directory"], "python3")
-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)...]`` - """ - - issues = {} - - if is_pep8_available(): - import pep8 - - class StyleReport(pep8.BaseReport): - def __init__(self, options): - super(StyleReport, self).__init__(options) - - def error(self, line_number, offset, text, check): - code = super(StyleReport, self).error(line_number, offset, text, check) - - if code: - issues.setdefault(self.filename, []).append((offset + line_number, "%s %s" % (code, text))) - - style_checker = pep8.StyleGuide(ignore = CONFIG["pep8.ignore"], reporter = StyleReport) - style_checker.check_files(list(_python_files(paths))) - - for path in _python_files(paths): - with open(path) as f: - file_contents = f.read() - - lines, prev_indent = file_contents.split("\n"), 0 - is_block_comment = False - - for index, line in enumerate(lines): - whitespace, content = re.match("^(\s*)(.*)$", line).groups() - - # TODO: This does not check that block indentations are two spaces - # because differentiating source from string blocks ("""foo""") is more - # of a pita than I want to deal with right now. - - if '"""' in content: - is_block_comment = not is_block_comment - - if "\t" in whitespace: - issues.setdefault(path, []).append((index + 1, "indentation has a tab")) - elif "\r" in content: - issues.setdefault(path, []).append((index + 1, "contains a windows newline")) - elif content != content.rstrip(): - issues.setdefault(path, []).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. - - issues.setdefault(path, []).append((index + 1, "except clause should use 'as', not comma")) - - return issues - - -def get_pyflakes_issues(paths): - """ - Performs static checks via pyflakes. - - :param list paths: paths to search for problems - - :returns: dict of the form ``path => [(line_number, message)...]`` - """ - - issues = {} - - if is_pyflakes_available(): - import pyflakes.api - import pyflakes.reporter - - class Reporter(pyflakes.reporter.Reporter): - def __init__(self): - self._ignored_issues = {} - - for line in CONFIG["pyflakes.ignore"]: - path, issue = line.split("=>") - self._ignored_issues.setdefault(path.strip(), []).append(issue.strip()) - - 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 _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 ignored_path, ignored_issues in self._ignored_issues.items(): - if path.endswith(ignored_path) and issue in ignored_issues: - return True - - return False - - def _register_issue(self, path, line_number, issue): - if not self._is_ignored(path, issue): - issues.setdefault(path, []).append((line_number, issue)) - - reporter = Reporter() - - for path in _python_files(paths): - pyflakes.api.checkPath(path, reporter) - - return issues - - def check_stem_version(): return stem.__version__
@@ -420,40 +257,12 @@ def check_pep8_version():
def clean_orphaned_pyc(paths): """ - Deletes any file with a *.pyc extention without a corresponding *.py. This - helps to address a common gotcha when deleting python files... - - * You delete module 'foo.py' and run the tests to ensure that you haven't - broken anything. They pass, however there *are* still some 'import foo' - statements that still work because the bytecode (foo.pyc) is still around. - - * You push your change. - - * Another developer clones our repository and is confused because we have a - bunch of ImportErrors. + Deletes any file with a *.pyc extention without a corresponding *.py.
:param list paths: paths to search for orphaned pyc files """
- orphaned_pyc = [] - - for path in paths: - for pyc_path in stem.util.system.files_with_suffix(path, '.pyc'): - # If we're running python 3 then the *.pyc files are no longer bundled - # with the *.py. Rather, they're in a __pycache__ directory. - # - # At the moment there's no point in checking for orphaned bytecode with - # python 3 because it's an exported copy of the python 2 codebase, so - # skipping. - - if "__pycache__" in pyc_path: - continue - - if not os.path.exists(pyc_path[:-1]): - orphaned_pyc.append(pyc_path) - os.remove(pyc_path) - - return ["removed %s" % path for path in orphaned_pyc] + return ["removed %s" % path for path in stem.util.test_tools.clean_orphaned_pyc(paths)]
def check_for_unused_tests(paths): @@ -570,13 +379,6 @@ def run_tasks(category, *tasks): println()
-def _python_files(paths): - for path in paths: - for file_path in stem.util.system.files_with_suffix(path, '.py'): - if not _is_test_data(file_path): - yield file_path - - class Task(object): """ Task we can process while running our tests. The runner can return either a