-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
test(pubsub): dedupe by message data in ordering keys json test #8526
test(pubsub): dedupe by message data in ordering keys json test #8526
Conversation
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.
Just one nit. Approving as admin to unblock failed test resolution.
pubsub/integration_test.go
Outdated
_, err := r.Get(ctx) | ||
if err != nil { | ||
// Can't fail inside goroutine, so just log the error | ||
log.Printf("publish error for message(%s): %v", msg, 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.
Might be worth just using the test logger:
log.Printf("publish error for message(%s): %v", msg, err) | |
t.Logf("publish error for message(%s): %v", msg, err) |
pubsub/integration_test.go
Outdated
"io/ioutil" | ||
"log" |
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.
"io/ioutil" | |
"log" | |
"io/ioutil" |
Goes with the other change suggested, but feel free to do them in your own single commit
Closes #8520
See comments in above issue for how this PR fixes this test