Skip to content

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Mar 8, 2021

Completes the unified runner by passing all the poc spec tests.

NODE-2287

Notes

There's some additional work in here that I'm happy to break out into its own ticket, but it came up in debugging:

  • Sessions claimed to accept MongoClientOptions when they were instead receiving MongoOptions, guaranteed to have the well typed WC,RP,RC etc.
  • The issue blocking transactions from working was that my test provided an options object that had { writeConcern: undefined } and that would overwrite the client options that were supposed to be inherited. I can go back and fix the test to not defined a key if the value is undefined but maybe this is unexpected enough to fix in driver, so here I used ?? and explicit fetching of the options to resolve the correct value
  • APM overwrites the client.s.options which put client[kOptions] and client.options out of sync. There is also an opportunity for the client.s.WC/RP/RC values to be out of sync with client[kOptions]. Changing these to getters gives us one source of truth for client options.

@nbbeeken nbbeeken force-pushed the NODE-2287/UnifiedTestFormat branch 2 times, most recently from ebf1ed9 to c2b2b59 Compare March 8, 2021 23:05
@nbbeeken nbbeeken requested review from emadum and durran March 10, 2021 17:57
@nbbeeken nbbeeken marked this pull request as ready for review March 10, 2021 17:57
Copy link
Contributor

@emadum emadum left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

LGTM

Completes the unified runner by passing all the poc spec tests.

NODE-2287
@nbbeeken nbbeeken force-pushed the NODE-2287/UnifiedTestFormat branch from b7e1d05 to dd4bc07 Compare March 23, 2021 14:09
@nbbeeken nbbeeken merged commit b5f19ef into 4.0 Mar 23, 2021
@nbbeeken nbbeeken deleted the NODE-2287/UnifiedTestFormat branch March 23, 2021 14:25
ljhaywar pushed a commit that referenced this pull request Nov 9, 2021
Completes the unified runner by passing all the poc spec tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants