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
tests: refactor tests to use testutils helper functions #5728
Conversation
f68e5d4
to
250b421
Compare
d1f5665
to
0a118f0
Compare
return func(p balancer.Picker) error { | ||
for i := 0; i < rpcCount; i++ { | ||
if _, err := p.Pick(balancer.PickInfo{}); err == nil || !strings.Contains(err.Error(), want.Error()) { | ||
return fmt.Errorf("got %q, want error to contain %q, ", want, err) |
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.
Mention that the error was returned from Pick
. See go/go-style/decisions#identify-the-function
return fmt.Errorf("Pick() returned error: %v, want: %v", err, want)
We usually use %v
as the formatting directive for error types. If you feel strongly about using %q, then I don't mind. But otherwise, it would be better to stick to %v
.
Also note that you have for the order or got and want wrong in the arguments list passed to fmt.Error
.
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.
thats a good catch. updating the error message and also flipping the args around
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.
thanks for catching that. updating this
As a reactive from a previous PR (#5711) we want to update certain tests to use helper functions from
internal/testutils/balancer.go
RELEASE NOTES: n/a