-
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
Fix test goroutine leaks in ./cmd/collector/app/ #5292
Fix test goroutine leaks in ./cmd/collector/app/ #5292
Conversation
Please make sure all commits are signed. See CONTRIBUTING.md |
0a3c98c
to
cd9261b
Compare
Solves part of jaegertracing#5006. Signed-off-by: Will Sewell <willsewell@monzo.com>
cd9261b
to
2b0c4dd
Compare
Sorry, just force-pushed a signed commit. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5292 +/- ##
==========================================
+ Coverage 95.06% 95.08% +0.01%
==========================================
Files 340 340
Lines 16610 16615 +5
==========================================
+ Hits 15791 15798 +7
+ Misses 630 629 -1
+ Partials 189 188 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Thanks for the approval @pavolloffay (also thanks for hosting the kubecon contribfest - I was there!) Could you add the label that is causing CI to fail please? I don't think I have permission. |
Thanks! |
Which problem is this PR solving?
Solves part of #5006.
Description of the changes
I introduced
testutils.VerifyGoLeaks(m)
and then ensured all leaked goroutines were stopped.How was this change tested?
make lint
andmake test
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test