From bd09e7accc747b17be4989d8a744024f45d8f757 Mon Sep 17 00:00:00 2001 From: David Tucker Date: Sat, 29 Feb 2020 15:56:32 -0800 Subject: [PATCH 1/7] Adjust the default tox invocation --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index e3942d8..b4a87cc 100644 --- a/tox.ini +++ b/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 = From 040975d105ee97d88f504ff95ade7e6287ed034e Mon Sep 17 00:00:00 2001 From: David Tucker Date: Sat, 29 Feb 2020 19:02:08 -0800 Subject: [PATCH 2/7] Never import pytest_mypy --- tests/test_pytest_mypy.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/test_pytest_mypy.py b/tests/test_pytest_mypy.py index e77dcad..bc61817 100644 --- a/tests/test_pytest_mypy.py +++ b/tests/test_pytest_mypy.py @@ -54,13 +54,19 @@ 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) 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( @@ -88,13 +94,15 @@ def test_fails(): 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 PatchedMypyItem(plugin.MypyItem): + def runtest(self): + raise Exception('{message}') - pytest_mypy.MypyItem.runtest = _patched_runtest + plugin.MypyItem = PatchedMypyItem '''.format(message=message)) result = testdir.runpytest_subprocess(*xdist_args) result.assert_outcomes() From 32a27f62cb1d44ecdc1345b65c9a1c24cc095268 Mon Sep 17 00:00:00 2001 From: David Tucker Date: Sat, 29 Feb 2020 19:19:05 -0800 Subject: [PATCH 3/7] Use pytest-style names in the tests --- tests/test_pytest_mypy.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_pytest_mypy.py b/tests/test_pytest_mypy.py index bc61817..bf172ac 100644 --- a/tests/test_pytest_mypy.py +++ b/tests/test_pytest_mypy.py @@ -14,29 +14,29 @@ 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) + result.assert_outcomes(passed=pyfile_count) 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) @@ -178,7 +178,7 @@ def pytest_collection_modifyitems(session, config, items): items.pop(mypy_item_i) ''') testdir.makepyfile(''' - def myfunc(x: int) -> str: + def pyfunc(x: int) -> str: return x * 2 def test_pass(): From 07910700b7bb1cd5e43560067bc31e659d731a11 Mon Sep 17 00:00:00 2001 From: David Tucker Date: Sat, 29 Feb 2020 19:21:48 -0800 Subject: [PATCH 4/7] Add a missing test docstring --- tests/test_pytest_mypy.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_pytest_mypy.py b/tests/test_pytest_mypy.py index bf172ac..9273dfe 100644 --- a/tests/test_pytest_mypy.py +++ b/tests/test_pytest_mypy.py @@ -167,6 +167,11 @@ 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. + """ testdir.makepyfile(conftest=''' def pytest_collection_modifyitems(session, config, items): plugin = config.pluginmanager.getplugin('mypy') From 2120ff1b301e0898692605affe1f9ae57435fdef Mon Sep 17 00:00:00 2001 From: David Tucker Date: Sat, 29 Feb 2020 15:51:04 -0800 Subject: [PATCH 5/7] Split MypyFileItem out of MypyItem --- src/pytest_mypy.py | 35 ++++++++++++++++++++--------------- tests/test_pytest_mypy.py | 6 +++--- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/pytest_mypy.py b/src/pytest_mypy.py index ac64289..8cc3da5 100644 --- a/src/pytest_mypy.py +++ b/src/pytest_mypy.py @@ -73,14 +73,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,9 +89,9 @@ def pytest_collect_file(path, parent): return None -class MypyItem(pytest.Item, pytest.File): +class MypyItem(pytest.Item): - """A File that Mypy Runs On.""" + """A Mypy-related test Item.""" MARKER = 'mypy' @@ -99,6 +99,20 @@ 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( @@ -112,7 +126,7 @@ def runtest(self): abspaths=[ os.path.abspath(str(item.fspath)) for item in self.session.items - if isinstance(item, MypyItem) + if isinstance(item, MypyFileItem) ], ) ) @@ -129,15 +143,6 @@ 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) - def _cached_json_results(results_path, results_factory=None): """ diff --git a/tests/test_pytest_mypy.py b/tests/test_pytest_mypy.py index 9273dfe..91981be 100644 --- a/tests/test_pytest_mypy.py +++ b/tests/test_pytest_mypy.py @@ -98,11 +98,11 @@ def test_non_mypy_error(testdir, xdist_args): def pytest_configure(config): plugin = config.pluginmanager.getplugin('mypy') - class PatchedMypyItem(plugin.MypyItem): + class PatchedMypyFileItem(plugin.MypyFileItem): def runtest(self): raise Exception('{message}') - plugin.MypyItem = PatchedMypyItem + plugin.MypyFileItem = PatchedMypyFileItem '''.format(message=message)) result = testdir.runpytest_subprocess(*xdist_args) result.assert_outcomes() @@ -178,7 +178,7 @@ def pytest_collection_modifyitems(session, config, items): 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) ''') From 6cd5304b7212dafe057ea2fe1822927a8748d818 Mon Sep 17 00:00:00 2001 From: David Tucker Date: Sat, 29 Feb 2020 16:16:06 -0800 Subject: [PATCH 6/7] Add _mypy_results --- src/pytest_mypy.py | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/pytest_mypy.py b/src/pytest_mypy.py index 8cc3da5..8780682 100644 --- a/src/pytest_mypy.py +++ b/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 @@ -115,21 +116,7 @@ class MypyFileItem(MypyItem, pytest.File): 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, MypyFileItem) - ], - ) - ) + results = _mypy_results(self.session) abspath = os.path.abspath(str(self.fspath)) errors = results['abspath_errors'].get(abspath) if errors: @@ -144,6 +131,25 @@ def reportinfo(self): ) +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): """ Read results from results_path if it exists; From 0b66e30c7ba22cc626369c7c52a3fef6255385a9 Mon Sep 17 00:00:00 2001 From: David Tucker Date: Sat, 29 Feb 2020 17:19:05 -0800 Subject: [PATCH 7/7] Create MypyStatusItem --- src/pytest_mypy.py | 33 ++++++++++++++++ tests/test_pytest_mypy.py | 81 +++++++++++++++++++++++++++++++++++---- 2 files changed, 106 insertions(+), 8 deletions(-) diff --git a/src/pytest_mypy.py b/src/pytest_mypy.py index 8780682..71f8882 100644 --- a/src/pytest_mypy.py +++ b/src/pytest_mypy.py @@ -90,6 +90,24 @@ def pytest_collect_file(path, parent): return None +@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)) + + class MypyItem(pytest.Item): """A Mypy-related test Item.""" @@ -131,6 +149,21 @@ def reportinfo(self): ) +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( diff --git a/tests/test_pytest_mypy.py b/tests/test_pytest_mypy.py index 91981be..789405d 100644 --- a/tests/test_pytest_mypy.py +++ b/tests/test_pytest_mypy.py @@ -29,7 +29,10 @@ def pyfunc(x: int) -> int: result = testdir.runpytest_subprocess(*xdist_args) result.assert_outcomes() result = testdir.runpytest_subprocess('--mypy', *xdist_args) - result.assert_outcomes(passed=pyfile_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 @@ -42,7 +45,10 @@ def pyfunc(x: int) -> str: 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*', ]) @@ -62,7 +68,10 @@ def test_mypy_ignore_missings_imports(testdir, xdist_args): 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([ "2: error: Cannot find *module named '{module_name}'".format( module_name=module_name, @@ -73,7 +82,7 @@ def test_mypy_ignore_missings_imports(testdir, xdist_args): '--mypy-ignore-missing-imports', *xdist_args ) - result.assert_outcomes(passed=1) + result.assert_outcomes(passed=mypy_checks) assert result.ret == 0 @@ -84,10 +93,14 @@ 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 @@ -107,7 +120,12 @@ def runtest(self): 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 @@ -171,6 +189,9 @@ 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): @@ -190,5 +211,49 @@ 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