Skip to content

Fix "HttpResponse" has no attribute "data" on TestCases #205

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

Merged
merged 3 commits into from
May 29, 2022

Conversation

XF-FW
Copy link
Contributor

@XF-FW XF-FW commented Mar 31, 2022

I have made things!

Related issues

Fixes #204

@XF-FW
Copy link
Contributor Author

XF-FW commented May 28, 2022

Ok, this is me trying to ensure myself, that this fix is the correct one and to help anyone reading to interpret this as well:

The root of the issue is this error:
"_MonkeyPatchedWSGIResponse" has no attribute "data"

I think it's worth mentioning that the error has changed slightly, due to changes made in typeddjango/django-stubs#925.

Anyway, we can see that the revealed type is wrong, by this partial example:

response = self.client.get(self.url, format="json")
reveal_type(self.client)  
# Revealed type is "django.test.client.Client"
# Expected type would be "rest_framework.test.APIClient"

Ok, let me put out some links here, so we can quickly check whatever I'm writing:

Here we have the definition for the different testcases classes that DRF provides:
https://github.com/encode/django-rest-framework/blob/e5fb9af0eaffde683fa0af3987085f86cf0d2640/rest_framework/test.py#L345
And the equivalent stubs:

class APITransactionTestCase(testcases.TransactionTestCase):

The stubs for the parent testcase class that Django provides:
https://github.com/typeddjango/django-stubs/blob/42d8e18bf8d35482cdb4935866bd35304a74fc9d/django-stubs/test/testcases.pyi#L65

And stubs for Django's Client class:
https://github.com/typeddjango/django-stubs/blob/42d8e18bf8d35482cdb4935866bd35304a74fc9d/django-stubs/test/client.pyi#L150

Finally, for completeness, for the curious ones, this is where client is assigned to an instance of client_class
https://github.com/django/django/blob/8c0886b068ba4e224dd78104b93c9638b860b398/django/test/testcases.py#L432

Oof. I promise that's everything.
Ok, the problem here, seems to be that django-stubs declares the following:

class SimpleTestCase(unittest.TestCase):
    client_class: Type[Client] = ...
    client: Client

Client is typed as:

class Client(ClientMixin, _RequestFactory[_MonkeyPatchedWSGIResponse]):
    ...
    def request(self, **request: Any) -> _MonkeyPatchedWSGIResponse: ...

The initial error, now makes sense. It's using Django Client type for the client instance variable, instead of APIClient. Let's just check how are the stubs of APIClient defined:

from rest_framework.response import Response

...

class APIClient(APIRequestFactory, DjangoClient):
    ...
    def request(self, **kwargs: Any) -> Response: ...  # type: ignore[override]

Although it's pretty trivial knowledge, but we have our hands dirty, so,
while we're at it, we can confirm that rest_framework.response.Response has indeed a data attribute:

class Response(SimpleTemplateResponse):
    data: Any = ...
    ...

Anyway, back to reality. The problem is that we're using the parent class for the client type. This doesn't allow us to use the methods only defined in the child class. We need to narrow the type. I think that's what I did with this PR, by simply declaring client as having the narrower type.

Let's wrap this up, because this has like 100x times the size of the PR.

PS: Wrote an accompanying test.

@XF-FW XF-FW marked this pull request as ready for review May 28, 2022 02:06
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.

Great explanation ;)


class APILiveServerTestCase(testcases.LiveServerTestCase):
client_class: Type[APIClient] = ...
client: APIClient

class URLPatternsTestCase(testcases.SimpleTestCase): ...
Copy link
Member

Choose a reason for hiding this comment

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

What about this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

main: |
from rest_framework import test, status

class MyTest(test.APITestCase):
Copy link
Member

Choose a reason for hiding this comment

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

Can you please check other types as well? You can use https://github.com/typeddjango/pytest-mypy-plugins#2-parametrized

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the link, btw. It was pretty helpful.

@XF-FW XF-FW changed the title [WIP] Fix "HttpResponse" has no attribute "data" on TestCases Fix "HttpResponse" has no attribute "data" on TestCases May 29, 2022
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.

Thank you!

@sobolevn sobolevn merged commit a2f30a0 into typeddjango:master May 29, 2022
@XF-FW XF-FW deleted the patch-1 branch May 29, 2022 11:16
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.

"HttpResponse" has no attribute "data"
3 participants