-
Notifications
You must be signed in to change notification settings - Fork 245
test(DRIVERS-3034): remove/update 4.0 related tests #1796
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,7 @@ phases: [ | |
# servers from a primary isWritablePrimary are added to the working server set | ||
me: "a:27017", | ||
minWireVersion: 0, | ||
maxWireVersion: 7 | ||
maxWireVersion: 25 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These I just updated to latest to not need to continually update them. |
||
}] | ||
], | ||
outcome: { | ||
|
@@ -53,7 +53,7 @@ phases: [ | |
primary: "localhost:27017", | ||
me: "localhost:27018", | ||
minWireVersion: 0, | ||
maxWireVersion: 7 | ||
maxWireVersion: 25 | ||
}] | ||
], | ||
outcome: { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Wire version 16 is server 5.3
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.
The Java and C drivers are able to pass these tests still, I think because it's only in server selection where the maxWireVersion is checked. Do you expect other drivers to fail these tests, and if so, what causes the failure?
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.
My expectation is that no drivers would fail these tests currently as they are for pre 6.0 and this sets the max wire version for 5.3. If the drivers remove support for 4.0, I expect these tests to fail without this change on drivers that do wire version checks outside of server selection as the maxWireVersion would then be lower than the minWireVersion. (For example, Node does do this check outside of server selection in https://github.com/mongodb/node-mongodb-native/blob/fcbc6edfc3d54f5dbd3b142c06ef205ecd2729b8/src/cmap/connect.ts#L60)
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.
OK, this seems like a harmless change to make, even for drivers using submodules. There is no loss of test coverage, and the tests should pass all drivers.
But why stop at 5.3? Any reason not to just bump it up the MongoDB 8.0 wire version?
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.
Oh, as you explained in Slack, this is for pre-6.0