chore: expand inspect support of BSON types NODE-2875

[rose-m]

Description

Adds support for the NodeJS inspect function to more BSON data types. This is an upstream change proposed by the mongosh team.

A custom inspect function is added for the following data types:

• Binary
• Code
• DBRef
• Decimal128
• Int32
• Long
• MinKey
• MaxKey
• BSONSymbol
• Timestamp

[addaleax]

@rose-m These are also shell types, so I think at least this one would probably be best changed as well (and, because the different names aren’t enough confusing, the arguments work pretty differently … BinData() in the shell takes subtype + base64 string in that order, Binary in BSON takes a “““binary””” string or buffer + subtype, i.e. in reverse order)… I think ideally this line could be something like

Suggested change

	return `BinData(${this.sub_type}, "${asBuffer.toString('base64')}")`;
	return `Binary(Buffer.from("${asBuffer.toString('base64')}", "base64"), ${this.sub_type})`;

but it sure isn’t pretty…

(edit: if you want to know why I triple-quoted “binary”: https://twitter.com/addaleax/status/1189711048682680320)

[mbroadst]

Really just one small nit, but otherwise LGTM!

[mbroadst]

Suggested change

	describe('MinKey tests', function () {
	describe('BSONSymbol tests', function () {

[mbroadst]

oops, missed this one. Actually, now that we're here does it make sense to just add all of these tests in bson_tests instead? I'm imagining a future maintainer forgetting to change one of these in the future because they have to change ~10 files

[rose-m]

Yeah I wasn't sure how to handle that and then went with the individual files. I could implement a test case like https://github.com/mongodb/js-bson/blob/master/test/node/bson_test.js#L1310 which would inspect an object with all the different types and directly compare that instead of individual test cases?

[mbroadst]

I like the idea of checking the behavior in one place, it looks like maybe there's a build failure related to formatting of the string. Maybe its easier to test each type in isolation, rather than inspecting an object with each type? We've had spurious build failures in the past due to minor fluctuations in formatting.

[rose-m]

Ah alright now I see - sometimes it has line breaks, sometimes it doesn't. Will fix asap and split it up into individual cases to circumvent that.

[rose-m]

I've now added individual cases per type in one block in bson_tests - let's hope these now work in evergreen 😄

[emadum]

LGTM 👍

[nbbeeken]

Thanks for contributing this!
A few patches to the tests and it looks good to me

[mbroadst]

LGTM!