WIP [SPIKE] - test(NODE-7345): custom vm.Context runner for mocha #4844

tadjik1 · 2026-01-20T13:57:32Z

Description

Summary of Changes

Notes for Reviewers

To run the tests:

MONGODB_URI=mongodb://localhost:27017,localhost:27018,localhost:27019 npm run check:runtime-independency

What is the motivation for this change?

Release Highlight

Release notes highlight

Double check the following

Lint is passing (npm run check:lint)
Self-review completed using the steps outlined here
PR title follows the correct format: type(NODE-xxxx)[!]: description
- Example: feat(NODE-1234)!: rewriting everything in coffeescript
Changes are covered by tests
New TODOs have a related JIRA ticket

addaleax · 2026-01-20T14:06:09Z

test/tools/runner/vm_runner.js

@@ -0,0 +1,206 @@
+/* eslint-disable no-restricted-globals */


Suggested change

/* eslint-disable no-restricted-globals */

'use strict';

/* eslint-disable no-restricted-globals */

(this is a script, not an ES module, so strict mode is not the default)

The file had been converted to ts as @PavelSafronov suggested

addaleax · 2026-01-20T14:36:51Z

test/tools/runner/vm_runner.js

+  reporter: 'test/tools/reporter/mongodb_reporter.js',
+  sort: true,
+  color: true,
+  ignore: [


Maybe add a comment about why these are being ignored?

Yes, thanks! I copied this configuration from the integration tests setup, will link these files together.

I included the original file all along instead of manually picking up its' options

addaleax · 2026-01-20T14:38:09Z

test/tools/runner/vm_runner.js

+
+function createSandboxContext(filename) {
+  const exportsContainer = {};
+  return {


Suggested change

return {

return {

__proto__: null,

Just so none of the property lookups for built-ins fall back to the global one unintentionally

thanks, included!

addaleax · 2026-01-20T14:41:41Z

test/tools/runner/vm_runner.js

+    exports: exportsContainer,
+    module: { exports: exportsContainer },
+    __filename: filename,
+    __dirname: path.dirname(filename),


I'd try to emulate the behavior of the built-in CommonJS evaluator as closely as possible.

None of these are typically globals, and modules do share a single global – all of require, module, exports, __filenameand__dirname` are injected as parameters to an implicit function that is used to wrap around user code

e.g. https://github.com/mongodb-js/mongosh/blob/5ea0a3617c9692cbb5c1e632a859d6688b69a6f1/packages/shell-api/src/runtime-independence.spec.ts#L41-L46

(you should generally be able to use that file as a reference for a lot of this)

That is awesome, thanks! I haven't seen this file before, this is huge help indeed.

Added the same style wrapper around the script and define "globals" (context) just once so it's shared.

addaleax · 2026-01-20T14:43:52Z

test/tools/runner/vm_runner.js

+  return function sandboxRequire(moduleIdentifier) {
+    if (!moduleIdentifier.startsWith('.')) {
+      return require(moduleIdentifier);
+    }


Is this intentional? I figured we'd want to require almost everything inside the sandbox except for Node.js built-ins and maybe a list of modules that are explicit exceptions to this rule

This is temporary solution to be able to import all dev dependencies and to highlight the more general idea (this ticket and PR is spike), so I was focusing mostly on "running existing integration tests inside vm.Context sandbox". But you are absolutely right, ideally all prod dependencies should be required from the inside, otherwise it's pointless (if driver is browser-compatible but, say bson not - we can't run application).

PavelSafronov · 2026-01-20T20:17:11Z

test/tools/runner/vm_runner.js

Can we switch this file to TS? Some of our test runners are JS, but most are TS, would be nice if all new code was TS. Unless there's some reason that we can't/shouldn't.

PavelSafronov · 2026-01-20T21:08:01Z

Re: testing - there's a command for running tests against 3 local servers, but what is the command we should use to start those 3 servers?

I tried starting a replica set (TOPOLOGY=replica_set AUTH=noauth bash ./.evergreen/run-orchestration.sh) and executed the test command, but encountered 25 test failures.

PavelSafronov · 2026-01-20T21:37:02Z

test/tools/runner/vm_runner.js

+let compilerOptions = { module: ts.ModuleKind.CommonJS };
+const tsConfigPath = path.join(__dirname, '../../tsconfig.json');
+const configFile = ts.readConfigFile(tsConfigPath, ts.sys.readFile);
+if (!configFile.error) {


Throw if configFile.error is present?

tadjik1 · 2026-01-21T09:21:15Z

Re: testing - there's a command for running tests against 3 local servers, but what is the command we should use to start those 3 servers?

I tried starting a replica set (TOPOLOGY=replica_set AUTH=noauth bash ./.evergreen/run-orchestration.sh) and executed the test command, but encountered 25 test failures.

I actually don't know the answer yet. I mean you did everything right, I personally use mlaunch but it doesn't matter how to run the server.
Tests are failing and I would need to dig deeper to understand if this can be easily fixed (or if this setup is not appropriate for some of our integration tests).

tadjik1 · 2026-01-23T13:12:34Z

test/tools/runner/vm_runner.ts

+          // force instanceof to work across contexts by defining custom `instanceof` function
+          Object.defineProperty(value, Symbol.hasInstance, {
+            value: (instance: any) => {
+              return (
+                instance && (instance.constructor.name === value.name || instance instanceof value)
+              );
+            }
+          });


I don't really like this part, but I found this problem a little bit tricky. The problem is that host environment (and consequently chai) has its' own set of Error classes, so instanceof from inside the sandbox is broken. @addaleax, maybe there is more convenient way to make it work rather than re-defining instanceof?

tadjik1 · 2026-01-23T13:14:57Z

test/tools/runner/vm_runner.ts

+    const sandboxedDependencies = ['bson'];
+    const isSandboxedDep = sandboxedDependencies.some(
+      dep => moduleIdentifier === dep || moduleIdentifier.startsWith(`${dep}/`)
+    );
+    if (!moduleIdentifier.startsWith('.') && !isSandboxedDep) {
+      return require(moduleIdentifier);
+    }


@addaleax this seems to work kinda of fine, we can list dependencies that should be loaded from within the sandbox. i decided to go with this approach because list of dev and prod dependencies we need to import in host is large. but maybe i'm missing something and it should be the other way around (everything is inside except certain builtin modules)?

test(NODE-7345): custom vm.Context runner for mocha

3f33ad8

tadjik1 force-pushed the NODE-7345 branch from 0158e19 to 3f33ad8 Compare January 20, 2026 14:15

addaleax reviewed Jan 20, 2026

View reviewed changes

PavelSafronov reviewed Jan 20, 2026

View reviewed changes

tadjik1 changed the title ~~test(NODE-7345): custom vm.Context runner for mocha~~ WIP - test(NODE-7345): custom vm.Context runner for mocha Jan 21, 2026

tadjik1 changed the title ~~WIP - test(NODE-7345): custom vm.Context runner for mocha~~ WIP [SPIKE] - test(NODE-7345): custom vm.Context runner for mocha Jan 21, 2026

address review comments

e267e34

tadjik1 commented Jan 23, 2026

View reviewed changes

tadjik1 added 6 commits January 23, 2026 14:16

throw if ts config is missing

8f98008

run tests on evegreen

a7b383a

make names consistent

38ff598

Merge branch 'main' into NODE-7345

3bf91cd

add test runner in evergreen

626e2a5

add buildvariant to run tests

1c0af0c

	/* eslint-disable no-restricted-globals */
	'use strict';
	/* eslint-disable no-restricted-globals */

WIP [SPIKE] - test(NODE-7345): custom vm.Context runner for mocha #4844

Are you sure you want to change the base?

WIP [SPIKE] - test(NODE-7345): custom vm.Context runner for mocha #4844

Conversation

tadjik1 commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Summary of Changes

Notes for Reviewers

What is the motivation for this change?

Release Highlight

Release notes highlight

Double check the following

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tadjik1 Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PavelSafronov commented Jan 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tadjik1 commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tadjik1 commented Jan 20, 2026 •

edited

Loading

tadjik1 Jan 23, 2026 •

edited

Loading

tadjik1 commented Jan 21, 2026 •

edited

Loading