fix(typing): add missing type annotations for assertNumQueries and non_debugging_runtest - #1294
Conversation
…n_debugging_runtest
|
While the change looks good on the surface, can you provide an example where it was failing? (real world use case?) |
|
Hi @kingbuzzman, Thanks for reviewing! The primary use case occurs when running Mypy in strict mode ( Real-world ExampleConsider a Django project configured with # tests/test_queries.py
import pytest
from pytest_django.asserts import assertNumQueries
from myapp.models import User
def test_user_query_count() -> None:
# 1. Context manager usage with strict typing
with assertNumQueries(2):
User.objects.filter(is_active=True).count()
# 2. Callable invocation usage
assertNumQueries(1, lambda: User.objects.get(id=1))
Mypy Error Output:
error: Function is missing a type annotation for parameter "func" [no-untyped-def]
error: Function is missing a return type annotation [no-untyped-def] |
bluetech
left a comment
There was a problem hiding this comment.
Thanks for trying to improve this.
Regarding the assertNumQueries, if we want to be completely accurate we need to do something like django-stubs: https://github.com/typeddjango/django-stubs/blob/61a3af749523e9645861057588fe67230ac6b6dd/django-stubs/test/testcases.pyi#L176-L181. But that's somewhat complex, so I punted it. But AbstractContextManager[Any] would side-step the main complexity and still be an improvement.
The non_debugging_runtest looks good.
| def assertNumQueries( | ||
| num: int, | ||
| func=..., | ||
| func: Callable[..., Any] | None = ..., |
There was a problem hiding this comment.
This should be an @overload rather than a param with default.
| using: str = ..., | ||
| **kwargs: Any, | ||
| ): ... | ||
| ) -> AbstractContextManager[None] | None: ... |
There was a problem hiding this comment.
The context manager is not exactly None, it actually returns itself. So Any would be better for now.
…n_debugging_runtest
This PR fixes missing type annotations identified during strict type checking:
pytest_django/asserts.py: Added missing parameter (func: Callable[..., Any] | None = ...) and return type (-> AbstractContextManager[None] | None) annotations to theassertNumQueriesoverload implementation.pytest_django/plugin.py: Added explicit type annotation (self: TestCaseFunction) fornon_debugging_runtestparameter and removed unnecessarynoqa: ANN001.All mypy strict checks pass cleanly.