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

Specify client attribute of TestCase classes as APIClient #105

Closed
wants to merge 1 commit into from
Closed

Specify client attribute of TestCase classes as APIClient #105

wants to merge 1 commit into from

Conversation

sidmitra
Copy link

@sidmitra sidmitra commented Oct 29, 2020

  • Fixes the following issue when you use self.client in any DRF test class
    Incompatible types in assignment (expression has type "HttpResponse", variable has type "Response")

Description

I've recently upgraded to the latest master of both repos and ran into the above error. The issue seems to be that DRF APITestCase classes seem to be revealing type as HttpResponse instead of drf Response.
The correctclient_class type hints exist here. But this alone doesn't seem enough to correctly type check all uses of the client. If one uses self.client inside a test method the client class still gets Client class instead of APIClient, because of the django-stubs here.

This new error could be because of the recent changes to django-stubs that have surfaced additional type hints?

Sample Test case

from rest_framework.test import APITestCase
from rest_framework.response import Response

class MyTest(APITestCase):
    ...
    
    def test_method(self):
       # error:
       # Incompatible types in assignment (expression has type "HttpResponse", variable has type "Response")
       response: Response = self.client.post("/dummy")

@sidmitra sidmitra changed the title Add type hint to client attribute of TestCase classes Add type hint to the client attribute of TestCase classes Oct 29, 2020
@sidmitra sidmitra changed the title Add type hint to the client attribute of TestCase classes Specify client attribute of TestCase classes as APIClient Oct 29, 2020
@sobolevn sobolevn requested a review from Goldziher October 29, 2020 00:06
@sobolevn
Copy link
Member

Thanks! Can you please also add a test case?

@sidmitra
Copy link
Author

@sobolevn Yes will do.

@Goldziher
Copy link
Member

This is failing in the pipeline

@Goldziher
Copy link
Member

Looking at this - this seems incorrect and not supported by the actual DRF code.

@sidmitra
Copy link
Author

sidmitra commented Nov 2, 2020

@Goldziher You're probably right.

  • DRF doesn't seem to define a client attribute here.
  • django-stubs is the one adding type hints for this client attribute here, which are perhaps getting inherited by drf-stubs.
    So i was kind of over-riding it to be more precise; But since it breaks tests, will figure out a better way.

Let me figure it out in the next day or so.

@Goldziher
Copy link
Member

This PR can be closed @sobolevn

Copy link
Member

@Goldziher Goldziher left a comment

Choose a reason for hiding this comment

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

Should be closed

@SETTER2000
Copy link

response: Any = CurrentClass.client.get('/')
self.assertEqual(len(response.data), 3)

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.

4 participants