Merge pull request #101 from sa-linetco/master

use timeout option when timeout config option is set

README.md

@@ -91,7 +91,13 @@ Below is the possible configuration
  * Cross platform config representation
  * @typedef {Object} PingConfig
  * @property {boolean} numeric - Map IP address to hostname or not
- * @property {number} timeout - Time duration, in seconds, for ping command to exit
+ * @property {number} timeout - Timeout in seconds for each ping request.
+ * Behaviour varies between platforms. Check platform ping documentation for more information.
+ * @property {number} deadline - Specify a timeout, in seconds, before ping exits regardless of
+              how many packets have been sent or received. In this case ping
+              does not stop after count packet are sent, it waits either for
+              deadline expire or until count probes are answered or for some
+              error notification from network. This option is only available on linux and mac.
  * @property {number} min_reply - Exit after sending number of ECHO_REQUEST
  * @property {boolean} v6 - Ping via ipv6 or not. Default is false
  * @property {string} sourceAddr - source address for sending the ping
@@ -149,6 +155,8 @@ without corresponding arguments.
 
 Try to install package `iputils`. For example, running `apk add iputils`
 
+* For questions regarding to the implementation of `timeout`, and `deadline`, please checkout discussions in #101
+
 # Contributing
 
 Before opening a pull request please make sure your changes follow the

lib/builder/linux.js

@@ -12,7 +12,15 @@ var builder = {};
  * Cross platform config representation
  * @typedef {Object} PingConfig
  * @property {boolean} numeric - Map IP address to hostname or not
- * @property {number} timeout - Time duration for ping command to exit
+ * @property {number} timeout - Time to wait for a response, in seconds.
+ * The option affects only timeout  in  absence  of any responses,
+ * otherwise ping waits for two RTTs.
+ * @property {number} deadline - Specify a timeout, in seconds,
+ * before ping exits regardless of how many packets have been sent or received.
+ * In this case ping does not stop after count packet are sent,
+ * it waits either for deadline expire or until count probes are answered
+ * or for some error notification from network.
+ * This option is only available on linux and mac.
  * @property {number} min_reply - Exit after sending number of ECHO_REQUEST
  * @property {boolean} v6 - Use IPv4 (default) or IPv6
  * @property {string} sourceAddr - source address for sending the ping
@@ -22,6 +30,7 @@ var builder = {};
 var defaultConfig = {
     numeric: true,
     timeout: 2,
+    deadline: false,
     min_reply: 1,
     v6: false,
     sourceAddr: '',
@@ -41,9 +50,10 @@ builder.getResult = function (target, config) {
     var ret = [];
 
     // Make every key in config has been setup properly
-    var keys = ['numeric', 'timeout', 'min_reply', 'v6', 'sourceAddr', 'extra'];
+    var keys = ['numeric', 'timeout', 'deadline', 'min_reply', 'v6',
+        'sourceAddr', 'extra'];
     keys.forEach(function (k) {
-        // Falsy value will be overrided without below checking
+        // Falsy value will be overridden without below checking
         if (typeof(_config[k]) !== 'boolean') {
             _config[k] = _config[k] || defaultConfig[k];
         }
@@ -55,11 +65,18 @@ builder.getResult = function (target, config) {
 
     if (_config.timeout) {
         ret = ret.concat([
-            '-w',
+            '-W',
             util.format('%d', _config.timeout),
         ]);
     }
 
+    if (_config.deadline) {
+        ret = ret.concat([
+            '-w',
+            util.format('%d', _config.deadline),
+        ]);
+    }
+
     if (_config.min_reply) {
         ret = ret.concat([
             '-c',

lib/builder/mac.js

@@ -12,7 +12,15 @@ var builder = {};
  * Cross platform config representation
  * @typedef {Object} PingConfig
  * @property {boolean} numeric - Map IP address to hostname or not
- * @property {number} timeout - Time duration for ping command to exit
+ * @property {number} timeout - Time to wait for a response, in seconds.
+ * The option affects only timeout  in  absence  of any responses,
+ * otherwise ping waits for two RTTs.
+ * @property {number} deadline - Specify a timeout, in seconds,
+ * before ping exits regardless of how many packets have been sent or received.
+ * In this case ping does not stop after count packet are sent,
+ * it waits either for deadline expire or until count probes are answered
+ * or for some error notification from network.
+ * This option is only available on linux and mac.
  * @property {number} min_reply - Exit after sending number of ECHO_REQUEST
  * @property {boolean} v6 - Use IPv4 (default) or IPv6
  * @property {string} sourceAddr - source address for sending the ping
@@ -22,6 +30,7 @@ var builder = {};
 var defaultConfig = {
     numeric: true,
     timeout: 2,
+    deadline: false,
     min_reply: 1,
     v6: false,
     sourceAddr: '',
@@ -42,9 +51,10 @@ builder.getResult = function (target, config) {
     var ret = [];
 
     // Make every key in config has been setup properly
-    var keys = ['numeric', 'timeout', 'min_reply', 'v6', 'sourceAddr', 'extra'];
+    var keys = ['numeric', 'timeout', 'deadline', 'min_reply', 'v6',
+        'sourceAddr', 'extra'];
     keys.forEach(function (k) {
-        // Falsy value will be overrided without below checking
+        // Falsy value will be overridden without below checking
         if (typeof(_config[k]) !== 'boolean') {
             _config[k] = _config[k] || defaultConfig[k];
         }
@@ -60,9 +70,16 @@ builder.getResult = function (target, config) {
             throw new Error('There is no timeout option on ping6');
         }
 
+        ret = ret.concat([
+            '-W',
+            util.format('%d', _config.timeout * 1000),
+        ]);
+    }
+
+    if (_config.deadline) {
         ret = ret.concat([
             '-t',
-            util.format('%d', _config.timeout),
+            util.format('%d', _config.deadline),
         ]);
     }
 

lib/builder/win.js

@@ -12,7 +12,7 @@ var builder = {};
  * Cross platform config representation
  * @typedef {Object} PingConfig
  * @property {boolean} numeric - Map IP address to hostname or not
- * @property {number} timeout - Time duration for ping command to exit
+ * @property {number} timeout - Timeout in seconds for each individual ping request
  * @property {number} min_reply - Exit after sending number of ECHO_REQUEST
  * @property {boolean} v6 - Use IPv4 (default) or IPv6
  * @property {string} sourceAddr - source address for sending the ping
@@ -65,6 +65,10 @@ builder.getResult = function (target, config) {
         ]);
     }
 
+    if (_config.deadline) {
+        throw new Error('There is no deadline option on windows');
+    }
+
     if (_config.min_reply) {
         ret = ret.concat([
             '-n',

test/load-fixture-path.js

@@ -55,7 +55,7 @@ module.exports = function (platform) {
         '**',
         '*.txt',
     ]);
-	targetDirectory = path.posix.join.apply(path.posix, targetDirectory);
+    targetDirectory = path.posix.join.apply(path.posix, targetDirectory);
 
     return glob.sync(targetDirectory);
 };

test/test-ping.js

@@ -1,4 +1,5 @@
 'use strict';
+
 /* global describe it before after*/
 /* eslint no-unused-expressions: 0 */
 
@@ -17,6 +18,7 @@ var ping = require('..');
 
 // Some constants
 var ANSWER = require('./fixture/answer');
+
 var PLATFORMS = [
     'window',
     'darwin',
@@ -117,6 +119,86 @@ var createTestCase = function (platform, pingExecution) {
     });
 };
 
+describe('ping timeout and deadline options', function () {
+    describe('on linux platform', function () {
+        beforeEach(function () {
+            this.platformStub = sinon.stub(os, 'platform',
+                function () { return 'linux'; });
+            const fixturePath = path.join(__dirname, 'fixture',
+                'linux', 'en', 'sample1.txt');
+            this.spawnStub = sinon.stub(cp, 'spawn', mockOutSpawn(fixturePath));
+        });
+
+        afterEach(function () {
+            this.platformStub.restore();
+            this.spawnStub.restore();
+        });
+
+        it('are forwarded to the ping binary', function () {
+            return ping.promise.probe('whatever', {
+                timeout: 47,
+                deadline: 83,
+            }).then(function () {
+                const spawnArgs = this.spawnStub.getCalls()[0].args;
+                const pingArgs = spawnArgs[1];
+                expect(pingArgs[pingArgs.indexOf('-W') + 1]).to.equal('47');
+                expect(pingArgs[pingArgs.indexOf('-w') + 1]).to.equal('83');
+            }.bind(this));
+        });
+    });
+
+    describe('on windows platform', function () {
+        beforeEach(function () {
+            this.platformStub = sinon.stub(os, 'platform',
+                function () { return 'window'; });
+            const fixturePath = path.join(__dirname, 'fixture',
+                'window', 'en', 'sample1.txt');
+            this.spawnStub = sinon.stub(cp, 'spawn', mockOutSpawn(fixturePath));
+        });
+
+        afterEach(function () {
+            this.platformStub.restore();
+            this.spawnStub.restore();
+        });
+
+        it('results in an error as deadline is not supported', function () {
+            return ping.promise.probe('whatever', {
+                timeout: 47,
+                deadline: 83,
+            }).then(function () {
+                throw new Error('deadline should result in an error');
+            }).catch(function () {});
+        });
+    });
+
+    describe('on mac platform', function () {
+        beforeEach(function () {
+            this.platformStub = sinon.stub(os, 'platform',
+                function () { return 'freebsd'; });
+            const fixturePath = path.join(__dirname, 'fixture',
+                'macos', 'en', 'sample1.txt');
+            this.spawnStub = sinon.stub(cp, 'spawn', mockOutSpawn(fixturePath));
+        });
+
+        afterEach(function () {
+            this.platformStub.restore();
+            this.spawnStub.restore();
+        });
+
+        it('are forwarded to the ping binary', function () {
+            return ping.promise.probe('whatever', {
+                timeout: 47,
+                deadline: 83,
+            }).then(function () {
+                const spawnArgs = this.spawnStub.getCalls()[0].args;
+                const pingArgs = spawnArgs[1];
+                expect(pingArgs[pingArgs.indexOf('-W') + 1]).to.equal('4700');
+                expect(pingArgs[pingArgs.indexOf('-t') + 1]).to.equal('83');
+            }.bind(this));
+        });
+    });
+});
+
 describe('Ping in callback mode', function () {
     var pingExecution = function (fp, args) {
         var deferred = q.defer();