Skip to content

Commit 2eb1aa8

Browse files
Trottevanlucas
authored andcommitted
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: #18096 PR-URL: #17401 Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent 167e9c6 commit 2eb1aa8

File tree

3 files changed

+39
-53
lines changed

3 files changed

+39
-53
lines changed

test/common/README.md

-9
Original file line numberDiff line numberDiff line change
@@ -119,15 +119,6 @@ Tests whether `name` and `expected` are part of a raised warning.
119119

120120
Checks if `pathname` exists
121121

122-
### fires(promise, [error], [timeoutMs])
123-
* promise [&lt;Promise]
124-
* error [&lt;String] default = 'timeout'
125-
* timeoutMs [&lt;Number] default = 100
126-
127-
Returns a new promise that will propagate `promise` resolution or rejection if
128-
that happens within the `timeoutMs` timespan, or rejects with `error` as
129-
a reason otherwise.
130-
131122
### getArrayBufferViews(buf)
132123
* `buf` [&lt;Buffer>]
133124
* return [&lt;ArrayBufferView&#91;&#93;>]

test/common/index.js

-42
Original file line numberDiff line numberDiff line change
@@ -866,32 +866,6 @@ function restoreWritable(name) {
866866
delete process[name].writeTimes;
867867
}
868868

869-
function onResolvedOrRejected(promise, callback) {
870-
return promise.then((result) => {
871-
callback();
872-
return result;
873-
}, (error) => {
874-
callback();
875-
throw error;
876-
});
877-
}
878-
879-
function timeoutPromise(error, timeoutMs) {
880-
let clearCallback = null;
881-
let done = false;
882-
const promise = onResolvedOrRejected(new Promise((resolve, reject) => {
883-
const timeout = setTimeout(() => reject(error), timeoutMs);
884-
clearCallback = () => {
885-
if (done)
886-
return;
887-
clearTimeout(timeout);
888-
resolve();
889-
};
890-
}), () => done = true);
891-
promise.clear = clearCallback;
892-
return promise;
893-
}
894-
895869
exports.hijackStdout = hijackStdWritable.bind(null, 'stdout');
896870
exports.hijackStderr = hijackStdWritable.bind(null, 'stderr');
897871
exports.restoreStdout = restoreWritable.bind(null, 'stdout');
@@ -905,19 +879,3 @@ exports.firstInvalidFD = function firstInvalidFD() {
905879
} catch (e) {}
906880
return fd;
907881
};
908-
909-
exports.fires = function fires(promise, error, timeoutMs) {
910-
if (!timeoutMs && util.isNumber(error)) {
911-
timeoutMs = error;
912-
error = null;
913-
}
914-
if (!error)
915-
error = 'timeout';
916-
if (!timeoutMs)
917-
timeoutMs = 100;
918-
const timeout = timeoutPromise(error, timeoutMs);
919-
return Promise.race([
920-
onResolvedOrRejected(promise, () => timeout.clear()),
921-
timeout
922-
]);
923-
};

test/common/inspector-helper.js

+39-2
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ class InspectorSession {
216216
waitForNotification(methodOrPredicate, description) {
217217
const desc = description || methodOrPredicate;
218218
const message = `Timed out waiting for matching notification (${desc}))`;
219-
return common.fires(
219+
return fires(
220220
this._asyncWaitForNotification(methodOrPredicate), message, TIMEOUT);
221221
}
222222

@@ -323,7 +323,7 @@ class NodeInstance {
323323
const instance = new NodeInstance(
324324
[], `${scriptContents}\nprocess._rawDebug('started');`, undefined);
325325
const msg = 'Timed out waiting for process to start';
326-
while (await common.fires(instance.nextStderrString(), msg, TIMEOUT) !==
326+
while (await fires(instance.nextStderrString(), msg, TIMEOUT) !==
327327
'started') {}
328328
process._debugProcess(instance._process.pid);
329329
return instance;
@@ -431,6 +431,43 @@ function readMainScriptSource() {
431431
return fs.readFileSync(_MAINSCRIPT, 'utf8');
432432
}
433433

434+
function onResolvedOrRejected(promise, callback) {
435+
return promise.then((result) => {
436+
callback();
437+
return result;
438+
}, (error) => {
439+
callback();
440+
throw error;
441+
});
442+
}
443+
444+
function timeoutPromise(error, timeoutMs) {
445+
let clearCallback = null;
446+
let done = false;
447+
const promise = onResolvedOrRejected(new Promise((resolve, reject) => {
448+
const timeout = setTimeout(() => reject(error), timeoutMs);
449+
clearCallback = () => {
450+
if (done)
451+
return;
452+
clearTimeout(timeout);
453+
resolve();
454+
};
455+
}), () => done = true);
456+
promise.clear = clearCallback;
457+
return promise;
458+
}
459+
460+
// Returns a new promise that will propagate `promise` resolution or rejection
461+
// if that happens within the `timeoutMs` timespan, or rejects with `error` as
462+
// a reason otherwise.
463+
function fires(promise, error, timeoutMs) {
464+
const timeout = timeoutPromise(error, timeoutMs);
465+
return Promise.race([
466+
onResolvedOrRejected(promise, () => timeout.clear()),
467+
timeout
468+
]);
469+
}
470+
434471
module.exports = {
435472
mainScriptPath: _MAINSCRIPT,
436473
readMainScriptSource,

0 commit comments

Comments
 (0)