From c81a403fefcde5545bd7d41c66efb7b658d166f5 Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Sat, 12 Nov 2016 13:42:43 -0600
Subject: [PATCH] Exit non-zero if something goes wrong during a run

If we handle an exception, or early exit, or really anything, we should
exit non-zero (and we used to). This was a minor oversight.

Closes #209
Closes #248
---
 docs/source/release-notes/3.1.0.rst | 11 ++++-
 src/flake8/main/application.py      |  8 +++-
 tests/unit/test_application.py      | 62 +++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 tests/unit/test_application.py

diff --git a/docs/source/release-notes/3.1.0.rst b/docs/source/release-notes/3.1.0.rst
index 2db6ccf..6046239 100644
--- a/docs/source/release-notes/3.1.0.rst
+++ b/docs/source/release-notes/3.1.0.rst
@@ -7,7 +7,7 @@
   hook. (See also `GitLab#210`_, `GitLab#223`_)
 
 - Avoid unhandled exceptions when dealing with SyntaxErrors. (See also
-  `GitLab#214`_)
+  `GitLab#214`_, `GitLab#238`_)
 
 - Exit early if the value for ``--diff`` is empty. (See also `GitLab#226`_)
 
@@ -35,6 +35,9 @@
 - Add new File Processor attribute, ``previous_unindented_logical_line`` to
   accomodate pycodestyle 2.1.0. (See also `GitLab#246`_)
 
+- When something goes wrong, exit non-zero. (See also `GitLab#248`_,
+  `GitLab#209`_)
+
 - Add ``--tee`` as an option to allow use of ``--output-file`` and printing to
   standard out.
 
@@ -43,6 +46,8 @@
 - Allow for pycodestyle 2.1 series and pyflakes 1.3 series.
 
 .. links
+.. _GitLab#209:
+    https://gitlab.com/pycqa/flake8/issues/209
 .. _GitLab#210:
     https://gitlab.com/pycqa/flake8/issues/210
 .. _GitLab#214:
@@ -55,6 +60,8 @@
     https://gitlab.com/pycqa/flake8/issues/235
 .. _GitLab#237:
     https://gitlab.com/pycqa/flake8/issues/237
+.. _GitLab#238:
+    https://gitlab.com/pycqa/flake8/issues/238
 .. _GitLab#239:
     https://gitlab.com/pycqa/flake8/issues/239
 .. _GitLab#242:
@@ -63,3 +70,5 @@
     https://gitlab.com/pycqa/flake8/issues/245
 .. _GitLab#246:
     https://gitlab.com/pycqa/flake8/issues/246
+.. _GitLab#248:
+    https://gitlab.com/pycqa/flake8/issues/248
diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py
index 88db0a8..4765ee1 100644
--- a/src/flake8/main/application.py
+++ b/src/flake8/main/application.py
@@ -102,6 +102,9 @@ class Application(object):
         #: The total number of errors before accounting for ignored errors and
         #: lines.
         self.total_result_count = 0
+        #: Whether or not something catastrophic happened and we should exit
+        #: with a non-zero status code
+        self.catastrophic_failure = False
 
         #: Whether the program is processing a diff or not
         self.running_against_diff = False
@@ -119,7 +122,8 @@ class Application(object):
             print(self.result_count)
 
         if not self.options.exit_zero:
-            raise SystemExit(self.result_count > 0)
+            raise SystemExit((self.result_count > 0) or
+                             self.catastrophic_failure)
 
     def find_plugins(self):
         # type: () -> NoneType
@@ -321,5 +325,7 @@ class Application(object):
             LOG.critical('Caught keyboard interrupt from user')
             LOG.exception(exc)
             self.file_checker_manager._force_cleanup()
+            self.catastrophic_failure = True
         except exceptions.EarlyQuit:
+            self.catastrophic_failure = True
             print('... stopped while processing files')
diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py
new file mode 100644
index 0000000..6a3ef91
--- /dev/null
+++ b/tests/unit/test_application.py
@@ -0,0 +1,62 @@
+"""Tests for the Application class."""
+import optparse
+
+import mock
+import pytest
+
+from flake8.main import application as app
+
+
+def options(**kwargs):
+    """Generate optparse.Values for our Application."""
+    kwargs.setdefault('verbose', 0)
+    kwargs.setdefault('output_file', None)
+    kwargs.setdefault('count', False)
+    kwargs.setdefault('exit_zero', False)
+    return optparse.Values(kwargs)
+
+
+@pytest.mark.parametrize(
+    'result_count, catastrophic, exit_zero', [
+        (0, True, True),
+        (2, False, True),
+        (2, True, True),
+    ]
+)
+def test_exit_does_not_raise(result_count, catastrophic, exit_zero):
+    """Verify Application.exit doesn't raise SystemExit."""
+    with mock.patch('flake8.options.manager.OptionManager') as optionmanager:
+        optmgr = optionmanager.return_value = mock.Mock()
+        optmgr.parse_known_args.return_value = (options(), [])
+        application = app.Application()
+
+    application.result_count = result_count
+    application.catastrophic_failure = catastrophic
+    application.options = options(exit_zero=exit_zero)
+
+    assert application.exit() is None
+
+
+@pytest.mark.parametrize(
+    'result_count, catastrophic, exit_zero, value', [
+        (0, False, False, False),
+        (0, True, False, True),
+        (2, False, False, True),
+        (2, True, False, True),
+    ]
+)
+def test_exit_does_raise(result_count, catastrophic, exit_zero, value):
+    """Verify Application.exit doesn't raise SystemExit."""
+    with mock.patch('flake8.options.manager.OptionManager') as optionmanager:
+        optmgr = optionmanager.return_value = mock.Mock()
+        optmgr.parse_known_args.return_value = (options(), [])
+        application = app.Application()
+
+    application.result_count = result_count
+    application.catastrophic_failure = catastrophic
+    application.options = options(exit_zero=exit_zero)
+
+    with pytest.raises(SystemExit) as excinfo:
+        application.exit()
+
+    assert excinfo.value.args[0] is value