commit 3d7bcee1aeba0f2ec2b8d808a1b57ba45ef731ae Author: Damian Johnson atagar@torproject.org Date: Fri Apr 12 23:14:06 2013 -0700

Merging static_checks.py with the test utils

Revising the static check functions and merging them with the test util module. --- run_tests.py | 27 ++++-- test/__init__.py | 2 - test/settings.cfg | 34 ++++++++ test/static_checks.py | 225 ------------------------------------------------- test/util.py | 160 ++++++++++++++++++++++++++++++++++- 5 files changed, 207 insertions(+), 241 deletions(-)

diff --git a/run_tests.py b/run_tests.py index cbb1941..5e80441 100755 --- a/run_tests.py +++ b/run_tests.py @@ -23,7 +23,6 @@ from stem.util import log, system, term

import test.output import test.runner -import test.static_checks import test.util

from test.runner import Target @@ -47,6 +46,14 @@ SOURCE_BASE_PATHS = [os.path.join(base, path) for path in ('stem', 'test', 'run_

def _python3_setup(python3_destination, clean): + """ + Exports the python3 counterpart of our codebase using 2to3. + + :param str python3_destination: location to export our codebase to + :param bool clean: deletes our priorly exported codebase if **True**, + otherwise this is a no-op + """ + # Python 2.7.3 added some nice capabilities to 2to3, like '--output-dir'... # # http://docs.python.org/2/library/2to3.html @@ -94,8 +101,8 @@ def _python3_setup(python3_destination, clean): return True

-def _print_style_issues(run_unit, run_integ, run_style): - style_issues = test.static_checks.get_issues(SOURCE_BASE_PATHS) +def _print_static_issues(run_unit, run_integ, run_style): + static_check_issues = {}

# If we're doing some sort of testing (unit or integ) and pyflakes is # available then use it. Its static checks are pretty quick so there's not @@ -103,23 +110,23 @@ def _print_style_issues(run_unit, run_integ, run_style):

if run_unit or run_integ: if system.is_available("pyflakes"): - style_issues.update(test.static_checks.pyflakes_issues(SOURCE_BASE_PATHS)) + static_check_issues.update(test.util.get_pyflakes_issues(SOURCE_BASE_PATHS)) else: test.output.print_error("Static error checking requires pyflakes. Please install it from ...\n http://pypi.python.org/pypi/pyflakes%5Cn")

if run_style: if system.is_available("pep8"): - style_issues.update(test.static_checks.pep8_issues(SOURCE_BASE_PATHS)) + static_check_issues = test.util.get_stylistic_issues(SOURCE_BASE_PATHS) else: test.output.print_error("Style checks require pep8. Please install it from...\n http://pypi.python.org/pypi/pep8%5Cn")

- if style_issues: - test.output.print_line("STYLE ISSUES", term.Color.BLUE, term.Attr.BOLD) + if static_check_issues: + test.output.print_line("STATIC CHECKS", term.Color.BLUE, term.Attr.BOLD)

- for file_path in style_issues: + for file_path in static_check_issues: test.output.print_line("* %s" % file_path, term.Color.BLUE, term.Attr.BOLD)

- for line_number, msg in style_issues[file_path]: + for line_number, msg in static_check_issues[file_path]: line_count = "%-4s" % line_number test.output.print_line(" line %s - %s" % (line_count, msg))

@@ -411,7 +418,7 @@ if __name__ == '__main__': # TODO: note unused config options afterward?

if not stem.prereq.is_python_3(): - _print_style_issues(run_unit, run_integ, run_style) + _print_static_issues(run_unit, run_integ, run_style)

runtime = time.time() - start_time

diff --git a/test/__init__.py b/test/__init__.py index bc7c694..ff55c3a 100644 --- a/test/__init__.py +++ b/test/__init__.py @@ -12,7 +12,5 @@ __all__ = [ "output", "prompt", "runner", - "static_checks", - "tutorial", "utils", ] diff --git a/test/settings.cfg b/test/settings.cfg index 625a482..9d28f4d 100644 --- a/test/settings.cfg +++ b/test/settings.cfg @@ -92,6 +92,40 @@ target.torrc RUN_SOCKET => SOCKET target.torrc RUN_SCOOKIE => SOCKET, COOKIE target.torrc RUN_PTRACE => PORT, PTRACE

+# PEP8 compliance issues that we're ignoreing... +# +# * E251 no spaces around keyword / parameter equals +# +# This one I dislike a great deal. It makes keyword arguments different +# from assignments which looks... aweful. I'm not sure what PEP8's author +# was on when he wrote this one but it's stupid. +# +# Someone else can change this if they really care. +# +# * E501 line is over 79 characters +# +# We're no longer on TTY terminals. Overly constraining line length makes +# things far less readable, encouraging bad practices like abbreviated +# variable names. +# +# If the code fits on my tiny netbook screen then it's narrow enough. +# +# * E111 and E121 four space indentations +# +# Ahhh, indentation. The holy war that'll never die. Sticking with two +# space indentations since it leads to shorter lines. +# +# * E127 continuation line over-indented for visual indent +# +# Pep8 only works with this one if we have four space indents (its +# detection is based on multiples of four). + +pep8.ignore E111 +pep8.ignore E121 +pep8.ignore E501 +pep8.ignore E251 +pep8.ignore E127 + # False positives from pyflakes. These are mappings between the path and the # issue.

diff --git a/test/static_checks.py b/test/static_checks.py deleted file mode 100644 index 307a858..0000000 --- a/test/static_checks.py +++ /dev/null @@ -1,225 +0,0 @@ -# Copyright 2012-2013, Damian Johnson -# See LICENSE for licensing information - -""" -Performs a check that our python source code follows its whitespace conventions -which are... - -* 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) -""" - -import re -import os - -from stem.util import conf, system - -# mapping of files to the issues that should be ignored -PYFLAKES_IGNORE = None - -CONFIG = conf.config_dict("test", { - "pyflakes.ignore": [], - "integ.test_directory": "./test/data", -}) - - -def pep8_issues(base_paths): - """ - Checks for stylistic issues that are an issue according to the parts of PEP8 - we conform to. - - :param str,list base_paths: directory to be iterated over - - :returns: dict of the form ``path => [(line_number, message)...]`` - """ - - if isinstance(base_paths, (tuple, list)): - results = {} - - for path in base_paths: - results.update(pep8_issues(path)) - - return results - - # The pep8 command give output of the form... - # - # FILE:LINE:CHARACTER ISSUE - # - # ... for instance... - # - # ./test/mocking.py:868:31: E225 missing whitespace around operator - # - # Ignoring the following compliance issues. - # - # * E251 no spaces around keyword / parameter equals - # - # This one I dislike a great deal. It makes keyword arguments different - # from assignments which looks... aweful. I'm not sure what PEP8's author - # was on when he wrote this one but it's stupid. - # - # Someone else can change this if they really care. - # - # * E501 line is over 79 characters - # - # We're no longer on TTY terminals. Overly constraining line length makes - # things far less readable, encouraging bad practices like abbreviated - # variable names. - # - # If the code fits on my tiny netbook screen then it's narrow enough. - # - # * E111 and E121 four space indentations - # - # Ahhh, indentation. The holy war that'll never die. Sticking with two - # space indentations since it leads to shorter lines. - # - # * E127 continuation line over-indented for visual indent - # - # Pep8 only works with this one if we have four space indents (its - # detection is based on multiples of four). - - ignored_issues = "E111,E121,E501,E251,E127" - - issues = {} - pep8_output = system.call("pep8 --ignore %s %s" % (ignored_issues, base_paths)) - - for line in pep8_output: - line_match = re.match("^(.*):(\d+):(\d+): (.*)$", line) - - if line_match: - path, line, _, issue = line_match.groups() - - if not _is_test_data(path): - issues.setdefault(path, []).append((int(line), issue)) - - return issues - - -def pyflakes_issues(base_paths): - """ - Checks for issues via pyflakes. False positives can be whitelisted via our - test configuration. - - :param str,list base_paths: directory to be iterated over - - :returns: dict of the form ``path => [(line_number, message)...]`` - """ - - if isinstance(base_paths, (tuple, list)): - results = {} - - for path in base_paths: - results.update(pyflakes_issues(path)) - - return results - - global PYFLAKES_IGNORE - - if PYFLAKES_IGNORE is None: - pyflakes_ignore = {} - - for line in CONFIG["pyflakes.ignore"]: - path, issue = line.split("=>") - pyflakes_ignore.setdefault(path.strip(), []).append(issue.strip()) - - PYFLAKES_IGNORE = pyflakes_ignore - - # 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 = {} - pyflakes_output = system.call("pyflakes %s" % base_paths) - - 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 issue in PYFLAKES_IGNORE.get(path, []): - issues.setdefault(path, []).append((int(line), issue)) - - return issues - - -def get_issues(base_paths): - """ - Checks python source code in the given directory for whitespace issues. - - :param str,list base_paths: directory to be iterated over - - :returns: dict of the form ``path => [(line_number, message)...]`` - """ - - if isinstance(base_paths, (tuple, list)): - results = {} - - for path in base_paths: - results.update(get_issues(path)) - - return results - - # 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. - - issues = {} - - for file_path in _get_files_with_suffix(base_paths): - if _is_test_data(file_path): - 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")) - - if file_issues: - issues[file_path] = file_issues - - return issues - - -def _is_test_data(path): - return os.path.normpath(path).startswith(os.path.normpath(CONFIG["integ.test_directory"])) - - -def _get_files_with_suffix(base_path, suffix = ".py"): - """ - Iterates over files in a given directory, providing filenames with a certain - suffix. - - :param str base_path: directory to be iterated over - :param str suffix: filename suffix to look for - - :returns: iterator that yields the absolute path for files with the given suffix - """ - - 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) diff --git a/test/util.py b/test/util.py index bd5dd37..9c0e23e 100644 --- a/test/util.py +++ b/test/util.py @@ -1,3 +1,6 @@ +# Copyright 2012-2013, Damian Johnson +# See LICENSE for licensing information + """ Helper functions for our test framework.

@@ -7,19 +10,27 @@ Helper functions for our test framework. get_integ_tests - provides our integration tests

clean_orphaned_pyc - removes any *.pyc without a corresponding *.py + get_stylistic_issues - checks for PEP8 and other stylistic issues + get_pyflakes_issues - static checks for problems via pyflakes """

+import re import os

import stem.util.conf - -import test.static_checks +import stem.util.system

CONFIG = stem.util.conf.config_dict("test", { + "pep8.ignore": [], + "pyflakes.ignore": [], + "integ.test_directory": "./test/data", "test.unit_tests": "", "test.integ_tests": "", })

+# mapping of files to the issues that should be ignored +PYFLAKES_IGNORE = None +

def get_unit_tests(prefix = None): """ @@ -89,8 +100,8 @@ def clean_orphaned_pyc(paths):

orphaned_pyc = []

- for base_dir in paths: - for pyc_path in test.static_checks._get_files_with_suffix(base_dir, ".pyc"): + for path in paths: + for pyc_path in _get_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. # @@ -106,3 +117,144 @@ def clean_orphaned_pyc(paths): os.remove(pyc_path)

return orphaned_pyc + + +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) + + :param list paths: paths to search for stylistic issues + + :returns: dict of the form ``path => [(line_number, message)...]`` + """ + + # The pep8 command give output of the form... + # + # FILE:LINE:CHARACTER ISSUE + # + # ... for instance... + # + # ./test/mocking.py:868:31: E225 missing whitespace around operator + + ignored_issues = ','.join(CONFIG["pep8.ignore"]) + issues = {} + + for path in paths: + pep8_output = stem.util.system.call("pep8 --ignore %s %s" % (ignored_issues, path)) + + for line in pep8_output: + line_match = re.match("^(.*):(\d+):(\d+): (.*)$", line) + + if line_match: + path, line, _, issue = line_match.groups() + + if not _is_test_data(path): + issues.setdefault(path, []).append((int(line), issue)) + + for file_path in _get_files_with_suffix(path): + if _is_test_data(file_path): + 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() + + # 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: + 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")) + + if file_issues: + issues[file_path] = file_issues + + 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)...]`` + """ + + global PYFLAKES_IGNORE + + if PYFLAKES_IGNORE is None: + pyflakes_ignore = {} + + for line in CONFIG["pyflakes.ignore"]: + path, issue = line.split("=>") + pyflakes_ignore.setdefault(path.strip(), []).append(issue.strip()) + + PYFLAKES_IGNORE = pyflakes_ignore + + # 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 = {} + + for path in paths: + pyflakes_output = stem.util.system.call("pyflakes %s" % path) + + 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 issue in PYFLAKES_IGNORE.get(path, []): + issues.setdefault(path, []).append((int(line), issue)) + + return issues + + +def _is_test_data(path): + return os.path.normpath(path).startswith(os.path.normpath(CONFIG["integ.test_directory"])) + + +def _get_files_with_suffix(base_path, suffix = ".py"): + """ + Iterates over files in a given directory, providing filenames with a certain + suffix. + + :param str base_path: directory to be iterated over + :param str suffix: filename suffix to look for + + :returns: iterator that yields the absolute path for files with the given suffix + """ + + 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)