pr feedback:

- removed unnecessary test checks
- added more export types
- made suggested fixes
- added comments to tests being skipped

Author: PavelSafronov

etc/build-runtime-barrel.mjs

@@ -1,12 +1,10 @@
 import fs from 'node:fs/promises';
 import path from 'node:path';
-import { fileURLToPath } from 'node:url';
 
 // eslint-disable-next-line no-restricted-globals
 const useBundled = process.env.MONGODB_BUNDLED === 'true';
 
-const __dirname = path.dirname(fileURLToPath(import.meta.url));
-const rootDir = path.join(__dirname, '..');
+const rootDir = path.join(import.meta.dirname, '..');
 const outputBarrelFile = path.join(rootDir, 'test/mongodb.ts');
 const source = useBundled ? './mongodb_bundled' : './mongodb_all';
 

etc/bundle-driver.mjs

@@ -21,16 +21,17 @@ await esbuild.build({
   format: 'cjs',
   target: 'node20',
   external: [
-    'bson',
-    'mongodb-connection-string-url',
+    '@aws-sdk/credential-providers',
     '@mongodb-js/saslprep',
     '@mongodb-js/zstd',
-    'mongodb-client-encryption',
-    'snappy',
     '@napi-rs/snappy*',
-    'kerberos',
+    'bson',
     'gcp-metadata',
-    '@aws-sdk/credential-providers'
+    'kerberos',
+    'mongodb-client-encryption',
+    'mongodb-connection-string-url',
+    'snappy',
+    'socks'
   ],
   plugins: [
     {

src/client-side-encryption/providers/azure.ts

@@ -163,10 +163,7 @@ export async function fetchAzureKMSToken(
     const response = await get(url, { headers });
     return await parseResponse(response);
   } catch (error) {
-    if (
-      error instanceof MongoNetworkTimeoutError ||
-      (error && error.constructor && error.constructor.name === 'MongoNetworkTimeoutError')
-    ) {
+    if (error instanceof MongoNetworkTimeoutError) {
       throw new MongoCryptAzureKMSRequestError(`[Azure KMS] ${error.message}`);
     }
     throw error;

test/mongodb_all.ts

@@ -128,6 +128,7 @@ export * from '../src/sdam/topology_description';
 export * from '../src/sessions';
 export * from '../src/sort';
 export * from '../src/timeout';
+export * from '../src/token_bucket';
 export * from '../src/transactions';
 export * from '../src/utils';
 export * from '../src/write_concern';

test/mongodb_bundled.ts

@@ -3,7 +3,7 @@ import { loadContextifiedMongoDBModule } from './tools/runner/vm_context_helper'
 type all = typeof import('./mongodb_all');
 let exportSource: all;
 try {
-  exportSource = loadContextifiedMongoDBModule() as all;
+  exportSource = loadContextifiedMongoDBModule();
 } catch (error) {
   throw new Error(
     `Failed to load contextified MongoDB module: ${error instanceof Error ? error.message : String(error)}`
@@ -152,6 +152,7 @@ export const {
   HEARTBEAT_EVENTS,
   HostAddress,
   hostMatchesWildcards,
+  INITIAL_TOKEN_BUCKET_SIZE,
   InsertOneOperation,
   InsertOperation,
   Int32,
@@ -450,8 +451,11 @@ import type {
   MongoClient as _MongoClient,
   MongoCredentials as _MongoCredentials,
   MongoError as _MongoError,
+  Monitor as _Monitor,
+  MonitorInterval as _MonitorInterval,
   ObjectId as _ObjectId,
   OnDemandDocument as _OnDemandDocument,
+  RTTSampler as _RTTSampler,
   Server as _Server,
   ServerApiVersion as _ServerApiVersion,
   ServerClosedEvent as _ServerClosedEvent,
@@ -508,8 +512,11 @@ export type HostAddress = _HostAddress;
 export type MongoClient = _MongoClient;
 export type MongoCredentials = _MongoCredentials;
 export type MongoError = _MongoError;
+export type Monitor = _Monitor;
+export type MonitorInterval = _MonitorInterval;
 export type ObjectId = _ObjectId;
 export type OnDemandDocument = _OnDemandDocument;
+export type RTTSampler = _RTTSampler;
 export type Server = _Server;
 export type ServerApiVersion = _ServerApiVersion;
 export type ServerClosedEvent = _ServerClosedEvent;

test/tools/runner/vm_context_helper.ts

@@ -1,4 +1,4 @@
-/* eslint-disable no-restricted-globals, @typescript-eslint/no-require-imports */
+/* eslint-disable @typescript-eslint/no-require-imports */
 
 import * as fs from 'node:fs';
 import { isBuiltin } from 'node:module';
@@ -15,10 +15,42 @@ const allowedModules = new Set([
   'mongodb-client-encryption',
   'mongodb-connection-string-url',
   'path',
-  'snappy'
+  'snappy',
+  'socks'
 ]);
 const blockedModules = new Set(['os']);
 
+// TODO: NODE-7460 - Remove Error and other unnecessary exports
+const exposedGlobals = new Set([
+  'AbortController',
+  'AbortSignal',
+  'BigInt',
+  'Buffer',
+  'Date',
+  'Error',
+  'Headers',
+  'Map',
+  'Math',
+  'Promise',
+  'TextDecoder',
+  'TextEncoder',
+  'URL',
+  'URLSearchParams',
+
+  'console',
+  'crypto',
+  'performance',
+  'process',
+
+  'clearImmediate',
+  'clearInterval',
+  'clearTimeout',
+  'setImmediate',
+  'setInterval',
+  'setTimeout',
+  'queueMicrotask'
+]);
+
 /**
  * Creates a require function that blocks access to specified core modules
  */
@@ -34,46 +66,12 @@ function createRestrictedRequire() {
       throw new Error(`Access to core module '${moduleName}' is restricted in this context`);
     }
     return require(moduleName);
-  } as NodeRequire;
+  } as NodeJS.Require;
 }
 
-// Create a sandbox context with necessary globals
-const sandbox = vm.createContext({
+const context = {
   __proto__: null,
 
-  // Console and timing
-  console: console,
-  AbortController: AbortController,
-  AbortSignal: AbortSignal,
-  Date: global.Date,
-  Error: global.Error,
-  URL: global.URL,
-  URLSearchParams: global.URLSearchParams,
-  queueMicrotask: queueMicrotask,
-  performance: global.performance,
-  setTimeout: global.setTimeout,
-  clearTimeout: global.clearTimeout,
-  setInterval: global.setInterval,
-  clearInterval: global.clearInterval,
-  setImmediate: global.setImmediate,
-  clearImmediate: global.clearImmediate,
-
-  // Process
-  process: process,
-
-  // TODO: NODE-7460 - Remove Error and other unnecessary exports
-
-  // Global objects needed for runtime
-  Buffer: Buffer,
-  Headers: global.Headers,
-  Map: Map,
-  Promise: Promise,
-  Math: Math,
-  TextEncoder: global.TextEncoder,
-  TextDecoder: global.TextDecoder,
-  BigInt: global.BigInt,
-  crypto: global.crypto,
-
   // Custom require that blocks core modules
   require: createRestrictedRequire(),
 
@@ -83,7 +81,17 @@ const sandbox = vm.createContext({
   // Needed for some modules
   global: undefined as any,
   globalThis: undefined as any
-});
+};
+
+// Expose allowed globals in the context
+for (const globalName of exposedGlobals) {
+  if (globalName in global) {
+    context[globalName] = (global as any)[globalName];
+  }
+}
+
+// Create a sandbox context with necessary globals
+const sandbox = vm.createContext(context);
 
 // Make global and globalThis point to the sandbox
 sandbox.global = sandbox;
@@ -93,7 +101,7 @@ sandbox.globalThis = sandbox;
  * Load the bundled MongoDB driver module in a VM context
  * This allows us to control the globals that the driver has access to
  */
-export function loadContextifiedMongoDBModule() {
+export function loadContextifiedMongoDBModule(): typeof import('../../mongodb_all') {
   const bundlePath = path.join(__dirname, 'bundle/driver-bundle.js');
 
   if (!fs.existsSync(bundlePath)) {
@@ -114,5 +122,5 @@ export function loadContextifiedMongoDBModule() {
   // Execute the bundle with the restricted require from the sandbox
   fn(moduleContainer.exports, moduleContainer, sandbox.require);
 
-  return moduleContainer.exports;
+  return moduleContainer.exports as unknown as typeof import('../../mongodb_all');
 }

test/unit/assorted/server_discovery_and_monitoring.spec.test.ts

@@ -195,22 +195,22 @@ describe('Server Discovery and Monitoring (spec)', function () {
   let serverConnect: sinon.SinonStub;
 
   before(function () {
-    if (runNodelessTests) {
-      this.skip();
-    }
-
     serverConnect = sinon.stub(Server.prototype, 'connect').callsFake(function () {
       this.s.state = 'connected';
       this.emit('connect');
     });
   });
 
-  after(() => {
+  after(function () {
+    serverConnect.restore();
+  });
+
+  beforeEach(function () {
     if (runNodelessTests) {
-      return;
+      this.currentTest.skipReason =
+        'SDAM spec tests rely heavily on stubbing Connection behavior, which is not currently possible in nodeless environments';
+      this.currentTest.skip();
     }
-
-    serverConnect.restore();
   });
 
   const specTests = collectTests();

test/unit/client-side-encryption/providers/credentialsProvider.test.ts

@@ -6,7 +6,6 @@ import * as sinon from 'sinon';
 // Intentionally import from src, because we need to stub the `get()` function.
 // eslint-disable-next-line @typescript-eslint/no-restricted-imports
 import * as utils from '../../../../src/utils';
-import { runNodelessTests } from '../../../mongodb';
 import {
   AWSSDKCredentialProvider,
   fetchAzureKMSToken,
@@ -24,12 +23,6 @@ const originalSecretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
 const originalSessionToken = process.env.AWS_SESSION_TOKEN;
 
 describe('#refreshKMSCredentials', function () {
-  before(function () {
-    if (runNodelessTests) {
-      this.skip();
-    }
-  });
-
   context('isEmptyCredentials()', () => {
     it('returns true for an empty object', () => {
       expect(isEmptyCredentials('aws', { aws: {} })).to.be.true;

test/unit/cmap/handshake/client_metadata.test.ts

@@ -318,7 +318,9 @@ describe('client metadata module', () => {
     context('when globalThis indicates alternative runtime', () => {
       beforeEach(function () {
         if (runNodelessTests) {
-          this.skip(); // these tests are meant to run in node and will fail in nodeless environments due to the presence of globalThis.Bun or globalThis.Deno
+          this.currentTest.skipReason =
+            'These tests are meant to run in node and will fail in nodeless environments';
+          this.currentTest.skip();
         }
       });
 

test/unit/commands.test.ts

@@ -16,12 +16,6 @@ import {
 } from '../mongodb';
 
 describe('class OpCompressedRequest', () => {
-  before(function () {
-    if (runNodelessTests) {
-      this.skip();
-    }
-  });
-
   context('canCompress()', () => {
     for (const command of uncompressibleCommands) {
       it(`returns true when the command is ${command}`, () => {
@@ -96,20 +90,30 @@ describe('class OpCompressedRequest', () => {
           expect(compressedMessage).to.deep.equal(expectedCompressedCommand);
         });
 
-        it('respects the zlib compression level', async () => {
-          const spy = sinon.spy(compression, 'compress');
-          const [messageHeader] = await new OpCompressedRequest(msg, {
-            agreedCompressor: 'snappy',
-            zlibCompressionLevel: 3
-          }).toBin();
+        describe('zlib compression', function () {
+          before(function () {
+            if (runNodelessTests) {
+              this.currentTest.skipReason =
+                'This test relies on sinon spying on the compress() function, which is not currently possible in nodeless environments';
+              this.currentTest.skip();
+            }
+          });
 
-          expect(messageHeader.readInt32LE(12), 'opcode is not OP_COMPRESSED').to.equal(2012);
+          it('respects the zlib compression level', async function () {
+            const spy = sinon.spy(compression, 'compress');
+            const [messageHeader] = await new OpCompressedRequest(msg, {
+              agreedCompressor: 'snappy',
+              zlibCompressionLevel: 3
+            }).toBin();
 
-          expect(spy).to.have.been.called;
+            expect(messageHeader.readInt32LE(12), 'opcode is not OP_COMPRESSED').to.equal(2012);
 
-          expect(spy.args[0][0]).to.deep.equal({
-            agreedCompressor: 'snappy',
-            zlibCompressionLevel: 3
+            expect(spy).to.have.been.called;
+
+            expect(spy.args[0][0]).to.deep.equal({
+              agreedCompressor: 'snappy',
+              zlibCompressionLevel: 3
+            });
           });
         });
       });