djangoflow / django-df-auth

Opinionated Django REST auth endpoints for JWT authentication and social accounts
MIT License
10 stars 19 forks source link

Create Serializers test cases #21

Closed mrhassanfarooq closed 11 months ago

mrhassanfarooq commented 1 year ago

Hi, I have completed the changes and requested them to review on Git. Looking forward to hearing from you.

Thank you.

On Tue, May 16, 2023 at 5:37 AM Sreekanth Reddy Balne < @.***> wrote:

@.**** requested changes on this pull request.

Could you please write a test case for AuthBackendSerializerMixin? All these serializers appear to be inheriting from that.

In tests/test_app/tests.py https://github.com/djangoflow/django-df-auth/pull/21#discussion_r1194843675 :

def test_dummy(): assert 1 == 1

Could you please remove this in a separate commit?

In tests/test_app/tests.py https://github.com/djangoflow/django-df-auth/pull/21#discussion_r1194864883 :

import pytest

+from df_auth.drf.serializers import TokenCreateSerializer +from df_auth.drf.serializers import TokenObtainSerializer +from df_auth.drf.serializers import OTPObtainSerializer +from df_auth.drf.serializers import SignupSerializer +from df_auth.drf.serializers import InviteSerializer +from rest_framework_simplejwt.settings import api_settings as simplejwt_settings

Unused import. Please remove this statement.

In tests/test_app/tests.py https://github.com/djangoflow/django-df-auth/pull/21#discussion_r1194877801 :

+from df_auth.drf.serializers import TokenCreateSerializer +from df_auth.drf.serializers import TokenObtainSerializer +from df_auth.drf.serializers import OTPObtainSerializer +from df_auth.drf.serializers import SignupSerializer +from df_auth.drf.serializers import InviteSerializer

minor: I think we should compress these into a single statement.

from df_auth.drf.serializers import ( InviteSerializer, OTPObtainSerializer, SignupSerializer, TokenCreateSerializer, TokenObtainSerializer, )


In tests/test_app/tests.py https://github.com/djangoflow/django-df-auth/pull/21#discussion_r1194891087 :

+def test_token_obtain_serializer_validate():

  • user = @.***', password='12345')
  • payload = @.***','password':'12345'}
  • serializer = TokenObtainSerializer(data=payload)
  • serializer.user=user
  • assert serializer.is_valid(raise_exception=True) is True

Additional validation is needed here. Please note that the validation depends on:

  • api_settings.REQUIRED_AUTH_FIELDS, the serializer should throw an error if these fields are not provided.
  • api_settings.OPTIONAL_AUTH_FIELDS, the serializer should be able to tolerate empty data for these fields.

So, we should patch the api_settings.REQUIRED_AUTH_FIELDS and see if that is affecting the serializer validation as expected. The same goes for api_settings.OPTIONAL_AUTH_FIELDS as well.

— Reply to this email directly, view it on GitHub https://github.com/djangoflow/django-df-auth/pull/21#pullrequestreview-1428077615, or unsubscribe https://github.com/notifications/unsubscribe-auth/A7TM4HSFQWP3Z55Q7MTZIV3XGNDD7ANCNFSM6AAAAAAX3PASYE . You are receiving this because you authored the thread.Message ID: @.***>

mrhassanfarooq commented 1 year ago

Updated. Please review

On Thu, 25 May 2023 at 1:54 PM, Sreekanth Reddy Balne < @.***> wrote:

@.**** requested changes on this pull request.

In tests/test_app/tests.py https://github.com/djangoflow/django-df-auth/pull/21#discussion_r1205205519 :

  • with pytest.raises(ValidationError): assert serializer.is_valid(raise_exception=True)

with pytest.raises(ValidationError): serializer.is_valid(raise_exception=True)


In tests/test_app/tests.py https://github.com/djangoflow/django-df-auth/pull/21#discussion_r1205207831 :

  • def test_validate_success(self):
  • attrs = {
  • 'username': 'test_user',
  • 'password': 'test_password',
  • }
  • self.serializer.backend_method_name = ""
  • with pytest.raises(AuthenticationFailed):
  • self.serializer.validate(attrs)

I think a test to check what happens if a backend_method_name is not set, is required.

— Reply to this email directly, view it on GitHub https://github.com/djangoflow/django-df-auth/pull/21#pullrequestreview-1443403851, or unsubscribe https://github.com/notifications/unsubscribe-auth/A7TM4HUFVZTHJMGEGOGXYMLXH4M6BANCNFSM6AAAAAAX3PASYE . You are receiving this because you authored the thread.Message ID: @.***>

mrhassanfarooq commented 1 year ago

Hi Sreekanth, Can you please merge my code? Thank you

Best,

On Thu, May 25, 2023 at 5:47 AM Hassan Farooq @.***> wrote:

Updated. Please review

On Thu, 25 May 2023 at 1:54 PM, Sreekanth Reddy Balne < @.***> wrote:

@.**** requested changes on this pull request.

In tests/test_app/tests.py https://github.com/djangoflow/django-df-auth/pull/21#discussion_r1205205519 :

  • with pytest.raises(ValidationError): assert serializer.is_valid(raise_exception=True)

with pytest.raises(ValidationError): serializer.is_valid(raise_exception=True)


In tests/test_app/tests.py https://github.com/djangoflow/django-df-auth/pull/21#discussion_r1205207831 :

  • def test_validate_success(self):
  • attrs = {
  • 'username': 'test_user',
  • 'password': 'test_password',
  • }
  • self.serializer.backend_method_name = ""
  • with pytest.raises(AuthenticationFailed):
  • self.serializer.validate(attrs)

I think a test to check what happens if a backend_method_name is not set, is required.

— Reply to this email directly, view it on GitHub https://github.com/djangoflow/django-df-auth/pull/21#pullrequestreview-1443403851, or unsubscribe https://github.com/notifications/unsubscribe-auth/A7TM4HUFVZTHJMGEGOGXYMLXH4M6BANCNFSM6AAAAAAX3PASYE . You are receiving this because you authored the thread.Message ID: @.***>