test(NODE-6438): check that BSON undefined is returned from deserialize #721
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
baileympearson
requested changes
Oct 21, 2024
test/node/bson_undefined.test.ts
Outdated
| describe('when serialize is given a javascript object that contains undefined', () => { | ||
| describe('when ignoreUndefined is set to false', function () { | ||
| it('serializes to document with a set to BSON null (type=10)', () => { | ||
| const jsObject = BSON.deserialize(bsonDocWithUndefined); |
There was a problem hiding this comment.
Suggested change
| const jsObject = BSON.deserialize(bsonDocWithUndefined); | |
| const jsObject = { a: undefined }; |
Is there a reason we can't use an object literal?
test/node/bson_undefined.test.ts
Outdated
| const jsObject = BSON.deserialize(bsonDocWithUndefined); | ||
| const bytes = BSON.serialize(jsObject, { ignoreUndefined: false }); | ||
| expect(bytes).to.have.lengthOf(8); | ||
| expect(bytes).to.have.own.property('4', BSON_DATA_NULL); |
There was a problem hiding this comment.
What is happening in this assertion? Does haveOwnProperty check for index access if the object is a Buffer?
Could we use parseToElementsToArray to re-parse the serialized bytes and confirm the serialized bytes contain one BSON null?
There was a problem hiding this comment.
good use of parseToElements
baileympearson
added
the
Primary Review
baileympearson
approved these changes
Oct 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
What is changing?
Is there new documentation needed for these changes?
Yes
What is the motivation for this change?
I noticed no test fails if I change
BSON_DATA_UNDEFINEDhandling, seems worthy of coverage given how the serializer special cases that same value.Double check the following
npm run check:lintscripttype(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript