[tor-commits] [stem/master] Use pyflakes' api rather than shelling out

[atagar at torproject.org]

commit 984b77b4f19592f3bc155f1fba895111ed9cba99
Author: Damian Johnson <atagar at 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