Fixes #3497 #3500

Closed
wants to merge 11 commits into
from

Projects

None yet

4 participants

@hutkev
hutkev commented Jun 20, 2012

This iterates all interfaces to locate an IPv4 loopback rather than assuming fixed interface names.
Tested on OS X 10.6.8, CentOS 6.2 (64-bit) & Win XP SP2 (64-bit).

@bnoordhuis bnoordhuis commented on an outdated diff Jun 21, 2012
test/simple/test-os.js
- var actual = interfaces.lo.filter(filter);
- var expected = [{ address: '127.0.0.1', family: 'IPv4', internal: true }];
- assert.deepEqual(actual, expected);
- break;
- case 'win32':
- var filter = function(e) { return e.address == '127.0.0.1'; };
- var actual = interfaces['Loopback Pseudo-Interface 1'].filter(filter);
- var expected = [{ address: '127.0.0.1', family: 'IPv4', internal: true }];
- assert.deepEqual(actual, expected);
- break;
+var filter = function(e) { return e.address == '127.0.0.1'; };
+var expected = { address: '127.0.0.1', family: 'IPv4', internal: true };
+var address = function(a) { assert.deepEqual(a, expected); return true; };
+var count = 0;
+for (var i in interfaces) {
+ var addr=interfaces[i].filter(filter).filter(address);
@bnoordhuis
bnoordhuis Jun 21, 2012 Member

Missing whitespace.

@bnoordhuis
bnoordhuis Jun 21, 2012 Member

Also, I prefer an iterative approach over a functional one in tests. It's easier to debug when something goes wrong.

@hutkev
hutkev commented Jun 22, 2012

Thanks Ben.

I made an unrelated change to fix a jslint warning, assume OK to include these in passing?

Kev

@bnoordhuis
Member

Kevin, can you rebase against the latest master? The PR doesn't apply cleanly right now.

@hutkev
hutkev commented Jun 23, 2012

Updated and removed a bit of redundant code. Thanks.

@bnoordhuis
Member

Kevin, it's still not applying. Can you do a git pull --rebase git://github.com/joyent/node.git master, fix the conflicts and squash your work into one or two commits with git rebase -i?

@hutkev
hutkev commented Jun 30, 2012

Sorry Ben, looks like I created a false conflict swapping between machines. I have tried to correct in westboost/node@74b60fa. If this is no good let's close this and I will clean up my fork and re-submit with some other test fixes I have.

@bnoordhuis
Member

There's still a lot of noise. If you can either rebase or submit again, that'd be great.

@Nodejs-Jenkins

Can one of the admins verify this patch?

@chrisdickinson

It looks like the corresponding issue has since been closed. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment