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

Auth service tests #31

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

dikwickley
Copy link

This PR aims to add tests for the auth service. WIP for #5

  • Separate out auth service layer from Routes
  • Write test cases for auth service

Signed-off-by: Aniket Singh Rawat <sprx@gmail.com>
Signed-off-by: Aniket Singh Rawat <sprx@gmail.com>
@dikwickley
Copy link
Author

@VallariAg I have separated out the service layer in /login route.
Now the GH login part can be mocked easily by replacing the AuthServiceGH with AuthServiceMock during testing.
We can use this to replace it: https://fastapi.tiangolo.com/advanced/testing-dependencies/

If this looks good I can proceed with writing the tests.

@dikwickley
Copy link
Author

hi @VallariAg can you review this?

@VallariAg
Copy link
Member

@dikwickley please refer my comments above

@dikwickley
Copy link
Author

@VallariAg i can't see your comments

@VallariAg
Copy link
Member

VallariAg commented Nov 17, 2023

@dikwickley oh, that's weird! I added a comment to that thread, I wonder if you are able to see that?

But here's what my comments said for AuthServiceMock class:

This class is used only for testing so it should be defined somewhere under /tests directory
You can probably create similar dir structure in /tests as it is in /src.
Also to mock functions, you can look into unittest.mock functions and pytest fixtures
https://docs.python.org/3/library/unittest.mock.html

edit: my bad, I didn't realise that if I don't "submit" a review then the comments would only be visible to me!

)
return response_org_dict

class AuthServiceMock(AuthService):
Copy link
Member

Choose a reason for hiding this comment

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

This class is used only for testing so it should be defined somewhere under /tests directory
You can probably create similar dir structure in /tests as it is in /src.

Copy link
Member

Choose a reason for hiding this comment

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

Also to mock functions, you can look into unittest.mock functions and pytest fixtures
https://docs.python.org/3/library/unittest.mock.html

Copy link
Member

Choose a reason for hiding this comment

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

@dikwickley ping!

Copy link
Author

Choose a reason for hiding this comment

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

Oh you added these 2 weeks ago, I am only able to see them now. I'll make these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants