-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Flakes: Setup new fake server if it has gone away #17023
Conversation
And plug unit test goroutine leaks Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Matt Lord <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17023 +/- ##
==========================================
+ Coverage 67.04% 67.14% +0.10%
==========================================
Files 1571 1571
Lines 251677 251846 +169
==========================================
+ Hits 168729 169101 +372
+ Misses 82948 82745 -203 ☔ View full report in Codecov by Sentry. |
FYI, this PR touches the same function #17024 modifies, but in a different way. So we should be good. |
Signed-off-by: Matt Lord <[email protected]>
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.
❤️
Signed-off-by: Matt Lord <[email protected]>
Description
We've had some flakiness in the
go/mysql
package's unit tests, but in particular withTestTLSRequired
:That test is unique in the package in that it calls
Connect
multiple times. So we address that in this PR by adding handling for when the listener goes away for any reason.In working on this, I also realized that these unit tests are all leaking goroutines. Which not only impacts the other tests in this package when they are run together, but impacts all tests that are run after it. So I added leak checker usage and plugged those leaks as well.
Related Issue(s)
Checklist