Merge pull request #79 from dmtucker/status

Inject a test that checks the mypy exit status

Author: dmtucker

src/pytest_mypy.py

@@ -1,5 +1,6 @@
 """Mypy static type checker plugin for Pytest"""
 
+import functools
 import json
 import os
 from tempfile import NamedTemporaryFile
@@ -73,14 +74,14 @@ def pytest_configure_node(self, node):  # xdist hook
 
 
 def pytest_collect_file(path, parent):
-    """Create a MypyItem for every file mypy should run on."""
+    """Create a MypyFileItem for every file mypy should run on."""
     if path.ext == '.py' and any([
             parent.config.option.mypy,
             parent.config.option.mypy_ignore_missing_imports,
     ]):
-        item = MypyItem(path, parent)
+        item = MypyFileItem(path, parent)
         if nodeid_name:
-            item = MypyItem(
+            item = MypyFileItem(
                 path,
                 parent,
                 nodeid='::'.join([item.nodeid, nodeid_name]),
@@ -89,33 +90,51 @@ def pytest_collect_file(path, parent):
     return None
 
 
-class MypyItem(pytest.Item, pytest.File):
+@pytest.hookimpl(hookwrapper=True, trylast=True)
+def pytest_collection_modifyitems(session, config, items):
+    """
+    Add a MypyStatusItem if any MypyFileItems were collected.
+
+    Since mypy might check files that were not collected,
+    pytest could pass even though mypy failed!
+    To prevent that, add an explicit check for the mypy exit status.
+
+    This should execute as late as possible to avoid missing any
+    MypyFileItems injected by other pytest_collection_modifyitems
+    implementations.
+    """
+    yield
+    if any(isinstance(item, MypyFileItem) for item in items):
+        items.append(MypyStatusItem(nodeid_name, session, config, session))
 
-    """A File that Mypy Runs On."""
+
+class MypyItem(pytest.Item):
+
+    """A Mypy-related test Item."""
 
     MARKER = 'mypy'
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.add_marker(self.MARKER)
 
+    def repr_failure(self, excinfo):
+        """
+        Unwrap mypy errors so we get a clean error message without the
+        full exception repr.
+        """
+        if excinfo.errisinstance(MypyError):
+            return excinfo.value.args[0]
+        return super().repr_failure(excinfo)
+
+
+class MypyFileItem(MypyItem, pytest.File):
+
+    """A File that Mypy Runs On."""
+
     def runtest(self):
         """Raise an exception if mypy found errors for this item."""
-        results = _cached_json_results(
-            results_path=(
-                self.config._mypy_results_path
-                if _is_master(self.config) else
-                self.config.slaveinput['_mypy_results_path']
-            ),
-            results_factory=lambda:
-                _mypy_results_factory(
-                    abspaths=[
-                        os.path.abspath(str(item.fspath))
-                        for item in self.session.items
-                        if isinstance(item, MypyItem)
-                    ],
-                )
-        )
+        results = _mypy_results(self.session)
         abspath = os.path.abspath(str(self.fspath))
         errors = results['abspath_errors'].get(abspath)
         if errors:
@@ -129,14 +148,39 @@ def reportinfo(self):
             self.config.invocation_dir.bestrelpath(self.fspath),
         )
 
-    def repr_failure(self, excinfo):
-        """
-        Unwrap mypy errors so we get a clean error message without the
-        full exception repr.
-        """
-        if excinfo.errisinstance(MypyError):
-            return excinfo.value.args[0]
-        return super().repr_failure(excinfo)
+
+class MypyStatusItem(MypyItem):
+
+    """A check for a non-zero mypy exit status."""
+
+    def runtest(self):
+        """Raise a MypyError if mypy exited with a non-zero status."""
+        results = _mypy_results(self.session)
+        if results['status']:
+            raise MypyError(
+                'mypy exited with status {status}.'.format(
+                    status=results['status'],
+                ),
+            )
+
+
+def _mypy_results(session):
+    """Get the cached mypy results for the session, or generate them."""
+    return _cached_json_results(
+        results_path=(
+            session.config._mypy_results_path
+            if _is_master(session.config) else
+            session.config.slaveinput['_mypy_results_path']
+        ),
+        results_factory=functools.partial(
+            _mypy_results_factory,
+            abspaths=[
+                os.path.abspath(str(item.fspath))
+                for item in session.items
+                if isinstance(item, MypyFileItem)
+            ],
+        )
+    )
 
 
 def _cached_json_results(results_path, results_factory=None):

tests/test_pytest_mypy.py

@@ -14,35 +14,41 @@ def xdist_args(request):
     return ['-n', 'auto'] if request.param else []
 
 
-@pytest.mark.parametrize('test_count', [1, 2])
-def test_mypy_success(testdir, test_count, xdist_args):
+@pytest.mark.parametrize('pyfile_count', [1, 2])
+def test_mypy_success(testdir, pyfile_count, xdist_args):
     """Verify that running on a module with no type errors passes."""
     testdir.makepyfile(
         **{
-            'test_' + str(test_i): '''
-                def myfunc(x: int) -> int:
+            'pyfile_' + str(pyfile_i): '''
+                def pyfunc(x: int) -> int:
                     return x * 2
             '''
-            for test_i in range(test_count)
+            for pyfile_i in range(pyfile_count)
         }
     )
     result = testdir.runpytest_subprocess(*xdist_args)
     result.assert_outcomes()
     result = testdir.runpytest_subprocess('--mypy', *xdist_args)
-    result.assert_outcomes(passed=test_count)
+    mypy_file_checks = pyfile_count
+    mypy_status_check = 1
+    mypy_checks = mypy_file_checks + mypy_status_check
+    result.assert_outcomes(passed=mypy_checks)
     assert result.ret == 0
 
 
 def test_mypy_error(testdir, xdist_args):
     """Verify that running on a module with type errors fails."""
     testdir.makepyfile('''
-        def myfunc(x: int) -> str:
+        def pyfunc(x: int) -> str:
             return x * 2
     ''')
     result = testdir.runpytest_subprocess(*xdist_args)
     result.assert_outcomes()
     result = testdir.runpytest_subprocess('--mypy', *xdist_args)
-    result.assert_outcomes(failed=1)
+    mypy_file_checks = 1
+    mypy_status_check = 1
+    mypy_checks = mypy_file_checks + mypy_status_check
+    result.assert_outcomes(failed=mypy_checks)
     result.stdout.fnmatch_lines([
         '2: error: Incompatible return value*',
     ])
@@ -54,20 +60,29 @@ def test_mypy_ignore_missings_imports(testdir, xdist_args):
     Verify that --mypy-ignore-missing-imports
     causes mypy to ignore missing imports.
     """
+    module_name = 'is_always_missing'
     testdir.makepyfile('''
-        import pytest_mypy
-    ''')
+        try:
+            import {module_name}
+        except ImportError:
+            pass
+    '''.format(module_name=module_name))
     result = testdir.runpytest_subprocess('--mypy', *xdist_args)
-    result.assert_outcomes(failed=1)
+    mypy_file_checks = 1
+    mypy_status_check = 1
+    mypy_checks = mypy_file_checks + mypy_status_check
+    result.assert_outcomes(failed=mypy_checks)
     result.stdout.fnmatch_lines([
-        "1: error: Cannot find *module named 'pytest_mypy'",
+        "2: error: Cannot find *module named '{module_name}'".format(
+            module_name=module_name,
+        ),
     ])
     assert result.ret != 0
     result = testdir.runpytest_subprocess(
         '--mypy-ignore-missing-imports',
         *xdist_args
     )
-    result.assert_outcomes(passed=1)
+    result.assert_outcomes(passed=mypy_checks)
     assert result.ret == 0
 
 
@@ -78,28 +93,39 @@ def test_fails():
             assert False
     ''')
     result = testdir.runpytest_subprocess('--mypy', *xdist_args)
-    result.assert_outcomes(failed=1, passed=1)
+    test_count = 1
+    mypy_file_checks = 1
+    mypy_status_check = 1
+    mypy_checks = mypy_file_checks + mypy_status_check
+    result.assert_outcomes(failed=test_count, passed=mypy_checks)
     assert result.ret != 0
     result = testdir.runpytest_subprocess('--mypy', '-m', 'mypy', *xdist_args)
-    result.assert_outcomes(passed=1)
+    result.assert_outcomes(passed=mypy_checks)
     assert result.ret == 0
 
 
 def test_non_mypy_error(testdir, xdist_args):
     """Verify that non-MypyError exceptions are passed through the plugin."""
     message = 'This is not a MypyError.'
-    testdir.makepyfile('''
-        import pytest_mypy
+    testdir.makepyfile(conftest='''
+        def pytest_configure(config):
+            plugin = config.pluginmanager.getplugin('mypy')
 
-        def _patched_runtest(*args, **kwargs):
-            raise Exception('{message}')
+            class PatchedMypyFileItem(plugin.MypyFileItem):
+                def runtest(self):
+                    raise Exception('{message}')
 
-        pytest_mypy.MypyItem.runtest = _patched_runtest
+            plugin.MypyFileItem = PatchedMypyFileItem
     '''.format(message=message))
     result = testdir.runpytest_subprocess(*xdist_args)
     result.assert_outcomes()
     result = testdir.runpytest_subprocess('--mypy', *xdist_args)
-    result.assert_outcomes(failed=1)
+    mypy_file_checks = 1  # conftest.py
+    mypy_status_check = 1
+    result.assert_outcomes(
+        failed=mypy_file_checks,  # patched to raise an Exception
+        passed=mypy_status_check,  # conftest.py has no type errors.
+    )
     result.stdout.fnmatch_lines(['*' + message])
     assert result.ret != 0
 
@@ -159,23 +185,75 @@ def pytest_configure(config):
 
 
 def test_pytest_collection_modifyitems(testdir, xdist_args):
+    """
+    Verify that collected files which are removed in a
+    pytest_collection_modifyitems implementation are not
+    checked by mypy.
+
+    This would also fail if a MypyStatusItem were injected
+    despite there being no MypyFileItems.
+    """
     testdir.makepyfile(conftest='''
         def pytest_collection_modifyitems(session, config, items):
             plugin = config.pluginmanager.getplugin('mypy')
             for mypy_item_i in reversed([
                     i
                     for i, item in enumerate(items)
-                    if isinstance(item, plugin.MypyItem)
+                    if isinstance(item, plugin.MypyFileItem)
             ]):
                 items.pop(mypy_item_i)
     ''')
     testdir.makepyfile('''
-        def myfunc(x: int) -> str:
+        def pyfunc(x: int) -> str:
             return x * 2
 
         def test_pass():
             pass
     ''')
     result = testdir.runpytest_subprocess('--mypy', *xdist_args)
-    result.assert_outcomes(passed=1)
+    test_count = 1
+    result.assert_outcomes(passed=test_count)
     assert result.ret == 0
+
+
+def test_mypy_indirect(testdir, xdist_args):
+    """Verify that uncollected files checked by mypy cause a failure."""
+    testdir.makepyfile(bad='''
+        def pyfunc(x: int) -> str:
+            return x * 2
+    ''')
+    testdir.makepyfile(good='''
+        import bad
+    ''')
+    xdist_args.append('good.py')  # Nothing may come after xdist_args in py34.
+    result = testdir.runpytest_subprocess('--mypy', *xdist_args)
+    assert result.ret != 0
+
+
+def test_mypy_indirect_inject(testdir, xdist_args):
+    """
+    Verify that uncollected files checked by mypy because of a MypyFileItem
+    injected in pytest_collection_modifyitems cause a failure.
+    """
+    testdir.makepyfile(bad='''
+        def pyfunc(x: int) -> str:
+            return x * 2
+    ''')
+    testdir.makepyfile(good='''
+        import bad
+    ''')
+    testdir.makepyfile(conftest='''
+        import py
+        import pytest
+
+        @pytest.hookimpl(trylast=True)  # Inject as late as possible.
+        def pytest_collection_modifyitems(session, config, items):
+            plugin = config.pluginmanager.getplugin('mypy')
+            items.append(
+                plugin.MypyFileItem(py.path.local('good.py'), session),
+            )
+    ''')
+    testdir.mkdir('empty')
+    xdist_args.append('empty')  # Nothing may come after xdist_args in py34.
+    result = testdir.runpytest_subprocess('--mypy', *xdist_args)
+    assert result.ret != 0

tox.ini

@@ -80,7 +80,7 @@ deps =
 
     pytest-cov ~= 2.5.1
     pytest-randomly ~= 2.1.1
-commands = py.test -p no:mypy -n auto --cov pytest_mypy --cov-fail-under 100 --cov-report term-missing {posargs}
+commands = py.test -p no:mypy --cov pytest_mypy --cov-fail-under 100 --cov-report term-missing {posargs:-n auto} tests
 
 [testenv:static]
 deps =