Test : Create unit tests for User Info

[decon-harsh]

Description

Created Test database test_osp
Added few unit tests for User Info Get & Post, Login & Register routes.

Fixes #78

Type of Change:

• Code
• Quality Assurance
• Documentation

Code/Quality Assurance Only

• This change requires a documentation update (software upgrade on readme file)
• New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Automated tests

Formatted with ./osp-qa-checks

Checklist:

• My PR follows the style guidelines of this project
• I have performed a self-review of my own code or materials
• I have commented on my code or provided relevant documentation, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation

Code/Quality Assurance Only

• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[decon-harsh]

@codesankalp I don't know why my build test is failing it is successfully running on my local

[codesankalp]

@codesankalp I don't know why my build test is failing it is successfully running on my local

Don't worry about the test build. It will fail for all pull requests for now.
@decon-harsh

[decon-harsh]

@codesankalp can you tell me what should have been done to avoid the previous mistake?

I rebased everything which added new commits of other contributers .

Then ran the linters which modified the files so i created a new commit thought of merging it with my previous commit

It got merged with all of them.

[decon-harsh]

@codesankalp nvm i got it i should have reordered commits and then squashed them

[codesankalp]

@decon-harsh Actually you added commits after your commit. This is the previous mistake.

[codesankalp]

Can you write another test for providing the wrong zulip id in the POST request for testing validation error?
I think it must be included in this test pr 🤔

Tested locally:
tests are working fine.
Screenshots:

[decon-harsh]

@codesankalp Umm but what should I compare??

• The username , they might be diff
• The email , zulip has diff email

[codesankalp]

@codesankalp Umm but what should I compare??

• The username , they might be diff
• The email , zulip has diff email

Use the data of test_user for comparing, get and post request.

[decon-harsh]

@codesankalp Umm but what should I compare??

• The username , they might be diff
• The email , zulip has diff email

Use the data of test_user for comparing, get and post request.

We can't, cause the data of test_user and Zulip user isn't the same.
For ex: Test data's full name is Test User 1 Full name whereas, Zulip User id is my personal id and username is Harsh Singh, the email is not the same.

@codesankalp

[codesankalp]

@codesankalp Umm but what should I compare??

• The username , they might be diff
• The email , zulip has diff email

Use the data of test_user for comparing, get and post request.

We can't, cause the data of test_user and Zulip user isn't the same.
For ex: Test data's full name is Test User 1 Full name whereas, Zulip User id is my personal id and username is Harsh Singh, the email is not the same.

@codesankalp

Add assertion for the response you get is not empty list in test_get_user_info_successfully.

[codesankalp]

Suggested some changes @decon-harsh

[devkapilbansal]

@codesankalp these tests should only run when pushed to master, not on pull request. As discussed with @isabelcosta tests related to zulip api will not be done on pull request because it will require API Key. However, we can ensure that these tests passed locally or on the fork of the contributor.

[isabelcosta]

@codesankalp these tests should only run when pushed to master, not on pull request. As discussed with @isabelcosta tests related to zulip api will not be done on pull request because it will require API Key. However, we can ensure that these tests passed locally or on the fork of the contributor.

we should still be able to test zulip functionalities without using a real key 🤔 not sure how though

[devkapilbansal]

@codesankalp these tests should only run when pushed to master, not on pull request. As discussed with @isabelcosta tests related to zulip api will not be done on pull request because it will require API Key. However, we can ensure that these tests passed locally or on the fork of the contributor.

we should still be able to test zulip functionalities without using a real key not sure how though

@isabelcosta We can't fetch the details without passing the key. However, may be we can run these cases only on push if needed. Else you have to create an environment to pass the tokens.

[decon-harsh]

Question: Can we run specific test files on PR? On the terminal, I do python3 manage.py test tests.{filename} or ignore some failures?.

As @devkapilbansal suggested we should run these tests only on push to master, a forked user must have zulip_key to run tests.

cc @codesankalp @isabelcosta

[decon-harsh]

I have done the necessary changes , was waiting for the approach on zulip_tests thought will do it together .

Please Let me know what should i need to do next!

[devkapilbansal]

@decon-harsh you need to mock the function calls of zulip APIs in test cases. Doing so will remove dependency of tests on zulip_api_key.
Cc:- @codesankalp @isabelcosta

[codesankalp]

@decon-harsh Take reference from this: #102 (comment)
And Let me know if you need any help.

[isabelcosta]

@decon-harsh can you please update this branch with develop to make sure we don't see errors on GitHub actions because of zulip key issue :)

[decon-harsh]

@isabelcosta I updated my branch with develop , which created this merge commit (the second one), How should I avoid it ?

by doing git pull --rebase ?

[codesankalp]

@decon-harsh As you have made the merge commit git pull --rebase will show you updated.
To remove merge-commit you can try this:

git reset --soft 129064c0b71b1f34dd64faf9f8b39b6aa80264ce
git add .
git commit
git push -f

This will remove your merge commit and will squash your commits also.

[decon-harsh]

I will definitely try this @codesankalp thank you

[isabelcosta]

I see now the problem 🤦🏾‍♀️ it's because of a recent PR. Will have a think about it!

[codesankalp]

@decon-harsh You can use my implementation of mocking the API: #109

[decon-harsh]

@decon-harsh You can use my implementation of mocking the API: #109

I am afraid i can't use this directly as I have to mock zulip client . For this I think I have to use patch.

For some reasons I had to reinstall Postgres and now I am having some errors! I can't run server.

[codesankalp]

@decon-harsh You can use my implementation of mocking the API: #109

I am afraid i can't use this directly as I have to mock zulip client . For this I think I have to use patch.

For some reasons I had to reinstall Postgres and now I am having some errors! I can't run server.

What did you try @decon-harsh? Using my implementation will work, I have tried it.
But you can use any method of mocking.

[devkapilbansal]

@decon-harsh @codesankalp I would suggest using responses module to patch the responses.

[decon-harsh]

@codesankalp @devkapilbansal help me in understanding me this.

I have to mock the responses of a client . There is no endpoint to mock right? So I can't use responses.add(endpoint,...). There is a function get_self_zulip_id(). So i have to mock it's response. I mean the client.get_profile().

What should i do in this case?

[codesankalp]

@decon-harsh @codesankalp I would suggest using responses module to patch the responses.

Yes, that is the thing I have also used @devkapilbansal.

[codesankalp]

@codesankalp @devkapilbansal help me in understanding me this.

I have to mock the responses of a client . There is no endpoint to mock right? So I can't use responses.add(endpoint,...). There is a function get_self_zulip_id(). So i have to mock it's response. I mean the client.get_profile().

What should i do in this case?

But you have the endpoint for zulip, right?
Just mock it.
URL: http://127.0.0.1:8000/api/zulip_stat/

[codesankalp]

@decon-harsh Any update on this?

[decon-harsh]

@decon-harsh Any update on this?

Sorry to keep you waiting I will do this tommorow.

[codesankalp]

@decon-harsh Any update on this?

Sorry to keep you waiting I will do this tommorow.

@decon-harsh Please update this. Most of the work is done you have to just mock the API.

[codesankalp]

Closing this PR due to inactivity and not responding for giving updates.