feat(NODE-7335): Create dedicated mocha runner with isolated vm context#4876
feat(NODE-7335): Create dedicated mocha runner with isolated vm context#4876PavelSafronov wants to merge 47 commits intomongodb:mainfrom
Conversation
…o the bundled export file
…thub.com/PavelSafronov/node-mongodb-native into NODE-7335-bundle-and-barrel-approach-poc
…nd export a few more relevant types
…thub.com/PavelSafronov/node-mongodb-native into NODE-7335-bundle-and-barrel-approach-poc
- removed unnecessary test checks - added more export types - made suggested fixes - added comments to tests being skipped
nbbeeken
left a comment
There was a problem hiding this comment.
There's the one comment about global the legacy Node.js-only property, if there's something still relying on it we can revisit that in a follow up, if not we can just drop it.
| 'Map', | ||
| 'Math', | ||
| 'Promise', |
There was a problem hiding this comment.
| 'Map', | |
| 'Math', | |
| 'Promise', |
I'd still really look into why these are required before merging (if they indeed are), they're all JS builtins that should work just fine, and if they do not, it's going to be worth checking why that is
There was a problem hiding this comment.
These are related to NODE-7460: our tests/chai uses instanceof calls. We'll be able to remove these additions after we upgrade to chai 5, which uses a different instance check that is amenable to different contexts.
src/runtime_adapters.ts
Outdated
|
|
||
| return { | ||
| os: options.runtimeAdapters?.os ?? require('os') | ||
| os: options.runtimeAdapters?.os ?? correctRequire('os') |
There was a problem hiding this comment.
I think #4876 (comment) is still relevant here (this call isn't bundler-friendly by itself – we may be fine with that, but it's a change that at the very least requires config changes in downstream packages)
.evergreen/config.yml
Outdated
| tasks: | ||
| - test-latest-server | ||
| - test-latest-replica_set | ||
| - test-latest-sharded_cluster | ||
| - test-rapid-server | ||
| - test-rapid-replica_set | ||
| - test-rapid-sharded_cluster | ||
| - test-8.0-server | ||
| - test-8.0-replica_set | ||
| - test-8.0-sharded_cluster | ||
| - test-7.0-server | ||
| - test-7.0-replica_set | ||
| - test-7.0-sharded_cluster | ||
| - test-6.0-server | ||
| - test-6.0-replica_set | ||
| - test-6.0-sharded_cluster | ||
| - test-5.0-server | ||
| - test-5.0-replica_set | ||
| - test-5.0-sharded_cluster | ||
| - test-4.4-server | ||
| - test-4.4-replica_set | ||
| - test-4.4-sharded_cluster | ||
| - test-4.2-server | ||
| - test-4.2-replica_set | ||
| - test-4.2-sharded_cluster | ||
| - test-latest-server-v1-api | ||
| - test-x509-authentication | ||
| - test-atlas-connectivity | ||
| - test-5.0-load-balanced | ||
| - test-6.0-load-balanced | ||
| - test-7.0-load-balanced | ||
| - test-8.0-load-balanced | ||
| - test-rapid-load-balanced | ||
| - test-latest-load-balanced | ||
| - test-auth-kerberos | ||
| - test-auth-ldap | ||
| - test-socks5-csfle | ||
| - test-socks5-tls | ||
| - test-snappy-compression | ||
| - test-zstd-compression | ||
| - test-tls-support-latest | ||
| - test-tls-support-8.0 | ||
| - test-tls-support-7.0 | ||
| - test-tls-support-6.0 | ||
| - test-tls-support-5.0 | ||
| - test-tls-support-4.4 | ||
| - test-tls-support-4.2 |
There was a problem hiding this comment.
That's quite a list! Do we really need to test different mongodb versions here? The assumption is that different topologies might have different path branches we want to catch, but I think having either rapid or latest stable might be enough for the matrix, wdyt?
There was a problem hiding this comment.
This was a first-pass addition of the node-less variant, so I may have been overenthusiastic with the list. Let me take a look at .evergreen/generate_evergreen_tasks.js and see if we can specify which tasks should run.
There was a problem hiding this comment.
Current approach tosses everything in, which can cause issues: more tests means more chance for flaky failures, more resources spent, etc. I'm going to pare this list down to a hardcoded set of tasks that we will run in rhel8 and Windows. Updating now.
| _Note that in cases where the tests need to run longer than one hour to ensure that tokens expire | ||
| that the mocha timeout must be increased in order for the test not to timeout._ | ||
|
|
||
| ### Node-less Runtime Testing |
There was a problem hiding this comment.
Fantastic work on documenting this new testing approach! 👍
…thub.com/PavelSafronov/node-mongodb-native into NODE-7335-bundle-and-barrel-approach-poc
Description
Summary of Changes
Adds a way to run unit and integration tests against a special version of Node Driver where we block specific
requirecalls. This is so we can be confident that our updates don't accidentally re-introduce a dependence on Node.This PR also adds a number of Evergreen tasks for testing various combinations of the server with the "Node-less" VM. In these tasks we run all integ tests and most unit tests against the bundled and contextified version of the driver.
Notes for Reviewers
Take a look at the README updates, that should explain a lot about how this bundled testing works.
Below are steps for testing this out locally with timeout unit tests and CRUD API integ tests.
Steps to test unit tests
src/timeout.tsaround line 62, in the private constructornpm run check:unitnpm run check:unit-bundledSteps to test unit tests
src/mongo_client.ts, around line 430, in the constructornpm run check:testnpm run check:test-bundledWhat is the motivation for this change?
To fail our tests if our code starts to use a prohibited import, so we can guarantee that the driver will work in a non-Node runtime.
Release Highlight
Release notes highlight
Double check the following
npm run check:lint)type(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript