Repository: nodejs/node
Pull request: test: split indirect eval import tests

Split indirect eval import tests as they depend on the JS stack to
resolve the referrer.

These tests can be wonky if the loader internals are changed and performs
microtasks with a JS stack (e.g. with `CallbackScope`). In this case, the
referrer will be resolved to the top JS stack frame [`node:internal/process/task_queues.js`](https://github.com/nodejs/node/blob/b23648ef2e85b39fea6ded5e687a1b2eb4da6678/lib/internal/process/task_queues.js#L105).
This is due to the [implementation detail](https://github.com/nodejs/node/blob/b23648ef2e85b39fea6ded5e687a1b2eb4da6678/deps/v8/src/interpreter/bytecode-generator.cc#L7631) of how V8 finds the referrer for a dynamic import
call.

Changed files:
filename: test/es-module/test-esm-dynamic-import.js
patch: @@ -35,15 +35,15 @@ function expectFsNamespace(result) {
 }
 
 // For direct use of import expressions inside of CJS or ES modules, including
-// via eval, all kinds of specifiers should work without issue.
+// via direct/indirect eval, all kinds of specifiers should work without issue.
 (function testScriptOrModuleImport() {
-  // Importing another file, both direct & via eval
+  // Importing another file, both direct & via direct eval
   // expectOkNamespace(import(relativePath));
   expectOkNamespace(eval(`import("${relativePath}")`));
   expectOkNamespace(eval(`import("${relativePath}")`));
   expectOkNamespace(eval(`import(${JSON.stringify(targetURL)})`));
 
-  // Importing a built-in, both direct & via eval
+  // Importing a built-in, both direct & via direct eval
   expectFsNamespace(import('fs'));
   expectFsNamespace(eval('import("fs")'));
   expectFsNamespace(eval('import("fs")'));
@@ -70,6 +70,8 @@ function expectFsNamespace(result) {
   // be treated as a file: URL.
   expectOkNamespace(import(targetURL.pathname));
 
+  // Import with an indirect eval. In this case, the referrer is null and
+  // defaults to the realm record.
   // If the referrer is a realm record, there is no way to resolve the
   // specifier.
   // TODO(legendecas): https://github.com/tc39/ecma262/pull/3195

filename: test/es-module/test-vm-main-context-default-loader-eval.js
patch: @@ -0,0 +1,73 @@
+'use strict';
+/**
+ * This test verifies that dynamic import in an indirect eval without JS stacks.
+ * In this case, the referrer for the dynamic import will be null and the main
+ * context default loader in createContext() resolves to cwd.
+ *
+ * Caveat: this test can be unstable if the loader internals are changed and performs
+ * microtasks with a JS stack (e.g. with CallbackScope). In this case, the
+ * referrer will be resolved to the top JS stack frame `node:internal/process/task_queues.js`.
+ * This is due to the implementation detail of how V8 finds the referrer for a dynamic import
+ * call.
+ */
+
+const common = require('../common');
+
+// Can't process.chdir() in worker.
+const { isMainThread } = require('worker_threads');
+
+if (!isMainThread) {
+  common.skip('This test only works on a main thread');
+}
+
+const tmpdir = require('../common/tmpdir');
+const fixtures = require('../common/fixtures');
+const fs = require('node:fs');
+const {
+  Script,
+  createContext,
+  constants: { USE_MAIN_CONTEXT_DEFAULT_LOADER },
+} = require('node:vm');
+const assert = require('node:assert');
+
+common.expectWarning('ExperimentalWarning',
+                     'vm.USE_MAIN_CONTEXT_DEFAULT_LOADER is an experimental feature and might change at any time');
+assert(
+  !process.execArgv.includes('--experimental-vm-modules'),
+  'This test must be run without --experimental-vm-modules');
+assert.strictEqual(typeof USE_MAIN_CONTEXT_DEFAULT_LOADER, 'symbol');
+
+async function main() {
+  tmpdir.refresh();
+  process.chdir(tmpdir.path);
+
+  //
+  {
+    const options = {
+      importModuleDynamically: USE_MAIN_CONTEXT_DEFAULT_LOADER,
+    };
+    const ctx = createContext({}, options);
+    const s = new Script('Promise.resolve("import(\'./message.mjs\')").then(eval)', {
+      importModuleDynamically: common.mustNotCall(),
+    });
+    await assert.rejects(s.runInContext(ctx), { code: 'ERR_MODULE_NOT_FOUND' });
+  }
+
+  const moduleUrl = fixtures.fileURL('es-modules', 'message.mjs');
+  fs.copyFileSync(moduleUrl, tmpdir.resolve('message.mjs'));
+  {
+    const options = {
+      importModuleDynamically: USE_MAIN_CONTEXT_DEFAULT_LOADER,
+    };
+    const ctx = createContext({}, options);
+    const moduleUrl = fixtures.fileURL('es-modules', 'message.mjs');
+    const namespace = await import(moduleUrl.href);
+    const script = new Script('Promise.resolve("import(\'./message.mjs\')").then(eval)', {
+      importModuleDynamically: common.mustNotCall(),
+    });
+    const result = await script.runInContext(ctx);
+    assert.deepStrictEqual(result, namespace);
+  }
+}
+
+main().catch(common.mustNotCall());

filename: test/es-module/test-vm-main-context-default-loader.js
patch: @@ -16,7 +16,6 @@ const fs = require('fs');
 const {
   compileFunction,
   Script,
-  createContext,
   constants: { USE_MAIN_CONTEXT_DEFAULT_LOADER },
 } = require('vm');
 const assert = require('assert');
@@ -112,34 +111,8 @@ async function main() {
     await testNotFoundErrors(undefinedOptions);
     await testNotFoundErrors(nonPathOptions);
 
-    // createContext() with null referrer also resolves to cwd.
-    {
-      const options = {
-        importModuleDynamically: USE_MAIN_CONTEXT_DEFAULT_LOADER,
-      };
-      const ctx = createContext({}, options);
-      const s = new Script('Promise.resolve("import(\'./message.mjs\')").then(eval)', {
-        importModuleDynamically: common.mustNotCall(),
-      });
-      await assert.rejects(s.runInContext(ctx), { code: 'ERR_MODULE_NOT_FOUND' });
-    }
-
     await testLoader(undefinedOptions);
     await testLoader(nonPathOptions);
-
-    {
-      const options = {
-        importModuleDynamically: USE_MAIN_CONTEXT_DEFAULT_LOADER,
-      };
-      const ctx = createContext({}, options);
-      const moduleUrl = fixtures.fileURL('es-modules', 'message.mjs');
-      const namespace = await import(moduleUrl.href);
-      const script = new Script('Promise.resolve("import(\'./message.mjs\')").then(eval)', {
-        importModuleDynamically: common.mustNotCall(),
-      });
-      const result = await script.runInContext(ctx);
-      assert.deepStrictEqual(result, namespace);
-    }
   }
 }
 

filename: test/parallel/test-vm-module-referrer-realm.mjs
patch: @@ -3,6 +3,18 @@ import * as common from '../common/index.mjs';
 import assert from 'node:assert';
 import { Script, SourceTextModule, createContext } from 'node:vm';
 
+/**
+ * This test verifies that dynamic import in an indirect eval without JS stacks.
+ * In this case, the referrer for the dynamic import will be null and the
+ * per-context importModuleDynamically callback will be invoked.
+ *
+ * Caveat: this test can be unstable if the loader internals are changed and performs
+ * microtasks with a JS stack (e.g. with CallbackScope). In this case, the
+ * referrer will be resolved to the top JS stack frame `node:internal/process/task_queues.js`.
+ * This is due to the implementation detail of how V8 finds the referrer for a dynamic import
+ * call.
+ */
+
 async function test() {
   const foo = new SourceTextModule('export const a = 1;');
   await foo.link(common.mustNotCall());