fix(ext/node): node:test improvements (#33929)

- attach some missing functions to default export
- fix globalThis leaking into mock function
- support module level hooks
- enables 5 node compat tests

Fixes #33926

Author: nathanwhit

ext/node/polyfills/testing.ts

@@ -3,6 +3,7 @@
 // deno-lint-ignore-file prefer-primordials
 
 (function () {
+"use strict";
 const { core, primordials } = globalThis.__bootstrap;
 const {
   ArrayPrototypeForEach,
@@ -21,6 +22,7 @@ const {
   Promise,
   PromisePrototypeThen,
   ReflectApply,
+  ReflectConstruct,
   SafeArrayIterator,
   SafeMap,
   String,
@@ -327,33 +329,65 @@ class NodeTestContext {
 
 let currentSuite = null;
 
+const rootBeforeHooks = [];
+const rootAfterHooks = [];
+const rootBeforeEachHooks = [];
+const rootAfterEachHooks = [];
+let rootBeforeRan = false;
+
+async function runRootBeforeOnce() {
+  if (rootBeforeRan) return;
+  rootBeforeRan = true;
+  if (rootBeforeHooks.length === 0) return;
+  const rootCtx = { name: "<root>", fullName: "<root>" };
+  for (const hook of new SafeArrayIterator(rootBeforeHooks)) {
+    await hook(rootCtx);
+  }
+}
+
+async function runRootAfterIfDone() {
+  if (activeNodeTests !== 0) return;
+  if (rootAfterHooks.length === 0) return;
+  const rootCtx = { name: "<root>", fullName: "<root>" };
+  // Snapshot and clear so we only run once even if more tests get queued.
+  const hooks = ArrayPrototypeSplice(rootAfterHooks, 0, rootAfterHooks.length);
+  for (const hook of new SafeArrayIterator(hooks)) {
+    try {
+      await hook(rootCtx);
+    } catch { /* ignore */ }
+  }
+}
+
 class TestSuite {
   #denoTestContext;
+  nodeTestContext;
   entries = [];
   beforeAllHooks = [];
   afterAllHooks = [];
   beforeEachHooks = [];
   afterEachHooks = [];
 
-  constructor(t) {
+  constructor(t, nodeTestContext) {
     this.#denoTestContext = t;
+    this.nodeTestContext = nodeTestContext;
   }
 
   addTest(name, options, fn, overrides) {
     const prepared = prepareOptions(name, options, fn, overrides);
     const beforeEach = this.beforeEachHooks;
     const afterEach = this.afterEachHooks;
+    const suiteNodeContext = this.nodeTestContext;
     ArrayPrototypePush(this.entries, {
       name: prepared.name,
       fn: async (denoTestContext) => {
         const newNodeTextContext = new NodeTestContext(
           denoTestContext,
-          undefined,
+          suiteNodeContext,
           prepared.name,
         );
         try {
           for (const hook of new SafeArrayIterator(beforeEach)) {
-            await hook();
+            await hook(newNodeTextContext);
           }
           const result = await prepared.fn(newNodeTextContext);
           newNodeTextContext._checkPlan();
@@ -366,7 +400,9 @@ class TestSuite {
           }
         } finally {
           for (const hook of new SafeArrayIterator(afterEach)) {
-            await hook();
+            try {
+              await hook(newNodeTextContext);
+            } catch { /* ignore */ }
           }
         }
       },
@@ -377,9 +413,10 @@ class TestSuite {
   addSuite(name, options, fn, overrides) {
     const prepared = prepareOptions(name, options, fn, overrides);
     const { promise, resolve } = Promise.withResolvers();
+    const parentSuiteContext = this.nodeTestContext;
     ArrayPrototypePush(this.entries, {
       name: prepared.name,
-      fn: wrapSuiteFn(prepared.fn, resolve),
+      fn: wrapSuiteFn(prepared.fn, resolve, prepared.name, parentSuiteContext),
       ignore: !!prepared.options.todo || !!prepared.options.skip,
     });
     return promise;
@@ -429,7 +466,13 @@ function prepareOptions(name, options, fn, overrides) {
 function wrapTestFn(fn, resolve, name) {
   return async function (t) {
     const nodeTestContext = new NodeTestContext(t, undefined, name);
+    let beforeEachOk = false;
     try {
+      await runRootBeforeOnce();
+      for (const hook of new SafeArrayIterator(rootBeforeEachHooks)) {
+        await hook(nodeTestContext);
+      }
+      beforeEachOk = true;
       if (fn.length >= 2) {
         await new Promise((testResolve, testReject) => {
           pendingCallbackReject = testReject;
@@ -469,7 +512,15 @@ function wrapTestFn(fn, resolve, name) {
         throw sanitizeThrowValue(err);
       }
     } finally {
+      if (beforeEachOk) {
+        for (const hook of new SafeArrayIterator(rootAfterEachHooks)) {
+          try {
+            await hook(nodeTestContext);
+          } catch { /* swallow to match node behavior on hook error */ }
+        }
+      }
       activeNodeTests--;
+      await runRootAfterIfDone();
       resolve();
     }
   };
@@ -496,12 +547,15 @@ function prepareDenoTest(name, options, fn, overrides) {
   return promise;
 }
 
-function wrapSuiteFn(fn, resolve) {
+function wrapSuiteFn(fn, resolve, name, parentNodeContext) {
   return async function (t) {
+    const isTopLevel = parentNodeContext === undefined;
+    if (isTopLevel) await runRootBeforeOnce();
+    const suiteNodeContext = new NodeTestContext(t, parentNodeContext, name);
     const prevSuite = currentSuite;
-    const suite = currentSuite = new TestSuite(t);
+    const suite = currentSuite = new TestSuite(t, suiteNodeContext);
     try {
-      fn();
+      fn(suiteNodeContext);
     } finally {
       currentSuite = prevSuite;
     }
@@ -516,7 +570,10 @@ function wrapSuiteFn(fn, resolve) {
           await hook();
         }
       } finally {
-        activeNodeTests--;
+        if (isTopLevel) {
+          activeNodeTests--;
+          await runRootAfterIfDone();
+        }
         resolve();
       }
     }
@@ -532,7 +589,7 @@ function prepareDenoTestForSuite(name, options, fn, overrides) {
 
   const denoTestOptions = {
     name: prepared.name,
-    fn: wrapSuiteFn(prepared.fn, resolve),
+    fn: wrapSuiteFn(prepared.fn, resolve, prepared.name, undefined),
     only: prepared.options.only,
     ignore: !!prepared.options.todo || !!prepared.options.skip,
     sanitizeOnly: false,
@@ -593,7 +650,7 @@ function before(fn, _options) {
     ArrayPrototypePush(currentSuite.beforeAllHooks, fn);
     return;
   }
-  notImplemented("test.before (module-level, outside suite)");
+  ArrayPrototypePush(rootBeforeHooks, fn);
 }
 
 function after(fn, _options) {
@@ -604,7 +661,7 @@ function after(fn, _options) {
     ArrayPrototypePush(currentSuite.afterAllHooks, fn);
     return;
   }
-  notImplemented("test.after (module-level, outside suite)");
+  ArrayPrototypePush(rootAfterHooks, fn);
 }
 
 function beforeEach(fn, _options) {
@@ -615,7 +672,7 @@ function beforeEach(fn, _options) {
     ArrayPrototypePush(currentSuite.beforeEachHooks, fn);
     return;
   }
-  notImplemented("test.beforeEach (module-level, outside suite)");
+  ArrayPrototypePush(rootBeforeEachHooks, fn);
 }
 
 function afterEach(fn, _options) {
@@ -626,7 +683,7 @@ function afterEach(fn, _options) {
     ArrayPrototypePush(currentSuite.afterEachHooks, fn);
     return;
   }
-  notImplemented("test.afterEach (module-level, outside suite)");
+  ArrayPrototypePush(rootAfterEachHooks, fn);
 }
 
 test.it = test;
@@ -679,23 +736,26 @@ class MockFunctionContext {
       this.#restore();
       this.#restore = undefined;
     }
+    this._restored = true;
     const idx = ArrayPrototypeIndexOf(activeMocks, this);
     if (idx !== -1) {
       ArrayPrototypeSplice(activeMocks, idx, 1);
     }
   }
 
-  _recordCall(thisArg, args, result, error) {
+  _recordCall(thisArg, args, result, error, target) {
     ArrayPrototypePush(this.#calls, {
       arguments: args,
       error,
       result,
       stack: new Error(),
+      target,
       this: thisArg,
     });
   }
 
   _shouldMock() {
+    if (this._restored) return false;
     if (this.#times === undefined) return true;
     return this.#calls.length < this.#times;
   }
@@ -717,22 +777,47 @@ class MockFunctionContext {
 
 function createMockFunction(original, implementation, ctx) {
   const mockFn = function (...args) {
+    const newTarget = new.target;
+    const isCtor = newTarget !== undefined;
+    // The IIFE wrapping this module is sloppy, so a plain call leaks
+    // globalThis as `this`. Match strict-mode/Node semantics.
+    const thisArg = !isCtor && this === globalThis ? undefined : this;
     const impl = ctx._shouldMock()
       ? (ctx._nextImpl() ?? implementation ?? original)
       : original;
 
     let result;
     let error;
 
+    // If called directly (not via subclass), use the original constructor
+    // so the produced instance has its prototype, and so call.target reports
+    // the user's class (not the mock wrapper).
+    const ctorTarget = isCtor && newTarget === mockFn ? impl : newTarget;
     try {
-      result = impl ? ReflectApply(impl, this, args) : undefined;
+      if (isCtor) {
+        result = impl ? ReflectConstruct(impl, args, ctorTarget) : undefined;
+      } else {
+        result = impl ? ReflectApply(impl, thisArg, args) : undefined;
+      }
     } catch (e) {
       error = e;
-      ctx._recordCall(this, args, undefined, error);
+      ctx._recordCall(
+        isCtor ? thisArg : thisArg,
+        args,
+        undefined,
+        error,
+        ctorTarget,
+      );
       throw e;
     }
 
-    ctx._recordCall(this, args, result);
+    ctx._recordCall(
+      isCtor ? result : thisArg,
+      args,
+      result,
+      undefined,
+      ctorTarget,
+    );
     return result;
   };
 
@@ -907,6 +992,11 @@ const mock = {
 
 test.test = test;
 test.mock = mock;
+test.before = before;
+test.after = after;
+test.beforeEach = beforeEach;
+test.afterEach = afterEach;
+test.run = run;
 
 return {
   run,

tests/node_compat/config.jsonc

@@ -2920,6 +2920,7 @@
     "parallel/test-require-resolve.js": {},
     "parallel/test-require-resolve-invalid-paths.js": {},
     "parallel/test-require-resolve-opts-paths-relative.js": {},
+    "parallel/test-runner-aftereach-runtime-skip.js": {},
     "parallel/test-runner-aliases.js": {},
     "parallel/test-runner-cli-concurrency.js": {
       "ignore": true,
@@ -2939,6 +2940,7 @@
     // "parallel/test-runner-coverage-thresholds.js": {},
     "parallel/test-runner-enable-source-maps-issue.js": {},
     "parallel/test-runner-filter-warning.js": {},
+    "parallel/test-runner-root-after-with-refed-handles.js": {},
     "parallel/test-runner-snapshot-file-tests.js": {
       "ignore": true,
       "reason": "Node.js snapshot/heap profiling features (--build-snapshot, --heap-prof, --heapsnapshot-near-heap-limit) are not implemented in Deno"
@@ -4070,6 +4072,8 @@
       "ignore": true,
       "reason": "Node.js system CA certificate handling is not implemented in Deno"
     },
+    "test-runner/test-output-arbitrary-output-colored.mjs": {},
+    "test-runner/test-output-no-tests.mjs": {},
     "test-runner/test-output-output-cli.mjs": {
       "ignore": true,
       "reason": "Tests Node.js-specific CLI flags/options that are not supported in Deno"
@@ -4078,6 +4082,7 @@
       "ignore": true,
       "reason": "Tests Node.js-specific CLI flags/options that are not supported in Deno"
     },
+    "test-runner/test-output-typescript-coverage.mjs": {},
     // TODO(bartlomieju): disabled during work on `node:inspector`, this test didn't actualy run before
     // "sequential/test-watch-mode-inspect.mjs": {},
     // TODO(bartlomieju): this test is very flaky on CI, especially on Windows ARM