test: fix some failing and disabled node specs (#19518)

* test: fix some failing and disabled node specs

These tests were failing due to mismatched stacktraces as a result of
our Module._load hook.  This fixes those by adding a flag to optionally
not hook those calls and instead do the asar override eagily.

* ELECTRON_EAGER_ASAR_HOOK => ELECTRON_EAGER_ASAR_HOOK_FOR_TESTING

* test: parallel/test-zlib-unused-weak consistently fails

Co-authored-by: Cheng Zhao <zcbenz@github.com>

lib/common/asar.js

@@ -711,31 +711,42 @@
     overrideAPISync(Module._extensions, '.node', 1)
     overrideAPISync(fs, 'openSync')
 
-    // Lazily override the child_process APIs only when child_process is fetched the first time
+    const overrideChildProcess = (childProcess) => {
-    const originalModuleLoad = Module._load
+      // Executing a command string containing a path to an asar archive
-    Module._load = (request, ...args) => {
+      // confuses `childProcess.execFile`, which is internally called by
-      const loadResult = originalModuleLoad(request, ...args)
+      // `childProcess.{exec,execSync}`, causing Electron to consider the full
-      if (request === 'child_process') {
+      // command as a single path to an archive.
-        if (!v8Util.getHiddenValue(loadResult, 'asar-ready')) {
+      const { exec, execSync } = childProcess
-          v8Util.setHiddenValue(loadResult, 'asar-ready', true)
+      childProcess.exec = invokeWithNoAsar(exec)
-          // Just to make it obvious what we are dealing with here
+      childProcess.exec[util.promisify.custom] = invokeWithNoAsar(exec[util.promisify.custom])
-          const childProcess = loadResult
+      childProcess.execSync = invokeWithNoAsar(execSync)
 
-          // Executing a command string containing a path to an asar
+      overrideAPI(childProcess, 'execFile')
-          // archive confuses `childProcess.execFile`, which is internally
+      overrideAPISync(childProcess, 'execFileSync')
-          // called by `childProcess.{exec,execSync}`, causing
+    }
-          // Electron to consider the full command as a single path
-          // to an archive.
-          const { exec, execSync } = childProcess
-          childProcess.exec = invokeWithNoAsar(exec)
-          childProcess.exec[util.promisify.custom] = invokeWithNoAsar(exec[util.promisify.custom])
-          childProcess.execSync = invokeWithNoAsar(execSync)
 
-          overrideAPI(childProcess, 'execFile')
+    // Lazily override the child_process APIs only when child_process is
-          overrideAPISync(childProcess, 'execFileSync')
+    // fetched the first time.  We will eagerly override the child_process APIs
+    // when this env var is set so that stack traces generated inside node unit
+    // tests will match. This env var will only slow things down in users apps
+    // and should not be used.
+    if (process.env.ELECTRON_EAGER_ASAR_HOOK_FOR_TESTING) {
+      overrideChildProcess(require('child_process'))
+    } else {
+      const originalModuleLoad = Module._load
+      Module._load = (request, ...args) => {
+        const loadResult = originalModuleLoad(request, ...args)
+        if (request === 'child_process') {
+          if (!v8Util.getHiddenValue(loadResult, 'asar-ready')) {
+            v8Util.setHiddenValue(loadResult, 'asar-ready', true)
+            // Just to make it obvious what we are dealing with here
+            const childProcess = loadResult
+
+            overrideChildProcess(childProcess)
+          }
         }
+        return loadResult
       }
-      return loadResult
     }
   }
 })()

script/node-disabled-tests.json

@@ -30,36 +30,17 @@
   "es-module/test-esm-snapshot",
   "es-module/test-esm-specifiers",
   "es-module/test-esm-type-flag",
-  "message/assert_throws_stack",
   "message/async_error_eval_esm",
   "message/async_error_sync_esm",
-  "message/console",
-  "message/core_line_numbers",
-  "message/error_exit",
   "message/esm_display_syntax_error",
   "message/esm_display_syntax_error_import",
   "message/esm_display_syntax_error_import_module",
   "message/esm_display_syntax_error_module",
-  "message/eval_messages",
-  "message/events_unhandled_error_common_trace",
-  "message/events_unhandled_error_nexttick",
-  "message/events_unhandled_error_sameline",
-  "message/events_unhandled_error_subclass",
-  "message/if-error-has-good-stack",
-  "message/internal_assert",
-  "message/internal_assert_fail",
-  "message/promise_always_throw_unhandled",
   "message/source_map_throw_catch",
   "message/source_map_throw_first_tick",
   "message/source_map_throw_set_immediate",
-  "message/undefined_reference_in_new_context",
   "message/unhandled_promise_trace_warnings",
-  "message/util_inspect_error",
   "message/v8_warning",
-  "message/vm_display_runtime_error",
-  "message/vm_display_syntax_error",
-  "message/vm_dont_display_runtime_error",
-  "message/vm_dont_display_syntax_error",
   "parallel/test-assert",
   "parallel/test-async-hooks-close-during-destroy",
   "parallel/test-async-hooks-promise",

script/node-spec-runner.js

@@ -21,7 +21,8 @@ async function main () {
   const testChild = cp.spawn('python', ['tools/test.py', '--verbose', '-p', 'tap', '--logfile', TAP_FILE_NAME, '--mode=debug', 'default', `--skip-tests=${DISABLED_TESTS.join(',')}`, '--shell', utils.getAbsoluteElectronExec(), '-J'], {
     env: {
       ...process.env,
-      ELECTRON_RUN_AS_NODE: 'true'
+      ELECTRON_RUN_AS_NODE: 'true',
+      ELECTRON_EAGER_ASAR_HOOK_FOR_TESTING: 'true'
     },
     cwd: NODE_DIR,
     stdio: 'inherit'