From f8d26c60115566d1d287553cc93a9ab0515512f7 Mon Sep 17 00:00:00 2001
From: Antoine du Hamel <duhamelantoine1995@gmail.com>
Date: Fri, 29 Apr 2022 21:14:43 +0200
Subject: [PATCH] test: fix `common.mustNotCall` error message
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When using `util.inspect` as a callback, the array index (second
argument) is picked up as the `showHidden` param, and the argument
array (third argument) as the `depth` param. This fails with a seemingly
unrelated error message when the argument array contains a
`Symbol`-related property.

PR-URL: https://github.com/nodejs/node/pull/42917
Reviewed-By: Michaël Zasso <targos@protonmail.com>
---
 test/common/index.js                       | 24 +++++++++++-----------
 test/parallel/test-common-must-not-call.js | 15 ++++++++++++++
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/test/common/index.js b/test/common/index.js
index dbe40712862985..57da03daefd938 100644
--- a/test/common/index.js
+++ b/test/common/index.js
@@ -29,7 +29,7 @@ const fs = require('fs');
 // Do not require 'os' until needed so that test-os-checked-function can
 // monkey patch it. If 'os' is required here, that test will fail.
 const path = require('path');
-const util = require('util');
+const { inspect } = require('util');
 const { isMainThread } = require('worker_threads');
 
 const tmpdir = require('./tmpdir');
@@ -97,7 +97,7 @@ if (process.argv.length === 2 &&
           (process.features.inspector || !flag.startsWith('--inspect'))) {
         console.log(
           'NOTE: The test started as a child_process using these flags:',
-          util.inspect(flags),
+          inspect(flags),
           'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.'
         );
         const args = [...flags, ...process.execArgv, ...process.argv.slice(1)];
@@ -149,7 +149,7 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
   process.on('exit', () => {
     // Iterate through handles to make sure nothing crashes
     for (const k in initHandles)
-      util.inspect(initHandles[k]);
+      inspect(initHandles[k]);
   });
 
   const _queueDestroyAsyncId = async_wrap.queueDestroyAsyncId;
@@ -159,7 +159,7 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
       process._rawDebug();
       throw new Error(`same id added to destroy list twice (${id})`);
     }
-    destroyListList[id] = util.inspect(new Error());
+    destroyListList[id] = inspect(new Error());
     _queueDestroyAsyncId(id);
   };
 
@@ -173,7 +173,7 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
       }
       initHandles[id] = {
         resource,
-        stack: util.inspect(new Error()).substr(6)
+        stack: inspect(new Error()).substr(6)
       };
     },
     before() { },
@@ -184,7 +184,7 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
         process._rawDebug();
         throw new Error(`destroy called for same id (${id})`);
       }
-      destroydIdsList[id] = util.inspect(new Error());
+      destroydIdsList[id] = inspect(new Error());
     },
   }).enable();
 }
@@ -424,7 +424,7 @@ function _mustCallInner(fn, criteria = 1, field) {
   const context = {
     [field]: criteria,
     actual: 0,
-    stack: util.inspect(new Error()),
+    stack: inspect(new Error()),
     name: fn.name || '<anonymous>'
   };
 
@@ -512,7 +512,7 @@ function mustNotCall(msg) {
   const callSite = getCallSite(mustNotCall);
   return function mustNotCall(...args) {
     const argsInfo = args.length > 0 ?
-      `\ncalled with arguments: ${args.map(util.inspect).join(', ')}` : '';
+      `\ncalled with arguments: ${args.map((arg) => inspect(arg)).join(', ')}` : '';
     assert.fail(
       `${msg || 'function should not have been called'} at ${callSite}` +
       argsInfo);
@@ -614,7 +614,7 @@ function expectWarning(nameOrMap, expected, code) {
       if (!catchWarning[warning.name]) {
         throw new TypeError(
           `"${warning.name}" was triggered without being expected.\n` +
-          util.inspect(warning)
+          inspect(warning)
         );
       }
       catchWarning[warning.name](warning);
@@ -635,7 +635,7 @@ function expectsError(validator, exact) {
     if (args.length !== 1) {
       // Do not use `assert.strictEqual()` to prevent `inspect` from
       // always being called.
-      assert.fail(`Expected one argument, got ${util.inspect(args)}`);
+      assert.fail(`Expected one argument, got ${inspect(args)}`);
     }
     const error = args.pop();
     const descriptor = Object.getOwnPropertyDescriptor(error, 'message');
@@ -740,9 +740,9 @@ function invalidArgTypeHelper(input) {
     if (input.constructor && input.constructor.name) {
       return ` Received an instance of ${input.constructor.name}`;
     }
-    return ` Received ${util.inspect(input, { depth: -1 })}`;
+    return ` Received ${inspect(input, { depth: -1 })}`;
   }
-  let inspected = util.inspect(input, { colors: false });
+  let inspected = inspect(input, { colors: false });
   if (inspected.length > 25)
     inspected = `${inspected.slice(0, 25)}...`;
   return ` Received type ${typeof input} (${inspected})`;
diff --git a/test/parallel/test-common-must-not-call.js b/test/parallel/test-common-must-not-call.js
index dcea7059dac7f5..b3c94a2390ffb6 100644
--- a/test/parallel/test-common-must-not-call.js
+++ b/test/parallel/test-common-must-not-call.js
@@ -39,3 +39,18 @@ try {
 } catch (e) {
   validate2(e);
 }
+
+assert.throws(
+  () => new Proxy({ prop: Symbol() }, { get: common.mustNotCall() }).prop,
+  { code: 'ERR_ASSERTION' }
+);
+
+{
+  const { inspect } = util;
+  delete util.inspect;
+  assert.throws(
+    () => common.mustNotCall()(null),
+    { code: 'ERR_ASSERTION' }
+  );
+  util.inspect = inspect;
+}