-
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
fix(NODE-2944): Reintroduce bson-ext support #2823
Changes from 1 commit
c6fa310
22c3522
e62c508
9aa4082
dd1531b
a973508
06f5643
4ed77b3
4090343
aadc3ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
'use strict'; | ||
|
||
const { expect } = require('chai'); | ||
const BSON = require('../../src/bson'); | ||
|
||
describe('BSON Library Import', function () { | ||
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. we discussed organizing the tests here a little differently: using require.resolve instead of require, and splitting the tests into 2 describe blocks - one for bson-ext and one for js-bson, each of which should have one it block for 'should import the correct library' and another it block for 'should correctly roundtrip supported data types'; using helpers and/or a data table structure should help minimize code duplication |
||
it('should import bson-ext if it exists', function () { | ||
try { | ||
require('bson-ext'); | ||
} catch (_) { | ||
this.skip(); | ||
} | ||
expect(BSON.deserialize).to.be.a('function'); | ||
expect(BSON.serialize).to.be.a('function'); | ||
expect(BSON.calculateObjectSize).to.be.a('function'); | ||
// Confirms we are using the bson-ext library | ||
expect(BSON.deserialize.toString()).to.include('[native code]'); | ||
expect(BSON.serialize.toString()).to.include('[native code]'); | ||
expect(BSON.calculateObjectSize.toString()).to.include('[native code]'); | ||
}); | ||
|
||
it('bson-ext should correctly round trip a Long', function () { | ||
try { | ||
require('bson-ext'); | ||
} catch (_) { | ||
this.skip(); | ||
} | ||
|
||
const longValue = BSON.Long.fromNumber(2); | ||
|
||
const roundTrip = BSON.deserialize(BSON.serialize({ longValue })); | ||
|
||
expect(roundTrip).has.property('longValue'); | ||
}); | ||
it('should import js-bson if bson-ext does not exist', function () { | ||
try { | ||
require('bson-ext'); | ||
this.skip(); | ||
// eslint-disable-next-line no-empty | ||
} catch (_) {} | ||
expect(BSON.deserialize).to.be.a('function'); | ||
expect(BSON.serialize).to.be.a('function'); | ||
expect(BSON.calculateObjectSize).to.be.a('function'); | ||
|
||
expect(BSON.deserialize.toString()).to.not.include('[native code]'); | ||
expect(BSON.serialize.toString()).to.not.include('[native code]'); | ||
expect(BSON.calculateObjectSize.toString()).to.not.include('[native code]'); | ||
}); | ||
}); |
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.
we discovered that Serialize and Deserialize options end up being effectively the same here, so this code can be simplified accordingly
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 looked over this too quickly in our discussion, there is a difference and these keys below are the ones we desire from the underlying BSON library:
serialize options:
checkKeys
serializeFunctions
ignoreUndefined
deserialize options:
promoteLongs
promoteBuffers
promoteValues
fieldsAsRaw
I added a typescript test to cover this expectation.