#156687691 Fix Accounts Tests #8
Conversation
ghost
requested review from
jimmykamau,
alexmochu,
Collins33,
Bonifase and
zenjutahi
Pull Request Test Coverage Report for Build 147
💛 - Coveralls |
| def test_it_redirects_after_login_with_a_bad_token(self): | ||
| # Login with a bad token | ||
| url = "/accounts/check_token/alice/invalid-token/" | ||
| r = self.client.post(url, follow=True) |
There was a problem hiding this comment.
You should try logging in with an incorrect token, not call the incorrect token URL
There was a problem hiding this comment.
I disagree with you @jimmykamau . The healthchecks login route at /accounts/login/ accepts a form with input email address and password to log in a user with this email address. The login function at hc/accounts/views.py takes this form bundled in a request param. There is no method to login a user using a token other than the check_token(request, username, token) at hc/accounts/views.py. The GET route /accounts/check-token/<username>/<token>/ is the URL that will access this function.
There was a problem hiding this comment.
Have you tried calling the function with an incorrect token?
There was a problem hiding this comment.
Yeah, I it says I provide the request param which I dont have
There was a problem hiding this comment.
I'm forced to use the GET route to access this function
hc/accounts/tests/test_login.py
Outdated
| ### Assert that check is associated with the new user | ||
| # Assert that check is associated with the new user | ||
| get_check = Check.objects.get(code=check.code) | ||
| assert get_check.user |
There was a problem hiding this comment.
This doesn't actually check against the new User, just determines that there's a user
There was a problem hiding this comment.
I refactored this. I am now checking whether the user object associated with the check is the actual object created. I had to compare the whole objects, I couldn't find methods to retrieve user attributes in the User model. But I don't think this is a very efficient way @jimmykamau
hc/accounts/tests/test_profile.py
Outdated
| @@ -17,17 +17,26 @@ def test_it_sends_set_password_link(self): | |||
| # profile.token should be set now | |||
| self.alice.profile.refresh_from_db() | |||
| token = self.alice.profile.token | |||
| ### Assert that the token is set | |||
| # Assert that the token is set | |||
| self.assertTrue(len(token) > 10) | |||
There was a problem hiding this comment.
I don't think this is the best way of checking this, what happens when we change the length of the token?
There was a problem hiding this comment.
Thanks, I am now checking whether the token exists
|
Thank you for the observations Jimmy, I'll make some changes and update the PR. I'll notify you once I am done |
ghost
changed the title
What does this PR do?
Fixes and completes tests in the following modules under
accounts/tests/:Description of Task to be completed?
Fix accounts tests
How should this be manually tested?
python manage.py testpytest testsinaccounts/folderWhat are the relevant pivotal tracker stories?