[tor-commits] [stem/master] Integrating pyflakes with our tests

[atagar at torproject.org]

commit 73e69ac2a612d3eef7b4a6f8efd99063ecb7babb
Author: Damian Johnson <atagar at torproject.org>
Date:   Wed Jan 9 20:15:38 2013 -0800

    Integrating pyflakes with our tests
    
    Inspired by txtorcon, using pyflakes for static error checking of our codebase.
    Unlike pylint it has precious few false positives for our codebase. For the
    false positives we do have I've added a 'pylint.ignore' config mapping so they
    can be whitelisted.
---
 run_tests.py             |   16 ++++++++++++-
 test/check_whitespace.py |   54 +++++++++++++++++++++++++++++++++++++++++++++-
 test/settings.cfg        |    7 ++++++
 3 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/run_tests.py b/run_tests.py
index 35ed65d..9e6f5e1 100755
--- a/run_tests.py
+++ b/run_tests.py
@@ -467,18 +467,30 @@ if __name__ == '__main__':
 
     # TODO: note unused config options afterward?
 
-  base_path = os.path.sep.join(__file__.split(os.path.sep)[:-1])
+  base_path = os.path.sep.join(__file__.split(os.path.sep)[:-1]).lstrip("./")
   style_issues = test.check_whitespace.get_issues(os.path.join(base_path, "stem"))
   style_issues.update(test.check_whitespace.get_issues(os.path.join(base_path, "test")))
   style_issues.update(test.check_whitespace.get_issues(os.path.join(base_path, "run_tests.py")))
 
+  # 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
+  # much overhead in including it with all tests.
+
+  if CONFIG["argument.unit"] or CONFIG["argument.integ"]:
+    if system.is_available("pyflakes"):
+      style_issues.update(test.check_whitespace.pyflakes_issues(os.path.join(base_path, "stem")))
+      style_issues.update(test.check_whitespace.pyflakes_issues(os.path.join(base_path, "test")))
+      style_issues.update(test.check_whitespace.pyflakes_issues(os.path.join(base_path, "run_tests.py")))
+    else:
+      test.output.print_line("Static error checking requires pyflakes. Please install it from ...\n  http://pypi.python.org/pypi/pyflakes\n", *ERROR_ATTR)
+
   if CONFIG["argument.style"]:
     if system.is_available("pep8"):
       style_issues.update(test.check_whitespace.pep8_issues(os.path.join(base_path, "stem")))
       style_issues.update(test.check_whitespace.pep8_issues(os.path.join(base_path, "test")))
       style_issues.update(test.check_whitespace.pep8_issues(os.path.join(base_path, "run_tests.py")))
     else:
-      test.output.print_line("Style checks require pep8. Please install it from 'http://pypi.python.org/pypi/pep8'.")
+      test.output.print_line("Style checks require pep8. Please install it from...\n  http://pypi.python.org/pypi/pep8\n", *ERROR_ATTR)
 
   if style_issues:
     test.output.print_line("STYLE ISSUES", term.Color.BLUE, term.Attr.BOLD)
diff --git a/test/check_whitespace.py b/test/check_whitespace.py
index da81cae..4eca629 100644
--- a/test/check_whitespace.py
+++ b/test/check_whitespace.py
@@ -18,11 +18,18 @@ from __future__ import with_statement
 import re
 import os
 
-from stem.util import system
+from stem.util import conf, system
 
 # if ran directly then run over everything one level up
 DEFAULT_TARGET = os.path.sep.join(__file__.split(os.path.sep)[:-1])
 
+# mapping of files to the issues that should be ignored
+PYFLAKES_IGNORE = None
+
+CONFIG = conf.config_dict("test", {
+  "pyflakes.ignore": []
+})
+
 
 def pep8_issues(base_path = DEFAULT_TARGET):
   """
@@ -85,6 +92,51 @@ def pep8_issues(base_path = DEFAULT_TARGET):
   return issues
 
 
+def pyflakes_issues(base_path = DEFAULT_TARGET):
+  """
+  Checks for issues via pyflakes. False positives can be whitelisted via our
+  test configuration.
+
+  :param str base_path: directory to be iterated over
+
+  :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 = {}
+  pyflakes_output = system.call("pyflakes %s" % base_path)
+
+  for line in pyflakes_output:
+    line_match = re.match("^(.*):(\d+): (.*)$", line)
+
+    if line_match:
+      path, line, issue = line_match.groups()
+
+      if not issue in PYFLAKES_IGNORE.get(path, []):
+        issues.setdefault(path, []).append((int(line), issue))
+
+  return issues
+
+
 def get_issues(base_path = DEFAULT_TARGET):
   """
   Checks python source code in the given directory for whitespace issues.
diff --git a/test/settings.cfg b/test/settings.cfg
index 8c85e47..401b869 100644
--- a/test/settings.cfg
+++ b/test/settings.cfg
@@ -149,3 +149,10 @@ target.torrc RUN_SOCKET   => SOCKET
 target.torrc RUN_SCOOKIE  => SOCKET, COOKIE
 target.torrc RUN_PTRACE   => PORT, PTRACE
 
+# False positives from pyflakes. These are mappings between the path and the
+# issue.
+
+pyflakes.ignore stem/prereq.py => 'RSA' imported but unused
+pyflakes.ignore stem/prereq.py => 'asn1' imported but unused
+pyflakes.ignore stem/prereq.py => 'long_to_bytes' imported but unused
+