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.

Sign up for GitHub

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

Jump to bottom

Improve testing #688

Merged
merged 15 commits into from
Jun 21, 2023
Merged

Improve testing #688

merged 15 commits into from
Jun 21, 2023

Conversation

kiraware
Copy link
Member

@kiraware kiraware commented Mar 18, 2023 •
edited
Loading

Overview

  • Update override_api_settings so that it support decorator in python
  • Breaks down the tests to be more specific
  • Eliminate the use of with override_api_settings and use @override_api_settings instead
  • Use rest_framework.status instead of literal

Update override_api_settings so that it support decorator in python

This is done by add try and finally block to existing override_api_settings so that it makes it clearer where is the yield hence support decorator

Breaks down the tests to be more specific

  • test_authentication
    • test_get_header
      • test_get_header
      • test_get_header_x_access_token
    • test_get_raw_token
      • test_get_raw_token
      • test_get_raw_token_incorrect_header_keyword
      • test_get_raw_token_multi_header_keyword
    • test_get_validated_token
      • test_get_validated_token
      • test_get_validated_token_reject_unknown_token
      • test_get_validated_token_accept_known_token
  • test_tokens
    • test_for_user
      • test_for_user
      • test_for_user_with_username
  • test_views
    • TestTokenObtainPairView.test_update_last_login
      • test_update_last_login
      • test_update_last_login_updated
    • TestTokenRefreshView.test_update_last_login
      • test_update_last_login
      • test_update_last_login_updated

Eliminate the use of with override_api_settings and use @override_api_settings instead

Multi-test with @override_api_settings decorator Instead of one-test with multiple with override_api_settings context manager

Use rest_framework.status instead of literal

Use HTTP_200_OK instead of 200 and HTTP_401_UNAUTHORIZED instead of 401 in test_integration

kiraware
kiraware commented Mar 18, 2023
View reviewed changes
tests/test_token_blacklist.py

@override_api_settings(BLACKLIST_AFTER_ROTATION=False)
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this really needed since BLACKLIST_AFTER_ROTATION is default to False, maybe for explicitly sake.

@kiraware kiraware marked this pull request as ready for review March 18, 2023 09:39
@kiraware
Copy link
Member Author

Ready for review from maintainer!

kiraware
kiraware commented Mar 18, 2023
View reviewed changes
tests/test_views.py
# verify last_login is updated
with override_api_settings(UPDATE_LAST_LOGIN=True):
reload(serializers)
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if reload(serializers) really needed.

tests/test_views.py
self.assertIsNotNone(user.last_login)
self.assertGreaterEqual(timezone.now(), user.last_login)

reload(serializers)
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if reload(serializers) really needed.

@kiraware
Copy link
Member Author

@abczzz13 @Andrew-Chen-Wang need your review sir.

@kiraware kiraware changed the title Fix testing Improve testing Mar 21, 2023
Andrew-Chen-Wang
Andrew-Chen-Wang approved these changes Mar 21, 2023
View reviewed changes
Copy link
Member

@Andrew-Chen-Wang Andrew-Chen-Wang left a comment

Choose a reason for hiding this comment

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

lgtm

@kiraware
Copy link
Member Author

@Andrew-Chen-Wang Could you please review and merge this PR. Thanks in advance!

Andrew-Chen-Wang
Andrew-Chen-Wang approved these changes Jun 21, 2023
View reviewed changes
Copy link
Member

@Andrew-Chen-Wang Andrew-Chen-Wang left a comment

Choose a reason for hiding this comment

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

Thanks again for this; this makes things cleaner for sure

@Andrew-Chen-Wang Andrew-Chen-Wang merged commit d2cd59d into jazzband:master Jun 21, 2023
@kiraware kiraware deleted the fix-testing branch June 25, 2023 01:26
mahdirahimi1999 pushed a commit to mahdirahimi1999/djangorestframework-simplejwt that referenced this pull request Jun 26, 2023
@kiraware @pre-commit-ci
Improve testing (jazzband#688)
2c6e322 
* Support `override_api_settings` as decorator

* Update test_authentication

* black formatting  test_authentication

* Use drf status instead of literal status

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update test_integration

* Update test_serializers

* Update test_integration

* Update test_token_blacklist

* Update test_tokens

* Update test_views

* add `setUpTestData` to `TestToken`

* fix typo `self` should be `cls`

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Andrew-Chen-Wang Andrew-Chen-Wang Andrew-Chen-Wang approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kiraware @Andrew-Chen-Wang