-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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: add goleak to check goroutine leak in tests #5010
Conversation
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
@yurishkuro, i have added goleak to some packages (that do not fail) and added a linter to verify goleak is present in tests. Could you please give me some pointers on whether I'm on the right track? |
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.
Thanks, looks good as a direction.
Let's fix the comments but not add any more packages, I prefer to merge smaller PRs. |
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
I simplified the script and moved In the previous run some of the packages failed on goleak, so you need to remove them from this PR. |
Signed-off-by: Harshvir Potpose <[email protected]>
merge rmote in local
we could also add goleak into all empty_test.go files, right now they do come up in the output |
there are still some packages that do not fail, should i also add them |
let's do that in future PRs |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5010 +/- ##
==========================================
+ Coverage 95.61% 95.62% +0.01%
==========================================
Files 319 319
Lines 18786 18786
==========================================
+ Hits 17963 17965 +2
+ Misses 661 659 -2
Partials 162 162
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test