test: increase timeout in break-on-uncaught

[thefourtheye]

As the failures suggest, this test expects the uncaught exception to
be thrown within 100 milliseconds, but on some of the test machines it
takes longer than that limit to notify the exception. Thats why the
test was failing.

This patch increases the timeout to three seconds, with platform
specific timeout value.

Checklist

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

Affected core subsystem(s)

test

---

Refer: #10456

[Trott]

/cc @nodejs/testing

Might it be better to use a short setInterval() (or a setTimeout() that renews itself if the thing hasn't paused yet) and let the test time out via test.py rather than having an AssertionError here?

[Trott]

(That will also have the benefit of not requiring a minimum of 6 seconds for the test to run.)

[Trott]

Maybe like this?:

...
  let interval;
  function runTest(client) {
    client.req(
      {
        command: 'setexceptionbreak',
        arguments: {
          type: 'uncaught',
          enabled: true
        }
      },
      function(error) {
        assert.ifError(error);

        client.on('exception', function(event) {
          exceptions.push(event.body);
        });

        client.reqContinue(function(error) {
          assert.ifError(error);
          interval = setInterval(assertHasPaused.bind(null, client), 1);
        });
      }
    );
  }

  function assertHasPaused(client) {
    if (exceptions.length > 0) {
      assert.strictEqual(exceptions.length, 1,
                         'debugger did not pause on exception');
      assert.strictEqual(exceptions[0].uncaught, true);
      assert.strictEqual(exceptions[0].script.name, testScript);
      assert.strictEqual(exceptions[0].sourceLine + 1, throwsOnLine);
      asserted = true;
      client.reqContinue(assert.ifError);
      clearInterval(interval);
    }
  }
}

[thefourtheye]

It makes sense. Tested locally as well. Thanks :-)

[thefourtheye]

@Trott Does this LGTY?

[Trott]

If I'm not mistaken, it's impossible for this assertion to fire given the check two lines above. So this line can probably be removed?

[Trott]

Left a nit question/comment, but either way, LGTM if linter is green.

[thefourtheye]

@Trott I updated the PR with your suggestion. I'll land this today, if you have no objections.

[thefourtheye]

Landed in cc899a4