-
Notifications
You must be signed in to change notification settings - Fork 30.7k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
sequential/test-debug-host-port hanging when the port is not available #11536
Comments
Doing something like this fixes the issue for me: diff --git a/test/sequential/test-debug-host-port.js b/test/sequential/test-debug-host-port.js
index 88ce7bc..964774f 100644
--- a/test/sequential/test-debug-host-port.js
+++ b/test/sequential/test-debug-host-port.js
@@ -14,7 +14,8 @@ function test(args, needle) {
proc.stderr.setEncoding('utf8');
proc.stderr.on('data', (data) => {
stderr += data;
- if (stderr.includes(needle)) proc.kill();
+ if (stderr.includes(needle) || stderr.includes('Error:'))
+ proc.kill();
});
proc.on('exit', common.mustCall(() => {
assert(stderr.includes(needle)); What I don't know if it's correct behavior that calling |
Yes and no, see https://github.com/nodejs/node/blob/v7.6.0/lib/internal/bootstrap_node.js#L404-L418. It's intentional but it's a hack. EDIT: Also https://github.com/nodejs/node/blob/v7.6.0/lib/module.js#L549-L565 |
@bnoordhuis are you okay with @santigimeno's proposed fix? If so @santigimeno would you mind PRing? |
test/sequential/test-debug-host-port.js was removed in #12197 so this is no longer an issue. I'll close it out. |
@bnoordhuis sure, but this is failing for us on v6.x (and v4.x, but there's not much we can do about it at this point). |
Oh, right. Then PR'ing against v6 is a good idea. |
There is almost no time left for supporting v6.x so I'm just going to close this out. If someone wants to re-open this, perhaps consider following up with a way to progress this. |
We are getting an error when running this test in that if the port is in use, the test will just hang and never fail. I see two issues here:
The text was updated successfully, but these errors were encountered: