-
Notifications
You must be signed in to change notification settings - Fork 1.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
Extend bufferpool test #340
Conversation
Summary: Adds on to the bufferpool test to write random bytes into the buffers before putting them back into the sync.Pool
@willhug, thanks for your PR! By analyzing the history of the files in this pull request, we identified @akshayjshah and @prashantv to be potential reviewers. |
@@ -36,10 +37,23 @@ func TestBuffers(t *testing.T) { | |||
buf := Get() | |||
assert.Zero(t, buf.Len(), "Expected truncated buffer") | |||
assert.NotZero(t, buf.Cap(), "Expected non-zero capacity") | |||
|
|||
b := getRandBytes() |
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.
do we really need random data? maybe just write a constant to simplify this
const dummyData = "dummy data"
buf.WriteString(dummyData)
assert.Equal(t, buf.Len(), len(dummyData), "...")
|
||
func getRandBytes() []byte { | ||
b := make([]byte, rand.Intn(5000)) | ||
rand.Read(b) |
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.
technically read can return an error that is being ignored here. might be cleaner to not use this at all
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.
Please fix the last remaining nit; otherwise lgtm.
"sync" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
const dummyData = "dummy data" |
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.
Please underscore-prefix globals.
@@ -27,6 +27,8 @@ import ( | |||
"github.com/stretchr/testify/assert" | |||
) | |||
|
|||
const dummyData = "dummy data" |
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'd move the const into the test since it's not used anywhere else
Summary: Adds on to the bufferpool test to write random bytes into
the buffers before putting them back into the sync.Pool