diff --git a/src/pytest_mypy.py b/src/pytest_mypy.py index 6ba0a40..54ba108 100644 --- a/src/pytest_mypy.py +++ b/src/pytest_mypy.py @@ -125,26 +125,16 @@ def from_parent(cls, *args, **kwargs): def collect(self): """Create a MypyFileItem for the File.""" yield MypyFileItem.from_parent(parent=self, name=nodeid_name) - - -@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.from_parent(parent=session, name=nodeid_name), - ) + # 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. + if not any( + isinstance(item, MypyStatusItem) for item in self.session.items + ): + yield MypyStatusItem.from_parent( + parent=self, + name=nodeid_name + "-status", + ) class MypyItem(pytest.Item): diff --git a/tests/test_pytest_mypy.py b/tests/test_pytest_mypy.py index 45c211c..29f9ea9 100644 --- a/tests/test_pytest_mypy.py +++ b/tests/test_pytest_mypy.py @@ -1,3 +1,6 @@ +import signal +import textwrap + import pytest @@ -214,38 +217,6 @@ def pytest_configure(config): assert result.ret == 0 -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.MypyFileItem) - ]): - items.pop(mypy_item_i) - ''') - testdir.makepyfile(''' - def pyfunc(x: int) -> str: - return x * 2 - - def test_pass(): - pass - ''') - result = testdir.runpytest_subprocess('--mypy', *xdist_args) - 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=''' @@ -259,38 +230,6 @@ def pyfunc(x: int) -> str: 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.from_parent( - parent=session, - name=str(py.path.local('good.py')), - ), - ) - ''') - name = 'empty' - testdir.mkdir(name) - result = testdir.runpytest_subprocess('--mypy', *xdist_args, name) - assert result.ret != 0 - - def test_api_error_formatter(testdir, xdist_args): """Ensure that the plugin can be configured in a conftest.py.""" testdir.makepyfile(bad=''' @@ -333,3 +272,87 @@ def pyfunc(x): '1: error: Function is missing a type annotation', ]) assert result.ret != 0 + + +def test_looponfail(testdir): + """Ensure that the plugin works with --looponfail.""" + + pass_source = textwrap.dedent( + """\ + def pyfunc(x: int) -> int: + return x * 2 + """, + ) + fail_source = textwrap.dedent( + """\ + def pyfunc(x: int) -> str: + return x * 2 + """, + ) + pyfile = testdir.makepyfile(fail_source) + looponfailroot = testdir.mkdir("looponfailroot") + looponfailroot_pyfile = looponfailroot.join(pyfile.basename) + pyfile.move(looponfailroot_pyfile) + pyfile = looponfailroot_pyfile + testdir.makeini( + textwrap.dedent( + """\ + [pytest] + looponfailroots = {looponfailroots} + """.format( + looponfailroots=looponfailroot, + ), + ), + ) + + child = testdir.spawn_pytest( + "--mypy --looponfail " + str(pyfile), + expect_timeout=30.0, + ) + + def _expect_session(): + child.expect("==== test session starts ====") + + def _expect_failure(): + _expect_session() + child.expect("==== FAILURES ====") + child.expect(pyfile.basename + " ____") + child.expect("2: error: Incompatible return value") + # These only show with mypy>=0.730: + # child.expect("==== mypy ====") + # child.expect("Found 1 error in 1 file (checked 1 source file)") + child.expect("2 failed") + child.expect("#### LOOPONFAILING ####") + _expect_waiting() + + def _expect_waiting(): + child.expect("#### waiting for changes ####") + child.expect("Watching") + + def _fix(): + pyfile.write(pass_source) + _expect_changed() + _expect_success() + + def _expect_changed(): + child.expect("MODIFIED " + str(pyfile)) + + def _expect_success(): + for _ in range(2): + _expect_session() + # These only show with mypy>=0.730: + # child.expect("==== mypy ====") + # child.expect("Success: no issues found in 1 source file") + child.expect("2 passed") + _expect_waiting() + + def _break(): + pyfile.write(fail_source) + _expect_changed() + _expect_failure() + + _expect_failure() + _fix() + _break() + _fix() + child.kill(signal.SIGTERM) diff --git a/tox.ini b/tox.ini index 92d12bd..73eac3d 100644 --- a/tox.ini +++ b/tox.ini @@ -132,6 +132,8 @@ deps = mypy0.79: mypy >= 0.790, < 0.800 mypy0.7x: mypy >= 0.700, < 0.800 + pexpect ~= 4.8.0 + commands = py.test -p no:mypy --cov pytest_mypy --cov-fail-under 100 --cov-report term-missing {posargs:-n auto} tests [testenv:publish]