Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Backport debugger test fixes v6.x #11184

Closed
wants to merge 4 commits into from
Closed

Backport debugger test fixes v6.x #11184

wants to merge 4 commits into from

Commits on Feb 5, 2017

  1. test: fix and improve debugger-client test 

    This test expects the string 'Debugger listening on port' on stderr and
since the message has been changed to 'Debugger listening on host:port'
this was failing always.

Apart from that, this test expects the main script's name to be
`src/node.js`, but that has been renamed to `lib/internal/node.js` and
then to `lib/internal/bootstrap_node.js`. So, the script name has been
updated.

Apart from that, using `const` instead of `var` wherever possible.

Refer: nodejs#10361

PR-URL: nodejs#10371

Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
    @thefourtheye
    thefourtheye committed Feb 5, 2017
    Configuration menu
    Copy the full SHA
    8f61ca9 View commit details
    Browse the repository at this point in the history

  2. test: refactor test-debugger-remote 

    1. The test doesn't attach an event listener for `exit` events and
   removes them before killing. The intention is to fail the tests if
   the processes exit normally. This patch attaches the `exit` event
   handlers.

2. Replace `var`s with `let`s and `const`s.

3. Replace `==` based assertion with `strictEqual` assertion.

4. Use `common.PORT` instead of `5959`.

5. The test used to expect only one string "connecting to
   localhost:5959 ... ok", but the debugger actually emits another
   string, "break in test/fixtures/empty.js:2". This patch asserts if
   both of them are received in the same order.

Refer: nodejs#10361
PR-URL: nodejs#10455

Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
    @thefourtheye
    thefourtheye committed Feb 5, 2017
    Configuration menu
    Copy the full SHA
    fd04f9a View commit details
    Browse the repository at this point in the history

  3. test: fix process.title expectation 

    `process.title` would work properly only in FreeBSD, OSX, and Linux as
per test/parallel/test-setproctitle.js.

This patch makes sure that the test expects an empty string in other
platforms.

This patch helps fix the SmartOS failures in
https://ci.nodejs.org/job/node-test-commit/6962/ for
nodejs#10456

PR-URL: nodejs#10597
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
    @thefourtheye
    thefourtheye committed Feb 5, 2017
    Configuration menu
    Copy the full SHA
    94cbec6 View commit details
    Browse the repository at this point in the history

  4. test: increase timeout in break-on-uncaught 

    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 polls every 10 ms to see if the exception is received.

PR-URL: nodejs#10822

Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
    @thefourtheye
    thefourtheye committed Feb 5, 2017
    Configuration menu
    Copy the full SHA
    4ea2f2a View commit details
    Browse the repository at this point in the history