-
Notifications
You must be signed in to change notification settings - Fork 135
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
Fix missing error handling #487
Conversation
Also, thank you for the PR and welcome to the community! |
Codecov Report
@@ Coverage Diff @@
## master #487 +/- ##
=======================================
Coverage 32.70% 32.70%
=======================================
Files 44 44
Lines 3137 3137
=======================================
Hits 1026 1026
Misses 2019 2019
Partials 92 92
Continue to review full report at Codecov.
|
@shubham14bajpai this is the test cmd/tink-worker/internal/worker.go#L264 |
There is no _test file for worker.go Should I add one with a test case for |
Yes please |
I need a little help with that. I haven't written much tests around grpc clients. Can you guide me how to mock the |
@shubham14bajpai I have been really enjoying testify/mock. You can get a idea of the test mindset here |
@shubham14bajpai Could please update the branch and also fix the CI issue? |
Signed-off-by: shubham <[email protected]>
07327f0
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.
While this doesn't have tests, I feel comfortable approving.
Signed-off-by: shubham [email protected]
Description
Why is this needed
The err checked in line 265 is the error from the previous function call and the error from line 264 is not being handled. The fix captures the error at the correct place.
Fixes: #
How Has This Been Tested?
How are existing users impacted? What migration steps/scripts do we need?
Checklist:
I have: