From 2eb1aa81faeb7f060457044762e16a317f19939c Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Wed, 29 Nov 2017 15:59:04 -0500
Subject: [PATCH] test: move common.fires() to inspector-helper

common.fires() is specific to the inspector tests so move it to
inspector-helper.js. The one REPL test that used common.fires() does not
seem to need it. It provided a 1 second timeout for operations, but that
timeout appears both arbitrary and ineffective as the test passes if it
is reduced to even 1 millisecond.

Backport-PR-URL: https://github.com/nodejs/node/pull/18096
PR-URL: https://github.com/nodejs/node/pull/17401
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 test/common/README.md           |  9 -------
 test/common/index.js            | 42 ---------------------------------
 test/common/inspector-helper.js | 41 ++++++++++++++++++++++++++++++--
 3 files changed, 39 insertions(+), 53 deletions(-)

diff --git a/test/common/README.md b/test/common/README.md
index d3010d632ac5d5..a57b0df3e3f1ab 100644
--- a/test/common/README.md
+++ b/test/common/README.md
@@ -119,15 +119,6 @@ Tests whether `name` and `expected` are part of a raised warning.
 
 Checks if `pathname` exists
 
-### fires(promise, [error], [timeoutMs])
-* promise [&lt;Promise]
-* error [&lt;String] default = 'timeout'
-* timeoutMs [&lt;Number] default = 100
-
-Returns a new promise that will propagate `promise` resolution or rejection if
-that happens within the `timeoutMs` timespan, or rejects with `error` as
-a reason otherwise.
-
 ### getArrayBufferViews(buf)
 * `buf` [&lt;Buffer>]
 * return [&lt;ArrayBufferView&#91;&#93;>]
diff --git a/test/common/index.js b/test/common/index.js
index 80f59333626b9a..380bdf0f4b7af6 100644
--- a/test/common/index.js
+++ b/test/common/index.js
@@ -866,32 +866,6 @@ function restoreWritable(name) {
   delete process[name].writeTimes;
 }
 
-function onResolvedOrRejected(promise, callback) {
-  return promise.then((result) => {
-    callback();
-    return result;
-  }, (error) => {
-    callback();
-    throw error;
-  });
-}
-
-function timeoutPromise(error, timeoutMs) {
-  let clearCallback = null;
-  let done = false;
-  const promise = onResolvedOrRejected(new Promise((resolve, reject) => {
-    const timeout = setTimeout(() => reject(error), timeoutMs);
-    clearCallback = () => {
-      if (done)
-        return;
-      clearTimeout(timeout);
-      resolve();
-    };
-  }), () => done = true);
-  promise.clear = clearCallback;
-  return promise;
-}
-
 exports.hijackStdout = hijackStdWritable.bind(null, 'stdout');
 exports.hijackStderr = hijackStdWritable.bind(null, 'stderr');
 exports.restoreStdout = restoreWritable.bind(null, 'stdout');
@@ -905,19 +879,3 @@ exports.firstInvalidFD = function firstInvalidFD() {
   } catch (e) {}
   return fd;
 };
-
-exports.fires = function fires(promise, error, timeoutMs) {
-  if (!timeoutMs && util.isNumber(error)) {
-    timeoutMs = error;
-    error = null;
-  }
-  if (!error)
-    error = 'timeout';
-  if (!timeoutMs)
-    timeoutMs = 100;
-  const timeout = timeoutPromise(error, timeoutMs);
-  return Promise.race([
-    onResolvedOrRejected(promise, () => timeout.clear()),
-    timeout
-  ]);
-};
diff --git a/test/common/inspector-helper.js b/test/common/inspector-helper.js
index 0d010a8ca70617..1f1738e31b75a8 100644
--- a/test/common/inspector-helper.js
+++ b/test/common/inspector-helper.js
@@ -216,7 +216,7 @@ class InspectorSession {
   waitForNotification(methodOrPredicate, description) {
     const desc = description || methodOrPredicate;
     const message = `Timed out waiting for matching notification (${desc}))`;
-    return common.fires(
+    return fires(
       this._asyncWaitForNotification(methodOrPredicate), message, TIMEOUT);
   }
 
@@ -323,7 +323,7 @@ class NodeInstance {
     const instance = new NodeInstance(
       [], `${scriptContents}\nprocess._rawDebug('started');`, undefined);
     const msg = 'Timed out waiting for process to start';
-    while (await common.fires(instance.nextStderrString(), msg, TIMEOUT) !==
+    while (await fires(instance.nextStderrString(), msg, TIMEOUT) !==
              'started') {}
     process._debugProcess(instance._process.pid);
     return instance;
@@ -431,6 +431,43 @@ function readMainScriptSource() {
   return fs.readFileSync(_MAINSCRIPT, 'utf8');
 }
 
+function onResolvedOrRejected(promise, callback) {
+  return promise.then((result) => {
+    callback();
+    return result;
+  }, (error) => {
+    callback();
+    throw error;
+  });
+}
+
+function timeoutPromise(error, timeoutMs) {
+  let clearCallback = null;
+  let done = false;
+  const promise = onResolvedOrRejected(new Promise((resolve, reject) => {
+    const timeout = setTimeout(() => reject(error), timeoutMs);
+    clearCallback = () => {
+      if (done)
+        return;
+      clearTimeout(timeout);
+      resolve();
+    };
+  }), () => done = true);
+  promise.clear = clearCallback;
+  return promise;
+}
+
+// Returns a new promise that will propagate `promise` resolution or rejection
+// if that happens within the `timeoutMs` timespan, or rejects with `error` as
+// a reason otherwise.
+function fires(promise, error, timeoutMs) {
+  const timeout = timeoutPromise(error, timeoutMs);
+  return Promise.race([
+    onResolvedOrRejected(promise, () => timeout.clear()),
+    timeout
+  ]);
+}
+
 module.exports = {
   mainScriptPath: _MAINSCRIPT,
   readMainScriptSource,