-
Notifications
You must be signed in to change notification settings - Fork 70
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
feature/user_signup_6 #9
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 more change requested, apart from that, can we also add a test for the code that was added?
should test
- adding users
- adding 2 users with same email and the error that happens
- sending unparseable json in the request body
there is an example test in the test folder, can just expand on that.
Feature/user signup
@championswimmer Locally also I replicated the timeout error from tests. I increased the timeout duration to 1.5 seconds and the tests passed with duration of 1.1 seconds. Is there something wrong with the way DB ops are being done or is it just that test file DB (in-memory) I/O is causing some issue? |
The tests seem to be passing on GH Actions (on the forked repo) also now within the timeout duration. Not sure. Too tight timeout duration? |
We can increase the timeout a bit if we want to, sure. |
Welcome to Codecov 🎉Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment. Thanks for integrating Codecov - We've got you covered ☂️ |
Added a couple of tests to increase coverage. A few more tests can be added to increase but that will require mocking the DB. @championswimmer Do we need that? Although, I think coverage criteria should pass without that. |
No description provided.