test: move common.fires() to inspector-helper

[Trott]

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.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

test

[maclover7]

LGTM

[benjamingr]

Actually, this code for timeoutPromise is a little convoluted IMO - any reason we can't do.

const promisify = require('util').promisify;
const delay = promisify(setTimeout);
const timeoutPromise = async (error, timeoutMs, cancelToken) => {
  await delay(ms);
  if(cancelToken.cancelled) { 
    return;
  }
  throw error;
};

Turns fires to:

function fires(promise, error, timeoutMs) {
  var token = {};
  return Promise.race([
     onResolvedOrRejected(promise, () => token.cancelled = true),
     timeoutPromise(error, timeoutMs, token)
   ]);
 }

If we want to preserve the onResolvedOnRejected behavior (which I'm not sure I understand) we can just use Promise.prototype.finally which is in V8 6.3

[Trott]

No issues with further refactoring, but I'd prefer to do that in a separate PR.

[benjamingr]

Current code is a little complex IMO but actual changes LGTM

[Trott]

CI: https://ci.nodejs.org/job/node-test-pull-request/11840/

[Trott]

Landed in e1054ab

@benjamingr Would you be willing/interested to file a PR for the further refactoring you suggested? I'm happy to do it, but it seems right in your wheelhouse. (Alternatively, maybe you want to stash this one away for your next Code-and-Learn event?)

[MylesBorins]

This does not land cleanly on v9.x

Could you please backport!

[Trott]

Backported in #18096