Skip to content

Conversation

benjirewis
Copy link
Contributor

@benjirewis benjirewis commented May 17, 2022

GODRIVER-1986
GODRIVER-2362
GODRIVER-2296

Converts the change streams spec tests to the unified test format. Removes the legacy change stream spec test runner and associated functions. Adds tests to ensure the "to" field is present in rename events. Adds tests to ensure we do not error when parsing change stream event documents. Bumps supported unified test format schema version to 1.7. Adds support for ignoreExtraEvents and the rename operation in the unified test runner.

Copy link
Member

@prestonvasquez prestonvasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@benjirewis benjirewis requested a review from prestonvasquez May 20, 2022 16:57
Copy link
Collaborator

@qingyang-hu qingyang-hu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent job! Thank you!

@@ -454,7 +475,14 @@ func executeEstimatedDocumentCount(ctx context.Context, operation *operation) (*
}

opts := options.EstimatedDocumentCount()
elems, _ := operation.Arguments.Elements()
var elems []bson.RawElement
// Some estimatedDocumentCount operations in the unified test format have no arguments.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the estimatedDocumentCount operations in unified/estimatedDocumentCount do not have an arguments field, so I've added a special conditional here. I could add this to all execute* functions, but it should be easy to add later on if we encounter other operations with no arguments.

I checked to see who wrote these tests and decided to recklessly not include the arguments field, and it was me a year ago 😢 .

@benjirewis benjirewis merged commit 6ff1f5c into mongodb:master May 20, 2022
@benjirewis benjirewis deleted the convertChangeStreamsUnified branch May 20, 2022 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants