fix(NODE-3183,NODE-3249): bring versioned API impl up to date #2814
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
Bring the versioned API code up to date by addressing NODE-3183 and NODE-3249. Specifically: - Bump the versioned API tests to the latest version using `./etc/update-spec-tests.sh versioned-api`. - Manually apply the SDAM updates from mongodb/specifications@9946950 (because a full update would have also come with load balancer changes that do not pass yet). - Update the test runner slightly to account for the fact that some fields may have only a `$$unsetOrMatches` operator as its contents and thus the set of expected keys in an “expected” object is not a fixed-size set. - Remove the special check for getMore and transaction commands, and now pass the apiVersion for them as well when it is specified. - Use the `hello` command instead of `isMaster` when an API version is specified; make sure that the slightly different response format does not break anything by setting the `ismaster` property manually in that case. - Updating the mock server message handlers to account for both types of handshakes, everywhere.
mbroadst
reviewed
May 21, 2021
dariakp
requested review from
a team,
emadum,
nbbeeken and
dariakp
and removed request for
a team
nbbeeken
reviewed
May 24, 2021
There was a problem hiding this comment.
There seems to be a failure still, it is the same test it just happens to be one of the duplicated tests meant to test the unified test runner.
A successful find event with a getmore and the server kills the cursor in test/spec/unified-test-format/valid-pass/poc-command-monitoring.yml
I tried debugging a bit and it appears the server isn't killing the cursor (which I believe is returning a 0 cursor id) so this line is kind of the source of the error. I'll check with other drivers if this test is an issue for all of us.
# for free
to join this conversation on GitHub.
Already have an account?
# 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 changed?
Bring the versioned API code up to date by addressing NODE-3183 and
NODE-3249.
Specifically:
./etc/update-spec-tests.sh versioned-api.mongodb/specifications@9946950
(because a full update would have also come with load balancer
changes that do not pass yet).
fields may have only a
$$unsetOrMatchesoperator as its contentsand thus the set of expected keys in an “expected” object is
not a fixed-size set. (This was necessary in order for the updated
versioned-api tests to pass)
now pass the apiVersion for them as well when it is specified.
hellocommand instead ofisMasterwhen an API version isspecified; make sure that the slightly different response format
does not break anything by setting the
ismasterproperty manuallyin that case.
of handshakes, everywhere.
Are there any files to ignore?
I mean, the spec tests were really just updated automatically. The mock tests updates were performed semi-automatically, i.e. I did an automatic replacement and then went through all update sites + all message handlers manually to verify that they were not invalid changes. (Some of the tests are using replies like
mock.DEFAULT_ISMASTER_36. I’m not sure if it makes sense to actually update those, considering that the name implies a MongoDB 3.6 response. I don’t think it hurts, though.)