-
Notifications
You must be signed in to change notification settings - Fork 238
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
chore: update tests to pass the race detector and linter #130
Conversation
Should allow them to pass with the race detector.
Let's see if I remember how to yaml
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!
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 | ||
- uses: actions/setup-go@6edd4406fa81c3da01a34fa6f6343087c207a568 # v3.5.0 | ||
with: | ||
go-version: '1.20' |
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.
Not a more recent version?
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 copied and pasted raft's as a basis assuming they're the best maintained shared library. 😬 We can bump it. I'm not really sure what the optimal strategy is for a little library like this with no deps and no-post-1.14 features afaik.
Co-authored-by: Michael Schurter <michael.schurter@gmail.com>
It now passes:
golangci-lint run ./...
go test -race ./... -short
The
-short
is needed because both ofTestSendData_VeryLarge
andTestSendData_Large
fail for timing reasons (since race mode is generally slower) and I was hesitant to rewrite them to be less timing sensitive, so I have them get skipped when in testing short mode for now.