Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support mypy@1.8.0 #139

Merged
merged 1 commit into from Feb 29, 2024
Merged

Support mypy@1.8.0 #139

merged 1 commit into from Feb 29, 2024

Conversation

antecrescent
Copy link
Contributor

@antecrescent antecrescent commented Feb 25, 2024

mypy-1.8.0 added a third parameter to the flush_error signature (python/mypy@8c57df0). 97aff1e updated flush_error in pytest_mypy_plugins/item.py to match it, but failed taking into account that <mypy-1.8.0 still expects the two-parameter signature to flush errors after a file is processed.

As a consequence, pytest test cases fail if mypy's process_stale_scc function calls our three-parameter flush_error with two arguments.
I'm not familiar with the innards of mypy but this happens every time pytest runs with the --mypy-same-process option.

Reproducible setup:

$ pip install mypy==1.7.1
$ pip install -r requirements.txt
$ pytest --mypy-same-process
Output
============================= test session starts ==============================
platform linux -- Python 3.12.2, pytest-8.0.2, pluggy-1.4.0
rootdir: /root/pytest-mypy-plugins
configfile: pyproject.toml
plugins: mypy-plugins-3.0.0
collected 60 items

pytest_mypy_plugins/tests/test-extension.yml F
pytest_mypy_plugins/tests/test-mypy-config.yml F
pytest_mypy_plugins/tests/test-parametrized.yml FFFFFFFFFF
pytest_mypy_plugins/tests/test-paths-from-env.yml F
pytest_mypy_plugins/tests/test-regex_assertions.yml FFFFFFFF
pytest_mypy_plugins/tests/test-simple-cases.yml FFFFFFFFF
pytest_mypy_plugins/tests/test_configs/test_join_toml_configs.py ......
pytest_mypy_plugins/tests/test_explicit_configs.py ......
pytest_mypy_plugins/tests/test_input_schema.py .......
pytest_mypy_plugins/tests/test_utils.py ...........

=================================== FAILURES ===================================
_______________________ reveal_type_extension_is_loaded ________________________
Traceback (most recent call last):
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/runner.py", line 342, in from_call
    result: Optional[TResult] = func()
                                ^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/runner.py", line 263, in <lambda>
    lambda: ihook(item=item, **kwds), when=when, reraise=reraise
            ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/pluggy/_hooks.py", line 501, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/pluggy/_manager.py", line 119, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/pluggy/_callers.py", line 138, in _multicall
    raise exception.with_traceback(exception.__traceback__)
  File "/tmp/venv/lib/python3.12/site-packages/pluggy/_callers.py", line 121, in _multicall
    teardown.throw(exception)  # type: ignore[union-attr]
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/threadexception.py", line 87, in pytest_runtest_call
    yield from thread_exception_runtest_hook()
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/threadexception.py", line 63, in thread_exception_runtest_hook
    yield
  File "/tmp/venv/lib/python3.12/site-packages/pluggy/_callers.py", line 121, in _multicall
    teardown.throw(exception)  # type: ignore[union-attr]
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/unraisableexception.py", line 90, in pytest_runtest_call
    yield from unraisable_exception_runtest_hook()
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/unraisableexception.py", line 65, in unraisable_exception_runtest_hook
    yield
  File "/tmp/venv/lib/python3.12/site-packages/pluggy/_callers.py", line 121, in _multicall
    teardown.throw(exception)  # type: ignore[union-attr]
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/logging.py", line 839, in pytest_runtest_call
    yield from self._runtest_for(item, "call")
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/logging.py", line 822, in _runtest_for
    yield
  File "/tmp/venv/lib/python3.12/site-packages/pluggy/_callers.py", line 121, in _multicall
    teardown.throw(exception)  # type: ignore[union-attr]
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/capture.py", line 882, in pytest_runtest_call
    return (yield)
            ^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/pluggy/_callers.py", line 121, in _multicall
    teardown.throw(exception)  # type: ignore[union-attr]
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/skipping.py", line 256, in pytest_runtest_call
    return (yield)
            ^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/pluggy/_callers.py", line 102, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/runner.py", line 178, in pytest_runtest_call
    raise e
  File "/tmp/venv/lib/python3.12/site-packages/_pytest/runner.py", line 170, in pytest_runtest_call
    item.runtest()
  File "/root/pytest-mypy-plugins/pytest_mypy_plugins/item.py", line 273, in runtest
    returncode, (stdout, stderr) = self.typecheck_in_same_process(execution_path, mypy_cmd_options)
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/pytest-mypy-plugins/pytest_mypy_plugins/item.py", line 231, in typecheck_in_same_process
    return_code = run_mypy_typechecking(mypy_cmd_options, stdout=stdout, stderr=stderr)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/pytest-mypy-plugins/pytest_mypy_plugins/item.py", line 99, in run_mypy_typechecking
    build.build(sources, options, flush_errors=flush_errors, fscache=fscache, stdout=stdout, stderr=stderr)
  File "mypy/build.py", line 189, in build
  File "mypy/build.py", line 263, in _build
  File "mypy/build.py", line 2941, in dispatch
  File "mypy/build.py", line 3339, in process_graph
  File "mypy/build.py", line 3461, in process_stale_scc
TypeError: run_mypy_typechecking.<locals>.flush_errors() missing 1 required positional argument: 'serious'

[error repeats for all test cases that don't run mypy in a sub-process]

=========================== short test summary info ============================
FAILED pytest_mypy_plugins/tests/test-extension.yml::reveal_type_extension_is_loaded
FAILED pytest_mypy_plugins/tests/test-mypy-config.yml::custom_mypy_config_disallow_any_explicit_set
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::only_main[a=1,revealed_type=builtins.int]
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::only_main[a=1.0,revealed_type=builtins.float]
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::with_extra[a=2,b=None,rt=Any]
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::with_extra[a=3,b=3,rt=Any]
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::with_out[what=cat,rt=builtins.str]
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::with_out[what=dog,rt=builtins.str]
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::parametrized_can_skip_mypy_config_section[val=False]
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::parametrized_can_skip_mypy_config_section[val=True]
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::with_mypy_config[allow_any=true]
FAILED pytest_mypy_plugins/tests/test-parametrized.yml::with_mypy_config[allow_any=false]
FAILED pytest_mypy_plugins/tests/test-paths-from-env.yml::add_mypypath_env_var_to_package_search
FAILED pytest_mypy_plugins/tests/test-regex_assertions.yml::expected_message_regex
FAILED pytest_mypy_plugins/tests/test-regex_assertions.yml::expected_message_regex_with_out
FAILED pytest_mypy_plugins/tests/test-regex_assertions.yml::regex_with_out_does_not_hang
FAILED pytest_mypy_plugins/tests/test-regex_assertions.yml::regex_with_comment_does_not_hang
FAILED pytest_mypy_plugins/tests/test-regex_assertions.yml::expected_single_message_regex
FAILED pytest_mypy_plugins/tests/test-regex_assertions.yml::rexex_but_not_turned_on
FAILED pytest_mypy_plugins/tests/test-regex_assertions.yml::rexex_but_turned_off
FAILED pytest_mypy_plugins/tests/test-regex_assertions.yml::regex_does_not_match
FAILED pytest_mypy_plugins/tests/test-simple-cases.yml::simplest_case - TypeE...
FAILED pytest_mypy_plugins/tests/test-simple-cases.yml::revealed_type_with_environment
FAILED pytest_mypy_plugins/tests/test-simple-cases.yml::revealed_type_with_disabled_cache
FAILED pytest_mypy_plugins/tests/test-simple-cases.yml::external_output_lines
FAILED pytest_mypy_plugins/tests/test-simple-cases.yml::create_files - TypeEr...
FAILED pytest_mypy_plugins/tests/test-simple-cases.yml::error_case - TypeErro...
FAILED pytest_mypy_plugins/tests/test-simple-cases.yml::skip_if_false - TypeE...
FAILED pytest_mypy_plugins/tests/test-simple-cases.yml::fail_if_message_does_not_match
FAILED pytest_mypy_plugins/tests/test-simple-cases.yml::fail_if_message_from_outdoes_not_match
======================== 30 failed, 30 passed in 48.83s ========================

Issue pip install mypy==1.8.0 and the same test run succeeds.

gentoo-bot pushed a commit to gentoo/guru that referenced this pull request Feb 26, 2024
Both versions were previously incompatible with dev-python/mypy-1.8.0 due
to an incompatibility with mypy's new flush_errors signature.
See also: typeddjango/pytest-mypy-plugins#139

Signed-off-by: Lucio Sauer <watermanpaint@posteo.net>
@sobolevn
Copy link
Member

@antecrescent I suggest making a *args function here to support older versions instead.
There are projects that use matrix checks with different mypy versions. Let's change #138

@antecrescent
Copy link
Contributor Author

I've tried two possible approaches but 'm not sure how to proceed and I'd be happy to hear your thoughts.

*args approach

It does not work, because mypy does not call flush_errors with keyword arguments.
Also the static type-checking fails because mypy complains about our flush_errors signature not matching its own:

diff --git a/pytest_mypy_plugins/item.py b/pytest_mypy_plugins/item.py
index 2094eed..937d9f0 100644
--- a/pytest_mypy_plugins/item.py
+++ b/pytest_mypy_plugins/item.py
@@ -85,7 +85,7 @@ def run_mypy_typechecking(cmd_options: List[str], stdout: TextIO, stderr: TextIO
     # discard filename parameter '_'. Mypy uses it to generate
     # one junit-xml test entry per file with failures (--junit-format per_file)
     # and we don't support mypy's --junit-xml option in the first place.
-    def flush_errors(_: str | None, new_messages: List[str], serious: bool) -> None:
+    def flush_errors(*_: str | None, new_messages: List[str], serious: bool) -> None:
         error_messages.extend(new_messages)
         f = stderr if serious else stdout
         try:
pytest_mypy_plugins/item.py:99: error: Argument "flush_errors" to "build" has incompatible type "Callable[[VarArg(str | None), NamedArg(list[str], 'new_messages'), NamedArg(bool, 'serious')], None]"; expected "Callable[[str | None, list[str], bool], None] | None"  [arg-type]

Branch conditional on mypy version

This works fine but type-checking fails.

diff --git a/pytest_mypy_plugins/item.py b/pytest_mypy_plugins/item.py
index 2094eed..466cdac 100644
--- a/pytest_mypy_plugins/item.py
+++ b/pytest_mypy_plugins/item.py
@@ -7,6 +7,7 @@ import tempfile
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Dict, List, Optional, TextIO, Tuple, Union
 
+from packaging.version import Version
 import py
 import pytest
 from _pytest._code import ExceptionInfo
@@ -16,6 +17,7 @@ from _pytest.config import Config
 from mypy import build
 from mypy.fscache import FileSystemCache
 from mypy.main import process_options
+from mypy.version import __version__ as mypy_version
 
 if TYPE_CHECKING:
     from _pytest._code.code import _TracebackStyle
@@ -82,10 +84,7 @@ def run_mypy_typechecking(cmd_options: List[str], stdout: TextIO, stderr: TextIO
 
     error_messages = []
 
-    # discard filename parameter '_'. Mypy uses it to generate
-    # one junit-xml test entry per file with failures (--junit-format per_file)
-    # and we don't support mypy's --junit-xml option in the first place.
-    def flush_errors(_: str | None, new_messages: List[str], serious: bool) -> None:
+    def flush_errors(new_messages: List[str], serious: bool) -> None:
         error_messages.extend(new_messages)
         f = stderr if serious else stdout
         try:
@@ -96,7 +95,19 @@ def run_mypy_typechecking(cmd_options: List[str], stdout: TextIO, stderr: TextIO
             sys.exit(ReturnCodes.FATAL_ERROR)
 
     try:
-        build.build(sources, options, flush_errors=flush_errors, fscache=fscache, stdout=stdout, stderr=stderr)
+        if Version(mypy_version) <  Version("1.8.0"):
+            def short_args_wrapper(new_messages: List[str], serious: bool) -> None:
+                flush_errors(new_messages, serious)
+
+            build.build(sources, options, flush_errors=short_args_wrapper, fscache=fscache, stdout=stdout, stderr=stderr)
+        else:
+            # discard filename parameter '_'. Mypy >= 1.8.0 uses it to generate
+            # one junit-xml test entry per file with failures (--junit-format per_file)
+            # and we don't support mypy's --junit-xml option in the first place.
+            def long_args_wrapper(_: str | None, new_messages: List[str], serious: bool) -> None:
+                flush_errors(new_messages, serious)
+
+            build.build(sources, options, flush_errors=long_args_wrapper, fscache=fscache, stdout=stdout, stderr=stderr)
 
     except SystemExit as sysexit:
         return sysexit.code

The problem is that mypy type-checks both branches against its own signature, which obviously fails since only one branch has the correct signature for either <mypy-1.8.0 or >=mypy-1.8.0.
With mypy 1.7.1

# mypy .
pytest_mypy_plugins/item.py:110: error: Argument "flush_errors" to "build" has incompatible type "Callable[[str | None, list[str], bool], None]"; expected "Callable[[list[str], bool], None] | None"  [arg-type]
Found 1 error in 1 file (checked 11 source files)

With mypy 1.8.0

# mypy .
pytest_mypy_plugins/item.py:102: error: Argument "flush_errors" to "build" has incompatible type "Callable[[list[str], bool], None]"; expected "Callable[[str | None, list[str], bool], None] | None"  [arg-type]
Found 1 error in 1 file (checked 11 source files)

Workaround:
Patch the second approach as follows and run mypy with --no-warn-unused-ignores. The reason for --no-warn-unused-ignores is the same as above. One branch passes the type-checking despite its type: ignore comment:

diff --git a/pytest_mypy_plugins/item.py b/pytest_mypy_plugins/item.py
index 466cdac..83fffb2 100644
--- a/pytest_mypy_plugins/item.py
+++ b/pytest_mypy_plugins/item.py
@@ -99,7 +99,7 @@ def run_mypy_typechecking(cmd_options: List[str], stdout: TextIO, stderr: TextIO
             def short_args_wrapper(new_messages: List[str], serious: bool) -> None:
                 flush_errors(new_messages, serious)
 
-            build.build(sources, options, flush_errors=short_args_wrapper, fscache=fscache, stdout=stdout, stderr=stderr)
+            build.build(sources, options, flush_errors=short_args_wrapper, fscache=fscache, stdout=stdout, stderr=stderr) # type: ignore
         else:
             # discard filename parameter '_'. Mypy >= 1.8.0 uses it to generate
             # one junit-xml test entry per file with failures (--junit-format per_file)
@@ -107,7 +107,7 @@ def run_mypy_typechecking(cmd_options: List[str], stdout: TextIO, stderr: TextIO
             def long_args_wrapper(_: str | None, new_messages: List[str], serious: bool) -> None:
                 flush_errors(new_messages, serious)
 
-            build.build(sources, options, flush_errors=long_args_wrapper, fscache=fscache, stdout=stdout, stderr=stderr)
+            build.build(sources, options, flush_errors=long_args_wrapper, fscache=fscache, stdout=stdout, stderr=stderr) # type: ignore
 
     except SystemExit as sysexit:
         return sysexit.code

@sobolevn
Copy link
Member

*args: Any?

@antecrescent
Copy link
Contributor Author

I may not understand your suggestion correctly...
def flush_errors(*_: Any, new_messages: List[str], serious: bool) -> None fails the type-checking with:
pytest_mypy_plugins/item.py:99: error: Argument "flush_errors" to "build" has incompatible type "Callable[[VarArg(Any), NamedArg(list[str], 'new_messages'), NamedArg(bool, 'serious')], None]"; expected "Callable[[str | None, list[str], bool], None] | None" [arg-type]

This also fails at runtime for the same reason listed in the first approach:
TypeError: run_mypy_typechecking.<locals>.flush_errors() missing 2 required keyword-only arguments: 'new_messages' and 'ser...

@sobolevn
Copy link
Member

@antecrescent please, take a look at my suggestion :)

@antecrescent
Copy link
Contributor Author

Ah yes, makes sense! I'm not very versed in Python but this seems to be a more sensible approach. I'll rebase this to get rid my erroneous commit and fix a typo in your comment.

mypy's flush_errors function has a different arity for different
versions.
<mypy-1.8.0: 2
>=mypy-1.8.0: 3 (our flush_errors discards the first 'filename'
parameter)
@sobolevn sobolevn changed the title Bump mypy dependency to >=mypy-1.8.0 Support mypy@1.8.0 Feb 29, 2024
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sobolevn sobolevn merged commit 0c2163e into typeddjango:master Feb 29, 2024
9 checks passed
@antecrescent
Copy link
Contributor Author

Thank you for helping me fix this and teaching me a bit of Python :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants