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

Debugger tests are timing out on v4.x #10361

Closed
MylesBorins opened this issue Dec 20, 2016 · 2 comments
Closed

Debugger tests are timing out on v4.x #10361

MylesBorins opened this issue Dec 20, 2016 · 2 comments
Labels
test Issues and PRs related to the tests.

Comments

@MylesBorins
Copy link
Member

  • Version: v4.x-staging
  • Platform: OSX
  • Subsystem: test / debugger
$ python tools/test.py debugger

Trying to run these tests causes a variety of timeouts.
@mscdex mscdex added debugger test Issues and PRs related to the tests. labels Dec 20, 2016
@Trott
Copy link
Member

Trott commented Dec 21, 2016 •
edited

Debugger tests have been broken on all branches, including master, for literally longer than you or I have been on the project. There was a period of time when I was trying to fix them one at a time and move them to parallel. But it seemed like there were better uses of my time.

On the upside, debugger is going away sooner or later, so...
¯\(ツ)/¯

Hopefully we can have robust inspect tests going forward. :-)

@thefourtheye thefourtheye mentioned this issue Dec 21, 2016
Closed
2 tasks
@thefourtheye thefourtheye mentioned this issue Dec 21, 2016
Closed
1 task
@thefourtheye
Copy link
Contributor

I created #10370 and #10371. I think they fix the debugger test suite.

thefourtheye added a commit to thefourtheye/io.js that referenced this issue Dec 21, 2016
@thefourtheye
test: fix and improve debug-break-on-uncaught
0010e62 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refer: nodejs#10361
thefourtheye added a commit to thefourtheye/io.js that referenced this issue Dec 21, 2016
@thefourtheye
test: fix and improve debugger-client test
2df054b 
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
jasnell pushed a commit that referenced this issue Dec 24, 2016
@thefourtheye @jasnell
test: fix and improve debug-break-on-uncaught
c74d9d6 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refs: #10361
PR-URL: #10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
thefourtheye added a commit to thefourtheye/io.js that referenced this issue Dec 26, 2016
@thefourtheye
test: fix and improve debugger-client test
3ef4ec0 
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 <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@thefourtheye thefourtheye mentioned this issue Dec 26, 2016
Closed
2 tasks
@thefourtheye thefourtheye mentioned this issue Dec 26, 2016
Closed
2 tasks
thefourtheye added a commit to thefourtheye/io.js that referenced this issue Dec 28, 2016
@thefourtheye
test: refactor test-debugger-remote
a905e57 
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
thefourtheye added a commit that referenced this issue Jan 2, 2017
@thefourtheye
test: refactor test-debugger-remote
152bd82 
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: #10361
PR-URL: #10455

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
evanlucas pushed a commit that referenced this issue Jan 3, 2017
@thefourtheye @evanlucas
test: fix and improve debug-break-on-uncaught
44c340c 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refs: #10361
PR-URL: #10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
evanlucas pushed a commit that referenced this issue Jan 3, 2017
@thefourtheye @evanlucas
test: fix and improve debugger-client test
ddaa2a7 
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: #10361

PR-URL: #10371

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this issue Jan 3, 2017
@thefourtheye @evanlucas
test: refactor test-debugger-remote
f112a19 
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: #10361
PR-URL: #10455

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
evanlucas pushed a commit that referenced this issue Jan 4, 2017
@thefourtheye @evanlucas
test: fix and improve debug-break-on-uncaught
d33e560 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refs: #10361
PR-URL: #10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
evanlucas pushed a commit that referenced this issue Jan 4, 2017
@thefourtheye @evanlucas
test: fix and improve debugger-client test
94a894a 
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: #10361

PR-URL: #10371

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this issue Jan 4, 2017
@thefourtheye @evanlucas
test: refactor test-debugger-remote
05b0092 
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: #10361
PR-URL: #10455

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
@thefourtheye @MylesBorins
test: fix and improve debug-break-on-uncaught
4743066 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refs: #10361
PR-URL: #10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
@thefourtheye @MylesBorins
test: fix and improve debug-break-on-uncaught
f80084c 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refs: #10361
PR-URL: #10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@thefourtheye @MylesBorins
test: fix and improve debug-break-on-uncaught
3210653 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refs: #10361
PR-URL: #10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@thefourtheye @MylesBorins
test: fix and improve debug-break-on-uncaught
6bb735a 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refs: #10361
PR-URL: #10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
thefourtheye added a commit to thefourtheye/io.js that referenced this issue Jan 30, 2017
@thefourtheye
build: enable debugger tests in CI
e6b277c 
All the problems blocking nodejs#10361
have been resolved. This patch enables debugger in CI.
MylesBorins pushed a commit that referenced this issue Jan 31, 2017
@thefourtheye @MylesBorins
test: fix and improve debug-break-on-uncaught
5f18f0c 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refs: #10361
PR-URL: #10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
MylesBorins pushed a commit that referenced this issue Feb 1, 2017
@thefourtheye @MylesBorins
test: fix and improve debug-break-on-uncaught
dbfec29 
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.

Apart from that, changed `var`s to `let`s or `const`s.

Refs: #10361
PR-URL: #10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
thefourtheye added a commit to thefourtheye/io.js that referenced this issue Feb 5, 2017
@thefourtheye
test: fix and improve debugger-client test
8f61ca9 
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 <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
thefourtheye added a commit to thefourtheye/io.js that referenced this issue Feb 5, 2017
@thefourtheye
test: refactor test-debugger-remote
fd04f9a 
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 <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this issue Mar 8, 2017
@thefourtheye @MylesBorins
test: fix and improve debugger-client test
25963ec 
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: #10361

PR-URL: #10371

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 8, 2017
@thefourtheye @MylesBorins
test: refactor test-debugger-remote
6966a58 
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: #10361
PR-URL: #10455

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this issue Mar 9, 2017
@thefourtheye @MylesBorins
test: fix and improve debugger-client test
34e0bc6 
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: #10361

PR-URL: #10371

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 9, 2017
@thefourtheye @MylesBorins
test: refactor test-debugger-remote
ae338da 
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: #10361
PR-URL: #10455

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mscdex @MylesBorins @thefourtheye @Trott