-
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
test-net-connect-options-ipv6 takes way to long on OS X #4546
Comments
This is on your local machine? What happens when you do run it directly from the command line with |
(I ask because it's passing with no issues for me on master on OS X.) |
I just ran |
It is taking 120s on my machine to complete when run manually.
|
For me it returns immediately. Do you know where it's timing out? FWIW the OS X system I'm testing on only has this in /etc/hosts:
It also has (real) IPv6 addresses for lo0 and en0. |
@Fishrock123 im facing a similar issue . |
Using this rough patch for logging: diff --git a/test/parallel/test-net-connect-options-ipv6.js b/test/parallel/test-net-connect-options-ipv6.js
index 8b11612..3c937e7 100644
--- a/test/parallel/test-net-connect-options-ipv6.js
+++ b/test/parallel/test-net-connect-options-ipv6.js
@@ -7,50 +7,59 @@ if (!common.hasIPv6) {
console.log('1..0 # Skipped: no IPv6 support');
return;
}
-
+console.log('1')
const hosts = common.localIPv6Hosts;
var hostIdx = 0;
var host = hosts[hostIdx];
var localhostTries = 10;
const server = net.createServer({allowHalfOpen: true}, function(socket) {
+ console.log('2.1')
socket.resume();
- socket.on('end', common.mustCall(function() {}));
+ socket.on('end', common.mustCall(function() {console.log('2.2')}));
socket.end();
});
server.listen(common.PORT, '::1', tryConnect);
function tryConnect() {
+ console.log('3')
const client = net.connect({
host: host,
port: common.PORT,
family: 6,
allowHalfOpen: true
}, function() {
+ console.log('4')
console.error('client connect cb');
client.resume();
client.on('end', common.mustCall(function() {
+ console.log('4.1')
setTimeout(function() {
+ console.log('4.2')
assert(client.writable);
client.end();
}, 10);
}));
client.on('close', function() {
+ console.log('5')
server.close();
});
}).on('error', function(err) {
+ console.log('6.1')
if (err.syscall === 'getaddrinfo' && err.code === 'ENOTFOUND') {
if (host !== 'localhost' || --localhostTries === 0)
host = hosts[++hostIdx];
- if (host)
+ if (host) {
+ console.log('6.2')
tryConnect();
- else {
+ } else {
console.log('1..0 # Skipped: no IPv6 localhost support');
server.close();
}
return;
}
+ console.log('6.3')
throw err;
});
} The output is:
.... undo the patch, and now it works:
No clue. Each time it re-tries |
@Fishrock123 What do you have in your |
|
@mscdex ping |
@Fishrock123 I added that extra localhost entry you have to the OS X system I'm testing on and it didn't affect anything for me. |
This has persisted for over a month and has made me miss linter errors. Interestingly, this displays when the pause happens in the test runner: |
|
@Fishrock123 This is still a problem for you, right? |
@Trott yes. Very much so. I've missed multiple lint issues due to this. (Maybe lint should run regardless of tests?) |
@Fishrock123 What version of OS X are you running? |
10.10.5 |
+1 to lint running before tests. Or regardless of test failure |
@Fishrock123 Out of curiosity, what if you temporarily restrict your
? |
@Fishrock123 Does this change fix it for you?
|
Have `make test` run linting tools before tests rather than after. Lint is likely to find issues quickly. Tests may take a while to run. So do the linting first. Interestingly, it appears that `vcbuild.bat` is already set up this way on Windows. Refs: nodejs#4546 (comment)
@Fishrock123 wrote:
@evanlucas responded:
If I'm not misreading the file, it appears that PR opened to do the same for |
Do not try Ubuntu/SUSE/Debian-specific hostnames for IPv6 localhost unless we are on Linux. Fixes: nodejs#4546
PR #5471 should resolve this issue, I believe. |
Have `make test` run linting tools before tests rather than after. Lint is likely to find issues quickly. Tests may take a while to run. So do the linting first. Refs: nodejs#4546 (comment) PR-URL: nodejs#5470 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
Have `make test` run linting tools before tests rather than after. Lint is likely to find issues quickly. Tests may take a while to run. So do the linting first. Refs: #4546 (comment) PR-URL: #5470 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
Getting this both on master and v5.x when trying to do #4392
Maybe related to #4325 and/or #4395?
cc @Trott / @mscdex?
The text was updated successfully, but these errors were encountered: