-
Notifications
You must be signed in to change notification settings - Fork 143
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
Enable and fix race detector for CI #3096
Conversation
The race detector is now supported on arm64 and aarch64.
Pinging @elastic/elastic-agent (Team:Elastic-Agent) |
🌐 Coverage report
|
buildkite test this |
case <-time.After(200 * time.Microsecond): | ||
// we're not expecting any reset, | ||
case <-calledCh: | ||
// Handle was called, expected |
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.
Nice replacement of assert.Eventually
.
// we're not expecting any reset, | ||
case <-calledCh: | ||
// Handle was called, expected | ||
case <-time.After(1 * time.Second): |
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.
Nit: You could probably make this even shorter, given that the assert.Eventually
before had a timeout of 100*time.Millisecond
?
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.
I just like to keep it longer because it returns as soon as it happens. This is just to be sure that in the case that runner is slow that we don't hit that timeout just because of that.
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.
You okay with me leaving it as is?
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.
Yes, OK to leave as-is.
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.
LGTM.
* Stop ignoring RACE_DETECTOR when not on amd64. The race detector is now supported on arm64 and aarch64. * Enable the race detector in CI. * Remove pkg/errors from gotest.go. * Fix unchecked error return value. * Fix tests. --------- Co-authored-by: Craig MacKenzie <[email protected]> Co-authored-by: Fae Charlton <[email protected]> (cherry picked from commit f31ceaa)
* Stop ignoring RACE_DETECTOR when not on amd64. The race detector is now supported on arm64 and aarch64. * Enable the race detector in CI. * Remove pkg/errors from gotest.go. * Fix unchecked error return value. * Fix tests. --------- Co-authored-by: Craig MacKenzie <[email protected]> Co-authored-by: Fae Charlton <[email protected]> (cherry picked from commit f31ceaa) Co-authored-by: Blake Rouse <[email protected]>
What does this PR do?
Enables the race detector for the CI unit tests including fixing the issues detected by enabling the race detector.
Why is it important?
Ensures that we do not have any data race conditions in the code.
Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files[ ] I have added an entry in./changelog/fragments
using the changelog tool[ ] I have added an integration test or an E2E test