Merge branch 'bug/74' into 'master'

Refactor how we use StyleGuides for better error recovery

In bug 74 we discovered that there are some less than ideal problems
around our use of multiprocessing. This is a first attempt at fixing
74 by using a fake StyleGuide object which proxies to the real one,
and will catch and handle exceptions and then posibly retry the
operation we were trying to perform in the first place.

Currently we're only implementing that logic for StyleGuide.check_files
but we should be careful to implement this in other functions used in
hooks and elsewhere.

Note: there may be a simpler way to fix this with a context manager
that will do the right thing. That may also prove simpler to implement
but that will have a much larger impact on the code-base than this.

Related to bug #74

See merge request !36

flake8/engine.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
-import re
+import errno
 import platform
+import re
 import warnings
 
 import pep8
@@ -81,7 +82,7 @@ def get_parser():
     return parser, options_hooks
 
 
-class StyleGuide(pep8.StyleGuide):
+class NoQAStyleGuide(pep8.StyleGuide):
 
     def input_file(self, filename, lines=None, expected=None, line_offset=0):
         """Run all checks on a Python source file."""
@@ -95,6 +96,76 @@ class StyleGuide(pep8.StyleGuide):
         return fchecker.check_all(expected=expected, line_offset=line_offset)
 
 
+class StyleGuide(object):
+    """A wrapper StyleGuide object for Flake8 usage.
+
+    This allows for OSErrors to be caught in the styleguide and special logic
+    to be used to handle those errors.
+    """
+
+    # Reasoning for error numbers is in-line below
+    serial_retry_errors = set([
+        # ENOSPC: Added by sigmavirus24
+        # > On some operating systems (OSX), multiprocessing may cause an
+        # > ENOSPC error while trying to trying to create a Semaphore.
+        # > In those cases, we should replace the customized Queue Report
+        # > class with pep8's StandardReport class to ensure users don't run
+        # > into this problem.
+        # > (See also: https://gitlab.com/pycqa/flake8/issues/74)
+        errno.ENOSPC,
+        # NOTE(sigmavirus24): When adding to this list, include the reasoning
+        # on the lines before the error code and always append your error
+        # code. Further, please always add a trailing `,` to reduce the visual
+        # noise in diffs.
+    ])
+
+    def __init__(self, **kwargs):
+        # This allows us to inject a mocked StyleGuide in the tests.
+        self._styleguide = kwargs.pop('styleguide', NoQAStyleGuide(**kwargs))
+
+    @property
+    def options(self):
+        return self._styleguide.options
+
+    @property
+    def paths(self):
+        return self._styleguide.paths
+
+    def _retry_serial(self, func, *args, **kwargs):
+        """This will retry the passed function in serial if necessary.
+
+        In the event that we encounter an OSError with an errno in
+        :attr:`serial_retry_errors`, this function will retry this function
+        using pep8's default Report class which operates in serial.
+        """
+        try:
+            return func(*args, **kwargs)
+        except OSError as oserr:
+            if oserr.errno in self.serial_retry_errors:
+                self.init_report(pep8.StandardReport)
+            else:
+                raise
+            return func(*args, **kwargs)
+
+    def check_files(self, paths=None):
+        return self._retry_serial(self._styleguide.check_files, paths=paths)
+
+    def excluded(self, filename, parent=None):
+        return self._styleguide.excluded(filename, parent=parent)
+
+    def init_report(self, reporter=None):
+        return self._styleguide.init_report(reporter)
+
+    def input_file(self, filename, lines=None, expected=None, line_offset=0):
+        return self._retry_serial(
+            self._styleguide.input_file,
+            filename=filename,
+            lines=lines,
+            expected=expected,
+            line_offset=line_offset,
+        )
+
+
 def _disable_extensions(parser, options):
     ignored_extensions = set(getattr(parser, 'ignored_extensions', []))
     # Remove any of the selected extensions from the extensions ignored by

flake8/tests/test_engine.py

@@ -1,5 +1,6 @@
 from __future__ import with_statement
 
+import errno
 import unittest
 try:
     from unittest import mock
@@ -7,6 +8,7 @@ except ImportError:
     import mock  # < PY33
 
 from flake8 import engine, util, __version__, reporter
+import pep8
 
 
 class TestEngine(unittest.TestCase):
@@ -112,5 +114,87 @@ class TestEngine(unittest.TestCase):
             assert 'X' not in sg.options.ignore
 
 
+def oserror_generator(error_number, message='Ominous OSError message'):
+    def oserror_side_effect(*args, **kwargs):
+        if hasattr(oserror_side_effect, 'used'):
+            return
+
+        oserror_side_effect.used = True
+        raise OSError(error_number, message)
+
+    return oserror_side_effect
+
+
+class TestStyleGuide(unittest.TestCase):
+    def setUp(self):
+        mocked_styleguide = mock.Mock(spec=engine.NoQAStyleGuide)
+        self.styleguide = engine.StyleGuide(styleguide=mocked_styleguide)
+        self.mocked_sg = mocked_styleguide
+
+    def test_proxies_excluded(self):
+        self.styleguide.excluded('file.py', parent='.')
+
+        self.mocked_sg.excluded.assert_called_once_with('file.py', parent='.')
+
+    def test_proxies_init_report(self):
+        reporter = object()
+        self.styleguide.init_report(reporter)
+
+        self.mocked_sg.init_report.assert_called_once_with(reporter)
+
+    def test_proxies_check_files(self):
+        self.styleguide.check_files(['foo', 'bar'])
+
+        self.mocked_sg.check_files.assert_called_once_with(
+            paths=['foo', 'bar']
+        )
+
+    def test_proxies_input_file(self):
+        self.styleguide.input_file('file.py',
+                                   lines=[9, 10],
+                                   expected='foo',
+                                   line_offset=20)
+
+        self.mocked_sg.input_file.assert_called_once_with(filename='file.py',
+                                                          lines=[9, 10],
+                                                          expected='foo',
+                                                          line_offset=20)
+
+    def test_check_files_retries_on_specific_OSErrors(self):
+        self.mocked_sg.check_files.side_effect = oserror_generator(
+            errno.ENOSPC, 'No space left on device'
+        )
+
+        self.styleguide.check_files(['foo', 'bar'])
+
+        self.mocked_sg.init_report.assert_called_once_with(pep8.StandardReport)
+
+    def test_input_file_retries_on_specific_OSErrors(self):
+        self.mocked_sg.input_file.side_effect = oserror_generator(
+            errno.ENOSPC, 'No space left on device'
+        )
+
+        self.styleguide.input_file('file.py')
+
+        self.mocked_sg.init_report.assert_called_once_with(pep8.StandardReport)
+
+    def test_check_files_reraises_unknown_OSErrors(self):
+        self.mocked_sg.check_files.side_effect = oserror_generator(
+            errno.EADDRINUSE,
+            'lol why are we talking about binding to sockets'
+        )
+
+        self.assertRaises(OSError, self.styleguide.check_files,
+                          ['foo', 'bar'])
+
+    def test_input_file_reraises_unknown_OSErrors(self):
+        self.mocked_sg.input_file.side_effect = oserror_generator(
+            errno.EADDRINUSE,
+            'lol why are we talking about binding to sockets'
+        )
+
+        self.assertRaises(OSError, self.styleguide.input_file,
+                          ['foo', 'bar'])
+
 if __name__ == '__main__':
     unittest.main()