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

test: fix helper-debugger-repl.js #9486

Closed
wants to merge 1 commit into from
Closed

test: fix helper-debugger-repl.js #9486

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 6, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test debugger

Description of change

The test debugger/test-debugger-repl-break-in-module (and probably
others) was failing because the handshake message for debugging is no
longer listening on port <port> but is instead listening on <address>:<port>.

This change makes the check less strict so as to hopefully future-proof
it at least a little bit against subsequent changes.

This test failure is not caught in CI because currently debugger tests
are not run in CI.

@Trott
test: fix helper-debugger-repl.js
72640f5 
The test `debugger/test-debugger-repl-break-in-module` (and probably
others) was failing because the handshake message for debugging is no
longer `listening on port <port>` but is instead `listening on
<address>:<port>`.

This change makes the check less strict so as to hopefully future-proof
it at least a little bit against subsequent changes.

This test failure is not caught in CI because currently debugger tests
are not run in CI.
@Trott Trott added debugger test Issues and PRs related to the tests. labels Nov 6, 2016
@Trott
Copy link
Member Author

Trott commented Nov 6, 2016

@nodejs/testing

cjihrig
cjihrig approved these changes Nov 6, 2016
View reviewed changes
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I forget - why are the debugger tests not run on the CI? Is it because they aren't in good enough shape?

@Trott
Copy link
Member Author

Trott commented Nov 6, 2016

why are the debugger tests not run on the CI? Is it because they aren't in good enough shape?

Yes.

Trott added a commit to Trott/io.js that referenced this pull request Nov 10, 2016
@Trott
test: fix helper-debugger-repl.js
1323db0 
The test `debugger/test-debugger-repl-break-in-module` (and probably
others) was failing because the handshake message for debugging is no
longer `listening on port <port>` but is instead `listening on
<address>:<port>`.

This change makes the check less strict so as to hopefully future-proof
it at least a little bit against subsequent changes.

This test failure is not caught in CI because currently debugger tests
are not run in CI.

PR-URL: nodejs#9486
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
@Trott
Copy link
Member Author

Trott commented Nov 10, 2016

Landed in 1323db0

@Trott Trott closed this Nov 10, 2016
addaleax pushed a commit that referenced this pull request Nov 22, 2016
@Trott @addaleax
test: fix helper-debugger-repl.js
ee76069 
The test `debugger/test-debugger-repl-break-in-module` (and probably
others) was failing because the handshake message for debugging is no
longer `listening on port <port>` but is instead `listening on
<address>:<port>`.

This change makes the check less strict so as to hopefully future-proof
it at least a little bit against subsequent changes.

This test failure is not caught in CI because currently debugger tests
are not run in CI.

PR-URL: #9486
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
@Trott @addaleax
test: fix helper-debugger-repl.js
e97c610 
The test `debugger/test-debugger-repl-break-in-module` (and probably
others) was failing because the handshake message for debugging is no
longer `listening on port <port>` but is instead `listening on
<address>:<port>`.

This change makes the check less strict so as to hopefully future-proof
it at least a little bit against subsequent changes.

This test failure is not caught in CI because currently debugger tests
are not run in CI.

PR-URL: nodejs#9486
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@Trott @MylesBorins
test: fix helper-debugger-repl.js
13cace1 
The test `debugger/test-debugger-repl-break-in-module` (and probably
others) was failing because the handshake message for debugging is no
longer `listening on port <port>` but is instead `listening on
<address>:<port>`.

This change makes the check less strict so as to hopefully future-proof
it at least a little bit against subsequent changes.

This test failure is not caught in CI because currently debugger tests
are not run in CI.

PR-URL: #9486
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@Trott @MylesBorins
test: fix helper-debugger-repl.js
28e33f3 
The test `debugger/test-debugger-repl-break-in-module` (and probably
others) was failing because the handshake message for debugging is no
longer `listening on port <port>` but is instead `listening on
<address>:<port>`.

This change makes the check less strict so as to hopefully future-proof
it at least a little bit against subsequent changes.

This test failure is not caught in CI because currently debugger tests
are not run in CI.

PR-URL: #9486
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@Trott @MylesBorins
test: fix helper-debugger-repl.js
36320f6 
The test `debugger/test-debugger-repl-break-in-module` (and probably
others) was failing because the handshake message for debugging is no
longer `listening on port <port>` but is instead `listening on
<address>:<port>`.

This change makes the check less strict so as to hopefully future-proof
it at least a little bit against subsequent changes.

This test failure is not caught in CI because currently debugger tests
are not run in CI.

PR-URL: #9486
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
This was referenced Dec 21, 2016
Merged
Merged
@Trott Trott deleted the fix-debug-test branch January 13, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JungMinu JungMinu JungMinu approved these changes

@cjihrig cjihrig cjihrig approved these changes

@princejwesley princejwesley princejwesley approved these changes

@santigimeno santigimeno santigimeno approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@Trott @jasnell @santigimeno @princejwesley @cjihrig @JungMinu @MylesBorins @addaleax