Add new headers parameter to test classes #1529

Alexerson · 2023-06-05T19:24:44Z

Adding the new headers parameter to django.test.RequestFactory, AsyncRequestFactory, Client, and AsyncClient classes

Related issues

Refs Django 4.2 Tracking Issue #1519

Also, extra was not defined on Client, but it was on AsyncClient, so I assumed it was a mistake and added it.

django-stubs/test/client.pyi

adamchainz

Thanks, looks good. Happy to see this feature I suggested getting proper type hints. I would like another maintainer to double check though.

sobolevn · 2023-06-06T04:55:32Z

django-stubs/test/client.pyi

    ) -> _T: ...

 class RequestFactory(_RequestFactory[WSGIRequest]): ...

 class _AsyncRequestFactory(_RequestFactory[_T]):
    def request(self, **request: Any) -> _T: ...
    def generic(
-        self, method: str, path: str, data: Any = ..., content_type: str | None = ..., secure: bool = ..., **extra: Any
+        self,


Why do we have this method? It is inherited from _RequestFactory and they are identical. Maybe we can remove this one?

And def request as well.

It’s most likely here because Django redefines it, and when the stubs are generated, the stub is added.
It would probably make sense to remove both and rely on the types in RequestFactory.

See https://github.com/django/django/blob/51fdea68415c580cd753990e3238ae2e0f015263/django/test/client.py#L610

sobolevn

LOOKS SOLID! 👍
Thanks!

intgr · 2023-06-06T09:53:48Z

django-stubs/test/client.pyi

+        data: _GetDataType = ...,
+        secure: bool = ...,
+        *,
+        headers: dict[str, Any] | None = ...,


Any reason to type the dict value as Any? Is there any type besides str that makes sense?

Django supports anything that can be cast to a string, such as headers={"x-user-id": user.id}.

Could be solved easily with str(user.id) in user code

I tested this, I don't think there's any "casting to string" that happens. Django's Client just passes the value verbatim to the view function.

So this actually misbehaves, since request.headers[x] is defined to return str, but here it returned list. And it can result code behaving differently during testing and with real world requests, causing more hair pulling for users.

For testing really weird cases there's always the **extra kwarg escape hatch.

Anyway let's continue this discussion in #1534.

Alexerson mentioned this pull request Jun 5, 2023

Django 4.2 Tracking Issue #1519

Open

30 tasks

Alexerson force-pushed the headers-parameter-django-4.2 branch 2 times, most recently from 7d84281 to c7dff6f Compare June 5, 2023 19:37

Add new headers parameter to test classes

6411ed6

Alexerson force-pushed the headers-parameter-django-4.2 branch from c7dff6f to 6411ed6 Compare June 5, 2023 19:55

sobolevn requested a review from intgr June 5, 2023 20:15

adamchainz reviewed Jun 5, 2023

View reviewed changes

django-stubs/test/client.pyi Outdated Show resolved Hide resolved

Tighten extra type hint for Client and AsyncClient

5ce18c8

adamchainz approved these changes Jun 5, 2023

View reviewed changes

sobolevn reviewed Jun 6, 2023

View reviewed changes

Remove useless method redefinition in AsyncRequestFactory

e315646

sobolevn approved these changes Jun 6, 2023

View reviewed changes

sobolevn merged commit 87fddd3 into typeddjango:master Jun 6, 2023

intgr mentioned this pull request Jun 6, 2023

CI: Add typecheck suppressions due to django-stubs changes typeddjango/djangorestframework-stubs#425

Closed

intgr reviewed Jun 6, 2023

View reviewed changes

This was referenced Jun 6, 2023

Change test Client headers parameter to Mapping[str, str] #1534

Open

Use Mapping instead of dict for test Client headers, add missing RequestFactory parameter #1537

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new headers parameter to test classes #1529

Add new headers parameter to test classes #1529

Alexerson commented Jun 5, 2023

adamchainz left a comment

sobolevn Jun 6, 2023 •

edited

Loading

sobolevn Jun 6, 2023

Alexerson Jun 6, 2023 •

edited

Loading

Alexerson Jun 6, 2023

sobolevn left a comment

intgr Jun 6, 2023

adamchainz Jun 6, 2023

intgr Jun 6, 2023 •

edited

Loading

intgr Jun 6, 2023

Add new headers parameter to test classes #1529

Add new headers parameter to test classes #1529

Conversation

Alexerson commented Jun 5, 2023

Related issues

adamchainz left a comment

Choose a reason for hiding this comment

sobolevn Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

sobolevn Jun 6, 2023

Choose a reason for hiding this comment

Alexerson Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

Alexerson Jun 6, 2023

Choose a reason for hiding this comment

sobolevn left a comment

Choose a reason for hiding this comment

intgr Jun 6, 2023

Choose a reason for hiding this comment

adamchainz Jun 6, 2023

Choose a reason for hiding this comment

intgr Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

intgr Jun 6, 2023

Choose a reason for hiding this comment

sobolevn Jun 6, 2023 •

edited

Loading

Alexerson Jun 6, 2023 •

edited

Loading

intgr Jun 6, 2023 •

edited

Loading