-
Notifications
You must be signed in to change notification settings - Fork 911
GODRIVER-2310 Add snapshot query examples #887
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
GODRIVER-2310 Add snapshot query examples #887
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.
Looking great! Just a few comments; I can help debug any remaining failing tests offline.
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 updates using mtest
!
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.
Some suggestions, but looks good (pending the one non-optional change to use options.Session()
) 👍
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.
Nicely done. The additional assertions outside of the example comments will help catch bugs. Wrapping the example functions in separate t.Run
calls helps to run the individual tests locally.
Suggested a change that may resolve the test failure in StableAPIStrictCountExample
.
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.
Looks great! Just a suggestion to remove Auth(false)
for StableAPIStrictCountExample
.
_, err := catColl.InsertMany(context.Background(), []interface{}{ | ||
bson.D{ | ||
{"adoptable", false}, | ||
{"name", "Miyagi"}, |
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.
Is one of these the name of your cat haha
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.
Miyagi is the name the shelter gave my cat, but I call him Mo :)
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!
GODRIVER-2310
TestDocumentationExamples
into subtests per POC subtestWrapper #885mtest
usage indocumentation_examples_test
package