Add basic IPv6 support. #67

elad · 2017-06-20T15:03:15Z

The following changes were made to support pinging IPv6 hosts:

On Linux and macOS, use /sbin/ping6 as the executable
On macOS, if pinging an IPv6 host, don't pass a timeout argument

The following changes were made to support pinging IPv6 hosts: - On Linux and macOS, use `/sbin/ping6` as the executable - On macOS, if pinging an IPv6 host, don't pass a timeout argument

mondwan · 2017-06-20T15:22:43Z

Hello, can you also provide the test cases and fixtures for your ipv6 codes? 2017年6月20日 23:03，"Elad Efrat" <notifications@github.com>寫道：

…

The following changes were made to support pinging IPv6 hosts: - On Linux and macOS, use /sbin/ping6 as the executable - On macOS, if pinging an IPv6 host, don't pass a timeout argument ------------------------------ You can view, comment on, or merge this pull request online at: #67 Commit Summary - Add basic IPv6 support. File Changes - *M* lib/builder/factory.js <https://github.com/danielzzz/node-ping/pull/67/files#diff-0> (6) - *M* lib/builder/mac.js <https://github.com/danielzzz/node-ping/pull/67/files#diff-1> (2) - *M* lib/ping-promise.js <https://github.com/danielzzz/node-ping/pull/67/files#diff-2> (2) Patch Links: - https://github.com/danielzzz/node-ping/pull/67.patch - https://github.com/danielzzz/node-ping/pull/67.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#67>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAoyaDHBzojyCYo2pUWkuny-Ihm3EYks5sF980gaJpZM4N_uGw> .

ajmas · 2017-06-20T16:36:58Z

lib/builder/factory.js

@@ -74,11 +74,11 @@ factory.getExecutablePath = function (platform) {
    if (platform === 'aix') {
        ret = '/usr/sbin/ping';
    } else if (factory.isLinux(platform)) {
-        ret = '/bin/ping';
+        ret = v6 ? '/sbin/ping6' : '/bin/ping';
    } else if (factory.isWindow(platform)) {
        ret = process.env.SystemRoot + '/system32/ping.exe';


Wondering whether we should support the '-6' for windows command, per docs: https://ss64.com/nt/ping.html
BTW I don't have access to to Windows machine to validate this behaviour.

Yeah, we probably should. I can test on Windows. I'll make the code force either IPv4 or IPv6.

(done, on merge we should squash with the latest commit)

Actually, how to create a message box with review icon like this? Any button in github UI for doing this ?

@mondwan click "Files changed" at the top, then hover over the line number and click when you want to add a line-specific comment. :)

The following changes were made to support pinging IPv6 hosts: - On Linux and macOS, use `/sbin/ping6` as the executable - On macOS, if pinging an IPv6 host, don't pass a timeout argument - On Windows, use `-4` and `-6` to force IPv4 and IPv6, respectively

elad · 2017-06-20T16:44:02Z

@mondwan sure, I'll try to get to it later today.

mondwan · 2017-06-20T17:01:39Z

Great. Thanks @elad

mondwan · 2017-06-21T09:54:35Z

Cool. Thanks 2017年6月21日下午5:47，"Elad Efrat" <notifications@github.com>寫道：

…

***@***.**** commented on this pull request. ------------------------------ In lib/builder/factory.js <#67 (comment)>: > @@ -74,11 +74,11 @@ factory.getExecutablePath = function (platform) { if (platform === 'aix') { ret = '/usr/sbin/ping'; } else if (factory.isLinux(platform)) { - ret = '/bin/ping'; + ret = v6 ? '/sbin/ping6' : '/bin/ping'; } else if (factory.isWindow(platform)) { ret = process.env.SystemRoot + '/system32/ping.exe'; @mondwan <https://github.com/mondwan> click "Files changed" at the top, then hover over the line number and click when you want to add a line-specific comment. :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#67 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAo0SJi7VOvnqWKj0bNskACpZyKvKPks5sGOajgaJpZM4N_uGw> .

elad · 2017-06-27T13:57:33Z

@mondwan how do you run the tests?

$ yarn test
yarn test v0.24.6
$ grunt test 
Running "eslint:src" (eslint) task

/private/tmp/node-ping/lib/parser/mac.js
  39:28  error  Unnecessary escape character: \.  no-useless-escape
  56:31  error  Unnecessary escape character: \/  no-useless-escape
  58:28  error  Unnecessary escape character: \.  no-useless-escape

/private/tmp/node-ping/lib/parser/win.js
   83:28  error  Unnecessary escape character: \.  no-useless-escape
  103:28  error  Unnecessary escape character: \.  no-useless-escape

✖ 5 problems (5 errors, 0 warnings)

Warning: Task "eslint:src" failed. Use --force to continue.

Aborted due to warnings.
error Command failed with exit code 3.
$

mondwan · 2017-06-27T14:02:47Z

It seems you are running correct command. Hopefully, you are using stricter eslintrc? 2017年6月27日 21:57，"Elad Efrat" <notifications@github.com>寫道：

…

@mondwan <https://github.com/mondwan> how do you run the tests? $ yarn test yarn test v0.24.6 $ grunt test Running "eslint:src" (eslint) task /private/tmp/node-ping/lib/parser/mac.js 39:28 error Unnecessary escape character: \. no-useless-escape 56:31 error Unnecessary escape character: \/ no-useless-escape 58:28 error Unnecessary escape character: \. no-useless-escape /private/tmp/node-ping/lib/parser/win.js 83:28 error Unnecessary escape character: \. no-useless-escape 103:28 error Unnecessary escape character: \. no-useless-escape ✖ 5 problems (5 errors, 0 warnings) Warning: Task "eslint:src" failed. Use --force to continue. Aborted due to warnings. error Command failed with exit code 3. $ — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#67 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAo9PC8BRHsCA7ngY3Iw54hV66aw0hks5sIQpNgaJpZM4N_uGw> .

elad · 2017-06-27T14:04:03Z

I have no idea. I just cloned the repository, installed the dependencies, and ran the command. Testing on macOS Sierra. What's your environment?

mondwan · 2017-06-27T14:09:55Z

What's the problem? Grunt test states codes in the repository does not meeting the linter's rules. So, you need to edit those lines mentioned until they have passed the rules. After lintering success, testing will be start. 2017年6月27日下午10:04，"Elad Efrat" <notifications@github.com>寫道：

…

I have no idea. I just cloned the repository, installed the dependencies, and ran the command. Testing on macOS Sierra. What's your environment? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#67 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAo6y9fNEdAMd07CWH56nu_stwzXRaks5sIQvTgaJpZM4N_uGw> .

elad · 2017-06-27T14:11:27Z

Those lines are checked out from the repository... they're not related to my changes.

mondwan · 2017-06-27T14:23:10Z

Oops. Hopefully, I have made some mistakes last time. Would you mind to fix them for me? As I am busy at this moment, it may take time for fixing these. 2017年6月27日下午10:12，"Elad Efrat" <notifications@github.com>寫道： Those lines are checked out from the repository... they're not related to my changes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#67 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAoxB-GTZOjQdrVkvWa-6D8_BhGOXbks5sIQ2QgaJpZM4N_uGw> .

mondwan · 2017-07-02T15:48:30Z

Ahh, I just found out linting errors from yours do not happen on my window machine. That's why I cannot spot them out.

refs danielzzz#33 danielzzz#67

elad · 2017-07-02T16:04:33Z

@mondwan you should consider switching to macOS then. :)
Btw, still haven't had the time to write the tests -- hopefully soon because I need this feature in an official release..

mondwan · 2017-07-02T16:05:01Z

@elad , test cases are still missing. Can you provide examples for running with ipv6?

For example,

Add examples/example3.js
- A new example js demonstrates how to use ipv6
Add examples for pinging with ipv6 in system's console
- Just like examples in test/fixture/linux/en/sample1.txt, test/fixture/macos/en/sample1.txt
Add expected answer into test/fixture/answer.json

mondwan · 2017-07-02T16:07:15Z

Yea. Development in Window is bad. Really bad. I will do them on my mac book air if they are heavy.

However, this is a kind of fun for seeing tricky things which happen on window platform only. LOL

ajmas · 2017-07-02T21:06:44Z

Another option is using Vagrant/Virtual Box?

mondwan · 2017-07-03T00:03:29Z

Which one? 2017年7月3日 05:06，"Andre-John Mas" <notifications@github.com>寫道：

…

Another option is using Vagrant/Virtual Box? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#67 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAo2SQqxwlVxMCZNz5dIO8ZVV_wAzMks5sKAZkgaJpZM4N_uGw> .

mondwan · 2018-02-11T12:30:05Z

Hey @elad, may I know the reason for not passing timeout when using ipv6 on mac OS?

Add basic IPv6 support.

53ce04c

The following changes were made to support pinging IPv6 hosts: - On Linux and macOS, use `/sbin/ping6` as the executable - On macOS, if pinging an IPv6 host, don't pass a timeout argument

elad mentioned this pull request Jun 20, 2017

Allow ping to specify IPv4, IPv6 or 'any protocol' #33

Closed

ajmas reviewed Jun 20, 2017

View reviewed changes

Add basic IPv6 support.

bfb515d

The following changes were made to support pinging IPv6 hosts: - On Linux and macOS, use `/sbin/ping6` as the executable - On macOS, if pinging an IPv6 host, don't pass a timeout argument - On Windows, use `-4` and `-6` to force IPv4 and IPv6, respectively

Elad Efrat added 2 commits June 29, 2017 16:09

Fix typo: config -> _config

56bd649

Fix lint errors.

a460b9e

Add documentations for ipv6

989093e

refs danielzzz#33 danielzzz#67

mondwan assigned mondwan and elad Jul 2, 2017

danielzzz merged commit 1601458 into danielzzz:master Jan 25, 2018

mondwan mentioned this pull request Feb 11, 2018

Need help on gathering pinging IPV6 fixtures on different platform #79

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic IPv6 support. #67

Add basic IPv6 support. #67

elad commented Jun 20, 2017

mondwan commented Jun 20, 2017 via email

ajmas Jun 20, 2017

elad Jun 20, 2017

elad Jun 20, 2017

mondwan Jun 20, 2017

elad Jun 21, 2017

elad commented Jun 20, 2017

mondwan commented Jun 20, 2017

mondwan commented Jun 21, 2017 via email

elad commented Jun 27, 2017

mondwan commented Jun 27, 2017 via email

elad commented Jun 27, 2017

mondwan commented Jun 27, 2017 via email

elad commented Jun 27, 2017

mondwan commented Jun 27, 2017 via email

mondwan commented Jul 2, 2017

elad commented Jul 2, 2017

mondwan commented Jul 2, 2017

mondwan commented Jul 2, 2017 •

edited

Loading

ajmas commented Jul 2, 2017

mondwan commented Jul 3, 2017 via email

mondwan commented Feb 11, 2018

Add basic IPv6 support. #67

Add basic IPv6 support. #67

Conversation

elad commented Jun 20, 2017

mondwan commented Jun 20, 2017 via email

ajmas Jun 20, 2017

Choose a reason for hiding this comment

elad Jun 20, 2017

Choose a reason for hiding this comment

elad Jun 20, 2017

Choose a reason for hiding this comment

mondwan Jun 20, 2017

Choose a reason for hiding this comment

elad Jun 21, 2017

Choose a reason for hiding this comment

elad commented Jun 20, 2017

mondwan commented Jun 20, 2017

mondwan commented Jun 21, 2017 via email

elad commented Jun 27, 2017

mondwan commented Jun 27, 2017 via email

elad commented Jun 27, 2017

mondwan commented Jun 27, 2017 via email

elad commented Jun 27, 2017

mondwan commented Jun 27, 2017 via email

mondwan commented Jul 2, 2017

elad commented Jul 2, 2017

mondwan commented Jul 2, 2017

mondwan commented Jul 2, 2017 • edited Loading

ajmas commented Jul 2, 2017

mondwan commented Jul 3, 2017 via email

mondwan commented Feb 11, 2018

mondwan commented Jul 2, 2017 •

edited

Loading