-
Notifications
You must be signed in to change notification settings - Fork 244
DRIVERS-2232: remove usages of currentOp
and collStats
from tests
#1402
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
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.
Test changes LGTM and am fine with the reasoning for skipping the currentOp test.
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.
An alternative solution would be to choose a max server version (say, 6.99.0) and prohibit it from running on any future server versions.
That's what we usually did, but feel free to do this in a separate PR. IMO, there's no advantage to wait until the first driver breaks, so we may as well do this already.
collStats
command usage to $collStats
currentOp
and collStats
from tests
@@ -26,17 +26,18 @@ tests: | |||
command: | |||
ping: 1 | |||
command_name: ping | |||
- description: "current op is not bypassed" | |||
- description: "kill op is not bypassed" |
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 intent of this test seems to test that bypassed commands are passed (it tests with ping
above) and then this test asserts that a command that isn't encrypted or bypassed errors.
Replacing currentOp
with killOp
(another command that is neither bypassed or encrypted) tests the same scenario.
@alcaeus I decided to address the |
Node POC: mongodb/node-mongodb-native#3638
This PR removes usage of the
collStats
command andcurrentOp
from tests.Please complete the following before merging: