Skip to content
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

chore(ci): xunit integration [3.5] #2641

Merged
merged 8 commits into from Nov 30, 2020
Merged

chore(ci): xunit integration [3.5] #2641

merged 8 commits into from Nov 30, 2020

Conversation

emadum
Copy link
Contributor

@emadum emadum commented Nov 29, 2020

Backports of #2596 and #2612 for 3.5

Note: also fixes a failing test on the latest server to get CI passing, related to setting collation on listCollections.

It seems we set collation a bit too aggressively and rely on the server ignoring it where it's not supported. However, the latest server version is returning an error:

FAILURE: BSON field 'listCollections.collation' is an unknown field. ()
 MongoError: BSON field 'listCollections.collation' is an unknown field.
     at Connection.<anonymous> (lib/core/connection/pool.js:451:61)
     at processMessage (lib/core/connection/connection.js:384:10)
     at Socket.<anonymous> (lib/core/connection/connection.js:553:15)
     at addChunk (_stream_readable.js:288:12)
     at readableAddChunk (_stream_readable.js:269:11)
     at Socket.Readable.push (_stream_readable.js:224:10)
     at TCP.onStreamRead [as onread] (internal/stream_base_commons.js:94:17)

@emadum emadum changed the title chore(ci): xunit integration chore(ci): xunit integration [3.5] Nov 29, 2020
@emadum emadum marked this pull request as ready for review November 29, 2020 15:53
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice to have this on all the branches! tyty!

Comment on lines 112 to 116
function commandSupportsCollation(command) {
if (command.listCollections) return false;
return true;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this sets a bit of a dangerous precedent, can we move this to the command construction of the listCollections operation? Ideally we would not ever depend on inspecting keys of command documents, hopefully we've designed the driver in such a way that we can make these determinations at the point of command construction, rather than at the wire protocol level.

Comment on lines 63 to 65
if (test.commandName === 'geoSearch') {
metadata.requires.mongodb += ' <=4.4';
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a data-driven test, shouldn't the instructions for building the tests should be in the specs above rather than special cased here? Maybe something like serverVersion: '<=4.4'?

Copy link
Member

@mbroadst mbroadst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@emadum emadum merged commit 06485ea into 3.5 Nov 30, 2020
@emadum emadum deleted the xunit-integration branch November 30, 2020 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants