Skip to content

Auth service tests#31

Open
dikwickley wants to merge 2 commits into
ceph:mainfrom
dikwickley:auth-service-tests
Open

Auth service tests#31
dikwickley wants to merge 2 commits into
ceph:mainfrom
dikwickley:auth-service-tests

Conversation

@dikwickley

Copy link
Copy Markdown

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

Aniket Singh Rawat added 2 commits November 4, 2023 12:11
Signed-off-by: Aniket Singh Rawat <sprx@gmail.com>
Signed-off-by: Aniket Singh Rawat <sprx@gmail.com>
@dikwickley

Copy link
Copy Markdown
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
Copy Markdown
Author

hi @VallariAg can you review this?

@VallariAg

Copy link
Copy Markdown
Member

@dikwickley please refer my comments above

@dikwickley

Copy link
Copy Markdown
Author

@VallariAg i can't see your comments

@VallariAg

VallariAg commented Nov 17, 2023

Copy link
Copy Markdown
Member

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

2 participants