Merge lp:~jml/divmod.org/pyflakes-reporter into lp:divmod.org

[Jonathan Lange]

A sketch for a reporter in pyflakes. This moves pyflakes toward having an
interface that can be called from Python to gather errors without requiring
stderr and stdout to be captured. It also separates the format of output
from the means of checking.

I was inspired to do this while trying to add a test to my own projects to
guarantee that it is pyflakes-clean. I didn't want to do stdout/err trapping,
and thought that something like this would be useful.

I'm not really proposing that this branch be merged as-is. Clearly, it lacks
unit tests for the change.

However, I do want to know whether a change along these lines would be considered
for inclusion. If so, I'll spend the energy to toughen it up.

Thanks,
jml

[Glyph Lefkowitz (glyph)]

For what it's worth, seems like a good idea to me.

Tests, please, though.

[Jonathan Jacobs (jjacobs)]

I, too, like this idea. I'd love to see it merged.

[Jonathan Lange (jml)]

On 27 June 2012 19:51, Glyph Lefkowitz <email address hidden> wrote:
> For what it's worth, seems like a good idea to me.
>

Cool, thanks.

> Tests, please, though.

Sure. I'll probably throw this away & start something again TDD. This
seemed the shortest way of describing the change I wished to make.

jml

[lp:~jml/divmod.org/pyflakes-reporter]

2697. By Jonathan Lange on 2012-07-06

Bazaar for the first time ever ate my changes. This commit collapses the history
for:
  - refactoring the tests
    - new assertion method
    - withStderr thingy taking *args rather than lambda
    - wrapper around creating temporary file with content
  - new reporter class with tests
    - actually using it in tree
    - adding a bunch of tests.

2698. By Jonathan Lange on 2012-07-06

Add ioError.

2699. By Jonathan Lange on 2012-07-06

Pass reporter all the way down.

2700. By Jonathan Lange on 2012-07-06

Start to factor the tests so they assert against the data in the reporter
rather than the format of the results.

2701. By Jonathan Lange on 2012-07-06

More tests using reporter.

2702. By Jonathan Lange on 2012-07-06

Handle multiple lines in the reporter.
Restore the multi-line test to chheck output, just in case.

2703. By Jonathan Lange on 2012-07-06

Add flake reporting.

2704. By Jonathan Lange on 2012-07-06

Report flakes from the reporter too.

2705. By Jonathan Lange on 2012-07-08

Whitespace

2706. By Jonathan Lange on 2012-07-08

Initial thing to iterate of source code.

2707. By Jonathan Lange on 2012-07-08

Make iterSourceCode take many paths.

2708. By Jonathan Lange on 2012-07-08

Now we have iterSourceCode instead.

2709. By Jonathan Lange on 2012-07-08

Split out the bit that checks recursively.

2710. By Jonathan Lange on 2012-07-08

A swag of integration tests.

2711. By Jonathan Lange on 2012-07-08

Do it propertly.

2712. By Jonathan Lange on 2012-10-19

0. Move Reporter to new module, pyflakes.reporter.

2713. By Jonathan Lange on 2012-10-19

3. Use FilePath rather than os.path (in tests)

2714. By Jonathan Lange on 2012-10-19

4. Add a bunch of docstrings.

2715. By Jonathan Lange on 2012-10-19

6 (partial). Rename 'popenPyflakes' to 'runPyflakes'.

2716. By Jonathan Lange on 2012-10-19

6 (partial). Contain the poison by returning returncode, out, err from runPyflakes6 (partial). Contain the poison by returning returncode, out, err from runPyflakes6 (partial). Contain the poison by returning returncode, out, err from runPyflakes6 (partial). Contain the poison by returning returncode, out, err from runPyflakes6 (partial). Contain the poison by returning returncode, out, err from runPyflakes6 (partial). Contain the poison by returning returncode, out, err from runPyflakes

2717. By Jonathan Lange on 2012-10-19

6 (partial). Change the integration tests to expect a Deferred from runPyflakes.

2718. By Jonathan Lange on 2012-10-19

6 (partial). Use Twisted to spawn the process, not subprocess.

2719. By Jonathan Lange on 2012-10-19

Change the test assertions to more naturally follow getProcessOutputAndValue's return

2720. By Jonathan Lange on 2012-10-19

Remove unnecessary parens

2721. By Jonathan Lange on 2012-10-19

Explain what's going on.

2722. By Jonathan Lange on 2012-10-23

New method for errors.

2723. By Jonathan Lange on 2012-10-23

Collapse problemDecodingSource and ioError into the one method.

2724. By Jonathan Lange on 2012-10-23

Document that we expect text, not bytes.

2725. By Jonathan Lange on 2012-10-23

Better ptype

2726. By Jonathan Lange on 2012-10-23

Pass unicode

2727. By Jonathan Lange on 2012-10-23

Correct coding convention

2728. By Jonathan Lange on 2012-10-23

Use absolute_import.

2729. By Jonathan Lange on 2012-10-23

Duplicate _EverythingGetter, rather than import it.

2730. By Jonathan Lange on 2012-10-23

Wrap types in C{}

Unmerged revisions

=== modified file 'Pyflakes/bin/pyflakes'
--- Pyflakes/bin/pyflakes	2008-08-28 14:33:07 +0000
+++ Pyflakes/bin/pyflakes	2012-07-08 13:52:19 +0000
@@ -1,4 +1,3 @@
 #!/usr/bin/python
-
 from pyflakes.scripts.pyflakes import main
 main()

=== modified file 'Pyflakes/pyflakes/scripts/pyflakes.py'
--- Pyflakes/pyflakes/scripts/pyflakes.py	2010-04-13 14:53:04 +0000
+++ Pyflakes/pyflakes/scripts/pyflakes.py	2012-07-08 13:52:19 +0000
@@ -9,7 +9,80 @@
 
 checker = __import__('pyflakes.checker').checker
 
-def check(codeString, filename):
+
+class Reporter(object):
+    """
+    Formats the results of pyflakes checks to users.
+    """
+
+    def __init__(self, warningStream, errorStream):
+        """
+        Construct a L{Reporter}.
+
+        @param warningStream: A file-like object where warnings will be
+            written to.  C{sys.stdout} is a good value.
+        @param errorStream: A file-like object where error output will be
+            written to.  C{sys.stderr} is a good value.
+        """
+        self._stdout = warningStream
+        self._stderr = errorStream
+
+
+    def ioError(self, filename, msg):
+        """
+        There was an C{IOError} while reading C{filename}.
+        """
+        self._stderr.write("%s: %s\n" % (filename, msg.args[1]))
+
+
+    def problemDecodingSource(self, filename):
+        """
+        There was a problem decoding the source code in C{filename}.
+        """
+        self._stderr.write(filename)
+        self._stderr.write(': problem decoding source\n')
+
+
+    def syntaxError(self, filename, msg, lineno, offset, text):
+        """
+        There was a syntax errror in C{filename}.
+
+        @param filename: The path to the file with the syntax error.
+        @param msg: An explanation of the syntax error.
+        @param lineno: The line number where the syntax error occurred.
+        @param offset: The column on which the syntax error occurred.
+        @param text: The source code containing the syntax error.
+        """
+        line = text.splitlines()[-1]
+        if offset is not None:
+            offset = offset - (len(text) - len(line))
+        self._stderr.write('%s:%d: %s\n' % (filename, lineno, msg))
+        self._stderr.write(line)
+        self._stderr.write('\n')
+        if offset is not None:
+            self._stderr.write(" " * (offset + 1) + "^\n")
+
+
+    def flake(self, message):
+        """
+        pyflakes found something wrong with the code.
+
+        @param: A L{pyflakes.messages.Message}.
+        """
+        self._stdout.write(str(message))
+        self._stdout.write('\n')
+
+
+
+def _makeDefaultReporter():
+    """
+    Make a reporter that can be used when no reporter is specified.
+    """
+    return Reporter(sys.stdout, sys.stderr)
+
+
+
+def check(codeString, filename, reporter=None):
     """
     Check the Python source given by C{codeString} for flakes.
 
@@ -20,9 +93,14 @@
         errors.
     @type filename: C{str}
 
+    @param reporter: A L{Reporter} instance, where errors and warnings will be
+        reported.
+
     @return: The number of warnings emitted.
     @rtype: C{int}
     """
+    if reporter is None:
+        reporter = _makeDefaultReporter()
     # First, compile into an AST and handle syntax errors.
     try:
         tree = compile(codeString, filename, "exec", _ast.PyCF_ONLY_AST)
@@ -36,55 +114,79 @@
             # Avoid using msg, since for the only known case, it contains a
             # bogus message that claims the encoding the file declared was
             # unknown.
-            print >> sys.stderr, "%s: problem decoding source" % (filename, )
+            reporter.problemDecodingSource(filename)
         else:
-            line = text.splitlines()[-1]
-
-            if offset is not None:
-                offset = offset - (len(text) - len(line))
-
-            print >> sys.stderr, '%s:%d: %s' % (filename, lineno, msg)
-            print >> sys.stderr, line
-
-            if offset is not None:
-                print >> sys.stderr, " " * offset, "^"
-
+            reporter.syntaxError(filename, msg, lineno, offset, text)
         return 1
     else:
         # Okay, it's syntactically valid.  Now check it.
         w = checker.Checker(tree, filename)
         w.messages.sort(lambda a, b: cmp(a.lineno, b.lineno))
         for warning in w.messages:
-            print warning
+            reporter.flake(warning)
         return len(w.messages)
 
 
-def checkPath(filename):
+def checkPath(filename, reporter=None):
     """
     Check the given path, printing out any warnings detected.
 
+    @param reporter: A L{Reporter} instance, where errors and warnings will be
+        reported.
+
     @return: the number of warnings printed
     """
+    if reporter is None:
+        reporter = _makeDefaultReporter()
     try:
-        return check(file(filename, 'U').read() + '\n', filename)
+        return check(file(filename, 'U').read() + '\n', filename, reporter)
     except IOError, msg:
-        print >> sys.stderr, "%s: %s" % (filename, msg.args[1])
+        reporter.ioError(filename, msg)
         return 1
 
 
+
+def iterSourceCode(paths):
+    """
+    Iterate over all Python source files in C{paths}.
+
+    @param paths: A list of paths.  Directories will be recursed into and
+        any .py files found will be yielded.  Any non-directories will be
+        yielded as-is.
+    """
+    for path in paths:
+        if os.path.isdir(path):
+            for dirpath, dirnames, filenames in os.walk(path):
+                for filename in filenames:
+                    if filename.endswith('.py'):
+                        yield os.path.join(dirpath, filename)
+        else:
+            yield path
+
+
+
+def checkRecursive(paths, reporter):
+    """
+    Recursively check all source files in C{paths}.
+
+    @param paths: A list of paths to Python source files and directories
+        containing Python source files.
+    @param reporter: A L{Reporter} where all of the warnings and errors
+        will be reported to.
+    @return: The number of warnings found.
+    """
+    warnings = 0
+    for sourcePath in iterSourceCode(paths):
+        warnings += checkPath(sourcePath, reporter)
+    return warnings
+
+
+
 def main():
-    warnings = 0
     args = sys.argv[1:]
+    reporter = _makeDefaultReporter()
     if args:
-        for arg in args:
-            if os.path.isdir(arg):
-                for dirpath, dirnames, filenames in os.walk(arg):
-                    for filename in filenames:
-                        if filename.endswith('.py'):
-                            warnings += checkPath(os.path.join(dirpath, filename))
-            else:
-                warnings += checkPath(arg)
+        warnings = checkRecursive(args, reporter)
     else:
-        warnings += check(sys.stdin.read(), '<stdin>')
-
+        warnings = check(sys.stdin.read(), '<stdin>', reporter)
     raise SystemExit(warnings > 0)

=== modified file 'Pyflakes/pyflakes/test/test_script.py'
--- Pyflakes/pyflakes/test/test_script.py	2009-06-17 20:58:48 +0000
+++ Pyflakes/pyflakes/test/test_script.py	2012-07-08 13:52:19 +0000
@@ -1,51 +1,293 @@
-
 """
 Tests for L{pyflakes.scripts.pyflakes}.
 """
 
+import os
+import subprocess
 import sys
 from StringIO import StringIO
 
 from twisted.python.filepath import FilePath
 from twisted.trial.unittest import TestCase
 
-from pyflakes.scripts.pyflakes import checkPath
-
-def withStderrTo(stderr, f):
+from pyflakes.messages import UnusedImport
+from pyflakes.scripts.pyflakes import (
+    checkPath,
+    checkRecursive,
+    iterSourceCode,
+    Reporter,
+    )
+
+
+def withStderrTo(stderr, f, *args, **kwargs):
     """
     Call C{f} with C{sys.stderr} redirected to C{stderr}.
     """
     (outer, sys.stderr) = (sys.stderr, stderr)
     try:
-        return f()
+        return f(*args, **kwargs)
     finally:
         sys.stderr = outer
 
 
 
+class LoggingReporter(object):
+
+    def __init__(self, log):
+        self.log = log
+
+
+    def flake(self, message):
+        self.log.append(('flake', str(message)))
+
+
+    def ioError(self, filename, exception):
+        self.log.append(('ioError', filename, exception.args[1]))
+
+
+    def problemDecodingSource(self, filename):
+        self.log.append(('problemDecodingSource', filename))
+
+
+    def syntaxError(self, filename, msg, lineno, offset, line):
+        self.log.append(('syntaxError', filename, msg, lineno, offset, line))
+
+
+
+class TestIterSourceCode(TestCase):
+    """
+    Tests for L{iterSourceCode}.
+    """
+
+    def test_emptyDirectory(self):
+        """
+        There are no Python files in an empty directory.
+        """
+        tempdir = FilePath(self.mktemp())
+        tempdir.createDirectory()
+        self.assertEqual(list(iterSourceCode([tempdir.path])), [])
+
+
+    def test_singleFile(self):
+        """
+        If the directory contains one Python file, C{iterSourceCode} will find
+        it.
+        """
+        tempdir = FilePath(self.mktemp())
+        tempdir.createDirectory()
+        tempdir.child('foo.py').touch()
+        self.assertEqual(
+            list(iterSourceCode([tempdir.path])),
+            [os.path.join(tempdir.path, 'foo.py')])
+
+
+    def test_onlyPythonSource(self):
+        """
+        Files that are not Python source files are not included.
+        """
+        tempdir = FilePath(self.mktemp())
+        tempdir.createDirectory()
+        tempdir.child('foo.pyc').touch()
+        self.assertEqual(list(iterSourceCode([tempdir.path])), [])
+
+
+    def test_recurses(self):
+        """
+        If the Python files are hidden deep down in child directories, we will
+        find them.
+        """
+        tempdir = FilePath(self.mktemp())
+        tempdir.createDirectory()
+        tempdir.child('foo').createDirectory()
+        tempdir.child('foo').child('a.py').touch()
+        tempdir.child('bar').createDirectory()
+        tempdir.child('bar').child('b.py').touch()
+        tempdir.child('c.py').touch()
+        self.assertEqual(
+            sorted(iterSourceCode([tempdir.path])),
+            sorted([os.path.join(tempdir.path, 'foo/a.py'),
+                    os.path.join(tempdir.path, 'bar/b.py'),
+                    os.path.join(tempdir.path, 'c.py')]))
+
+
+    def test_multipleDirectories(self):
+        """
+        L{iterSourceCode} can be given multiple directories.  It will recurse
+        into each of them.
+        """
+        tempdir = FilePath(self.mktemp())
+        tempdir.createDirectory()
+        foo = tempdir.child('foo')
+        foo.createDirectory()
+        foo.child('a.py').touch()
+        bar = tempdir.child('bar')
+        bar.createDirectory()
+        bar.child('b.py').touch()
+        self.assertEqual(
+            sorted(iterSourceCode([foo.path, bar.path])),
+            sorted([os.path.join(foo.path, 'a.py'),
+                    os.path.join(bar.path, 'b.py')]))
+
+
+    def test_explicitFiles(self):
+        """
+        If one of the paths given to L{iterSourceCode} is not a directory but
+        a file, it will include that in its output.
+        """
+        tempfile = FilePath(self.mktemp())
+        tempfile.touch()
+        self.assertEqual(list(iterSourceCode([tempfile.path])),
+                         [tempfile.path])
+
+
+
+class TestReporter(TestCase):
+    """
+    Tests for L{Reporter}.
+    """
+
+    def test_problemDecodingSource(self):
+        """
+        C{problemDecodingSource} reports that there was a problem decoding the
+        source to the error stream.  It includes the filename that it couldn't
+        decode.
+        """
+        err = StringIO()
+        reporter = Reporter(None, err)
+        reporter.problemDecodingSource('foo.py')
+        self.assertEquals("foo.py: problem decoding source\n", err.getvalue())
+
+
+    def test_syntaxError(self):
+        """
+        C{syntaxError} reports that there was a syntax error in the source
+        file.  It reports to the error stream and includes the filename, line
+        number, error message, actual line of source and a caret pointing to
+        where the error is.
+        """
+        err = StringIO()
+        reporter = Reporter(None, err)
+        reporter.syntaxError('foo.py', 'a problem', 3, 4, 'bad line of source')
+        self.assertEquals(
+            ("foo.py:3: a problem\n"
+             "bad line of source\n"
+             "     ^\n"),
+            err.getvalue())
+
+
+    def test_syntaxErrorNoOffset(self):
+        """
+        C{syntaxError} doesn't include a caret pointing to the error if
+        C{offset} is passed as C{None}.
+        """
+        err = StringIO()
+        reporter = Reporter(None, err)
+        reporter.syntaxError('foo.py', 'a problem', 3, None,
+                             'bad line of source')
+        self.assertEquals(
+            ("foo.py:3: a problem\n"
+             "bad line of source\n"),
+            err.getvalue())
+
+
+    def test_multiLineSyntaxError(self):
+        """
+        If there's a multi-line syntax error, then we only report the last
+        line.  The offset is adjusted so that it is relative to the start of
+        the last line.
+        """
+        err = StringIO()
+        lines = [
+            'bad line of source',
+            'more bad lines of source',
+            ]
+        reporter = Reporter(None, err)
+        reporter.syntaxError('foo.py', 'a problem', 3, len(lines[0]) + 5,
+                             '\n'.join(lines))
+        self.assertEquals(
+            ("foo.py:3: a problem\n" +
+             lines[-1] + "\n" +
+             "     ^\n"),
+            err.getvalue())
+
+
+    def test_ioError(self):
+        """
+        C{ioError} reports an error reading a source file.  It only includes
+        the human-readable bit of the error message, and excludes the errno.
+        """
+        err = StringIO()
+        reporter = Reporter(None, err)
+        exception = IOError(42, 'bar')
+        try:
+            raise exception
+        except IOError, e:
+            pass
+        reporter.ioError('source.py', e)
+        self.assertEquals('source.py: bar\n', err.getvalue())
+
+
+    def test_flake(self):
+        out = StringIO()
+        reporter = Reporter(out, None)
+        message = UnusedImport('foo.py', 42, 'bar')
+        reporter.flake(message)
+        self.assertEquals(out.getvalue(), "%s\n" % (message,))
+
+
+
 class CheckTests(TestCase):
     """
     Tests for L{check} and L{checkPath} which check a file for flakes.
     """
+
+    def makeTempFile(self, content):
+        """
+        Make a temporary file containing C{content} and return a path to it.
+        """
+        path = FilePath(self.mktemp())
+        path.setContent(content)
+        return path.path
+
+
+    def assertHasErrors(self, path, errorList):
+        """
+        Assert that C{path} causes errors.
+
+        @param path: A path to a file to check.
+        @param errorList: A list of errors expected to be printed to stderr.
+        """
+        err = StringIO()
+        count = withStderrTo(err, checkPath, path)
+        self.assertEquals(
+            (count, err.getvalue()), (len(errorList), ''.join(errorList)))
+
+
+    def getErrors(self, path):
+        log = []
+        reporter = LoggingReporter(log)
+        count = checkPath(path, reporter)
+        return count, log
+
+
     def test_missingTrailingNewline(self):
         """
         Source which doesn't end with a newline shouldn't cause any
         exception to be raised nor an error indicator to be returned by
         L{check}.
         """
-        fName = self.mktemp()
-        FilePath(fName).setContent("def foo():\n\tpass\n\t")
-        self.assertFalse(checkPath(fName))
+        fName = self.makeTempFile("def foo():\n\tpass\n\t")
+        self.assertHasErrors(fName, [])
 
 
     def test_checkPathNonExisting(self):
         """
         L{checkPath} handles non-existing files.
         """
-        err = StringIO()
-        count = withStderrTo(err, lambda: checkPath('extremo'))
-        self.assertEquals(err.getvalue(), 'extremo: No such file or directory\n')
+        count, errors = self.getErrors('extremo')
         self.assertEquals(count, 1)
+        self.assertEquals(
+            errors, [('ioError', 'extremo', 'No such file or directory')])
 
 
     def test_multilineSyntaxError(self):
@@ -72,19 +314,14 @@
         exc = self.assertRaises(SyntaxError, evaluate, source)
         self.assertTrue(exc.text.count('\n') > 1)
 
-        sourcePath = FilePath(self.mktemp())
-        sourcePath.setContent(source)
-        err = StringIO()
-        count = withStderrTo(err, lambda: checkPath(sourcePath.path))
-        self.assertEqual(count, 1)
-
-        self.assertEqual(
-            err.getvalue(),
-            """\
+        sourcePath = self.makeTempFile(source)
+        self.assertHasErrors(
+            sourcePath, ["""\
 %s:8: invalid syntax
     '''quux'''
            ^
-""" % (sourcePath.path,))
+"""
+                % (sourcePath,)])
 
 
     def test_eofSyntaxError(self):
@@ -92,19 +329,14 @@
         The error reported for source files which end prematurely causing a
         syntax error reflects the cause for the syntax error.
         """
-        source = "def foo("
-        sourcePath = FilePath(self.mktemp())
-        sourcePath.setContent(source)
-        err = StringIO()
-        count = withStderrTo(err, lambda: checkPath(sourcePath.path))
-        self.assertEqual(count, 1)
-        self.assertEqual(
-            err.getvalue(),
-            """\
+        sourcePath = self.makeTempFile("def foo(")
+        self.assertHasErrors(
+            sourcePath,
+            ["""\
 %s:1: unexpected EOF while parsing
 def foo(
          ^
-""" % (sourcePath.path,))
+""" % (sourcePath,)])
 
 
     def test_nonDefaultFollowsDefaultSyntaxError(self):
@@ -117,17 +349,13 @@
 def foo(bar=baz, bax):
     pass
 """
-        sourcePath = FilePath(self.mktemp())
-        sourcePath.setContent(source)
-        err = StringIO()
-        count = withStderrTo(err, lambda: checkPath(sourcePath.path))
-        self.assertEqual(count, 1)
-        self.assertEqual(
-            err.getvalue(),
-            """\
+        sourcePath = self.makeTempFile(source)
+        self.assertHasErrors(
+            sourcePath,
+            ["""\
 %s:1: non-default argument follows default argument
 def foo(bar=baz, bax):
-""" % (sourcePath.path,))
+""" % (sourcePath,)])
 
 
     def test_nonKeywordAfterKeywordSyntaxError(self):
@@ -139,32 +367,39 @@
         source = """\
 foo(bar=baz, bax)
 """
-        sourcePath = FilePath(self.mktemp())
-        sourcePath.setContent(source)
-        err = StringIO()
-        count = withStderrTo(err, lambda: checkPath(sourcePath.path))
-        self.assertEqual(count, 1)
-        self.assertEqual(
-            err.getvalue(),
-            """\
+        sourcePath = self.makeTempFile(source)
+        self.assertHasErrors(
+            sourcePath,
+            ["""\
 %s:1: non-keyword arg after keyword arg
 foo(bar=baz, bax)
-""" % (sourcePath.path,))
+""" % (sourcePath,)])
 
 
     def test_permissionDenied(self):
         """
-        If the a source file is not readable, this is reported on standard
+        If the source file is not readable, this is reported on standard
         error.
         """
         sourcePath = FilePath(self.mktemp())
         sourcePath.setContent('')
         sourcePath.chmod(0)
-        err = StringIO()
-        count = withStderrTo(err, lambda: checkPath(sourcePath.path))
-        self.assertEquals(count, 1)
-        self.assertEquals(
-            err.getvalue(), "%s: Permission denied\n" % (sourcePath.path,))
+        count, errors = self.getErrors(sourcePath.path)
+        self.assertEquals(count, 1)
+        self.assertEquals(
+            errors, [('ioError', sourcePath.path, "Permission denied")])
+
+
+    def test_pyflakesWarning(self):
+        """
+        If the source file has a pyflakes warning, this is reported as a
+        'flake'.
+        """
+        sourcePath = self.makeTempFile("import foo")
+        count, errors = self.getErrors(sourcePath)
+        self.assertEquals(count, 1)
+        self.assertEquals(
+            errors, [('flake', str(UnusedImport(sourcePath, 1, 'foo')))])
 
 
     def test_misencodedFile(self):
@@ -176,10 +411,109 @@
 # coding: ascii
 x = "\N{SNOWMAN}"
 """.encode('utf-8')
-        sourcePath = FilePath(self.mktemp())
-        sourcePath.setContent(source)
-        err = StringIO()
-        count = withStderrTo(err, lambda: checkPath(sourcePath.path))
-        self.assertEquals(count, 1)
-        self.assertEquals(
-            err.getvalue(), "%s: problem decoding source\n" % (sourcePath.path,))
+        sourcePath = self.makeTempFile(source)
+        self.assertHasErrors(
+            sourcePath, ["%s: problem decoding source\n" % (sourcePath,)])
+
+
+    def test_checkRecursive(self):
+        """
+        L{checkRecursive} descends into each directory, finding Python files
+        and reporting problems.
+        """
+        tempdir = FilePath(self.mktemp())
+        tempdir.createDirectory()
+        tempdir.child('foo').createDirectory()
+        file1 = tempdir.child('foo').child('bar.py')
+        file1.setContent("import baz\n")
+        file2 = tempdir.child('baz.py')
+        file2.setContent("import contraband")
+        log = []
+        reporter = LoggingReporter(log)
+        warnings = checkRecursive([tempdir.path], reporter)
+        self.assertEqual(warnings, 2)
+        self.assertEqual(
+            sorted(log),
+            sorted([('flake', str(UnusedImport(file1.path, 1, 'baz'))),
+                    ('flake',
+                     str(UnusedImport(file2.path, 1, 'contraband')))]))
+
+
+
+class IntegrationTests(TestCase):
+    """
+    Tests of the pyflakes script that actually spawn the script.
+    """
+
+    def getPyflakesBinary(self):
+        """
+        Return the path to the pyflakes binary.
+        """
+        import pyflakes
+        return os.path.join(
+            os.path.dirname(os.path.dirname(pyflakes.__file__)),
+            'bin', 'pyflakes')
+
+
+    def popenPyflakes(self, *args, **kwargs):
+        """
+        Launch a subprocess running C{pyflakes}.
+        """
+        env = dict(os.environ)
+        env['PYTHONPATH'] = os.pathsep.join(sys.path)
+        p = subprocess.Popen(
+            [sys.executable, self.getPyflakesBinary()] + list(args),
+            stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+            env=env, **kwargs)
+        return p
+
+
+    def test_goodFile(self):
+        """
+        When a Python source file is all good, the return code is zero and no
+        messages are printed to either stdout or stderr.
+        """
+        tempfile = FilePath(self.mktemp())
+        tempfile.touch()
+        p = self.popenPyflakes(tempfile.path)
+        out, err = p.communicate()
+        self.assertEqual((0, '', ''), (p.returncode, out, err))
+
+
+    def test_fileWithFlakes(self):
+        """
+        When a Python source file has warnings, the return code is non-zero
+        and the warnings are printed to stdout.
+        """
+        tempfile = FilePath(self.mktemp())
+        tempfile.setContent("import contraband\n")
+        p = self.popenPyflakes(tempfile.path)
+        out, err = p.communicate()
+        self.assertEqual(
+            (1, "%s\n" % UnusedImport(tempfile.path, 1, 'contraband'), ''),
+            (p.returncode, out, err))
+
+
+    def test_errors(self):
+        """
+        When pyflakes finds errors with the files it's given, (if they don't
+        exist, say), then the return code is non-zero and the errors are
+        printed to stderr.
+        """
+        tempfile = FilePath(self.mktemp())
+        p = self.popenPyflakes(tempfile.path)
+        out, err = p.communicate()
+        self.assertEqual(
+            (1, '', '%s: No such file or directory\n' % (tempfile.path,)),
+            (p.returncode, out, err))
+
+
+    def test_readFromStdin(self):
+        """
+        If no arguments are passed to C{pyflakes} then it reads from stdin.
+        """
+        p = self.popenPyflakes(stdin=subprocess.PIPE)
+        out, err = p.communicate('import contraband')
+        self.assertEqual(
+            (1, "%s\n" % UnusedImport('<stdin>', 1, 'contraband'), ''),
+            (p.returncode, out, err))