-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(NODE-4078): allow comment with estimated doc count #3301
Conversation
@@ -0,0 +1,31 @@ | |||
import { expect } from 'chai'; | |||
|
|||
describe('Collection', () => { |
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.
If we're going to add an integration test for this, I think we should add it with the others. Adding tests for all falsy values should be as easy as adding ['estimatedDocumentCount', { } ]
to the array of commands here:
node-mongodb-native/test/integration/node-specific/comment_with_falsy_values.test.ts
Line 27 in fe7c102
['findOneAndReplace', { filter: { _id: 1 }, replacement: { x: 12 } }] as const |
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.
I've moved over to that 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.
requesting changes for visibility
test/integration/node-specific/comment_with_falsy_values.test.ts
Outdated
Show resolved
Hide resolved
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.
Co-authored-by: Bailey Pearson <bailey.pearson@mongodb.com>
@baileympearson I committed your suggestion. Just waiting on evg run now. |
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.
failing tests are unrelated to these changes. LGTM 😄
Description
Allows comment to be provided with estimated document count.
What is changing?
Sets the comment on the command and adds the new unified tests.
Is there new documentation needed for these changes?
None
What is the motivation for this change?
NODE-4078
Double check the following
npm run check:lint
script<type>(NODE-xxxx)<!>: <description>