checks: wait for goroutine to complete (fix go-test-race failures) #8346
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CheckAlias already had a waitGroup, but the Add() call was happening too late, which was causing a race in tests. The add must happen before the goroutine is started. CheckHTTP did not have a waitGroup, so I added it to match CheckAlias. It looks like a lot of the implementation could be shared, and may not need all of channel, waitgroup and bool, but I will leave that refactor for another time.
|
The remaining failure in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8329 (comment)
CheckAlias already had a WaitGroup, but the Add() call was happening too late, which was causing a race in tests. The add must happen before the goroutine is started.
CheckHTTP did not have a WaitGroup, so I added it to match CheckAlias.
It looks like
CheckTCP,CheckMonitor,CheckDocker,CheckGRPC, andCheckTTLall may have the same problem. I'm not sure why it seems that only alias and http are failing. I saw one failure for GRPC, so maybe alias and http happen to have more tests written for them?If this fix looks good, I will look at refactoring so that all of these can share the "run" mechanism and remove the need to copy the fix 7 times.