From 043b3da2bfba466f43424061781b4f94d3f09367 Mon Sep 17 00:00:00 2001 From: sa-linetco Date: Thu, 22 Aug 2019 12:42:16 +0200 Subject: [PATCH 1/6] use timeout option when timeout config option is set --- lib/builder/linux.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/builder/linux.js b/lib/builder/linux.js index 004839b..baa13d7 100644 --- a/lib/builder/linux.js +++ b/lib/builder/linux.js @@ -55,7 +55,7 @@ builder.getResult = function (target, config) { if (_config.timeout) { ret = ret.concat([ - '-w', + '-W', util.format('%d', _config.timeout), ]); } From 4e60728011218c702a218a73d40e3c99aa42c55b Mon Sep 17 00:00:00 2001 From: sa-linetco Date: Fri, 23 Aug 2019 11:25:02 +0200 Subject: [PATCH 2/6] add support for the deadline option on linux platform --- README.md | 5 +++ lib/builder/linux.js | 20 +++++++++- lib/builder/mac.js | 4 ++ lib/builder/win.js | 4 ++ package-lock.json | 2 +- test/load-fixture-path.js | 2 +- test/test-ping.js | 79 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 112 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index d13f4c2..820e024 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,11 @@ Below is the possible configuration * @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} 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. * @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 diff --git a/lib/builder/linux.js b/lib/builder/linux.js index baa13d7..07439c7 100644 --- a/lib/builder/linux.js +++ b/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. * @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: '', @@ -43,7 +52,7 @@ builder.getResult = function (target, config) { // Make every key in config has been setup properly var keys = ['numeric', 'timeout', '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,6 +69,13 @@ builder.getResult = function (target, config) { ]); } + if (_config.deadline) { + ret = ret.concat([ + '-w', + util.format('%d', _config.deadline), + ]); + } + if (_config.min_reply) { ret = ret.concat([ '-c', diff --git a/lib/builder/mac.js b/lib/builder/mac.js index 73ecf8c..7c922ee 100644 --- a/lib/builder/mac.js +++ b/lib/builder/mac.js @@ -66,6 +66,10 @@ builder.getResult = function (target, config) { ]); } + if (_config.deadline) { + throw new Error('There is no deadline option on mac'); + } + if (_config.min_reply) { ret = ret.concat([ '-c', diff --git a/lib/builder/win.js b/lib/builder/win.js index a63783a..8d6a5d2 100644 --- a/lib/builder/win.js +++ b/lib/builder/win.js @@ -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', diff --git a/package-lock.json b/package-lock.json index 4aa5b6b..676060a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "ping", - "version": "0.2.2", + "version": "0.2.3", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/test/load-fixture-path.js b/test/load-fixture-path.js index a4ed5c9..ae988e1 100644 --- a/test/load-fixture-path.js +++ b/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); }; diff --git a/test/test-ping.js b/test/test-ping.js index 9d392a0..25728b5 100644 --- a/test/test-ping.js +++ b/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,83 @@ 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('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('Ping in callback mode', function () { var pingExecution = function (fp, args) { var deferred = q.defer(); From c0d6fd5c34cff9c0063d614b1a589e4e125d45e2 Mon Sep 17 00:00:00 2001 From: sa-linetco Date: Mon, 26 Aug 2019 16:17:11 +0200 Subject: [PATCH 3/6] add support for the deadline option on mac platform --- README.md | 5 +++-- lib/builder/linux.js | 5 +++-- lib/builder/mac.js | 23 ++++++++++++++++++----- test/test-ping.js | 9 ++++++--- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 820e024..60d821b 100644 --- a/README.md +++ b/README.md @@ -91,12 +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. + 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 diff --git a/lib/builder/linux.js b/lib/builder/linux.js index 07439c7..838f0c1 100644 --- a/lib/builder/linux.js +++ b/lib/builder/linux.js @@ -20,7 +20,7 @@ var builder = {}; * 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. + * 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 @@ -50,7 +50,8 @@ 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 overridden without below checking if (typeof(_config[k]) !== 'boolean') { diff --git a/lib/builder/mac.js b/lib/builder/mac.js index 7c922ee..58ddfff 100644 --- a/lib/builder/mac.js +++ b/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]; } @@ -61,13 +71,16 @@ builder.getResult = function (target, config) { } ret = ret.concat([ - '-t', + '-W', util.format('%d', _config.timeout), ]); } if (_config.deadline) { - throw new Error('There is no deadline option on mac'); + ret = ret.concat([ + '-t', + util.format('%d', _config.deadline), + ]); } if (_config.min_reply) { diff --git a/test/test-ping.js b/test/test-ping.js index 25728b5..c7971fd 100644 --- a/test/test-ping.js +++ b/test/test-ping.js @@ -185,13 +185,16 @@ describe('ping timeout and deadline options', function () { this.spawnStub.restore(); }); - it('results in an error as deadline is not supported', function () { + it('are forwarded to the ping binary', function () { return ping.promise.probe('whatever', { timeout: 47, deadline: 83, }).then(function () { - throw new Error('deadline should result in an error'); - }).catch(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('-t') + 1]).to.equal('83'); + }.bind(this)); }); }); }); From ffcc05238a2773fc23115029bc59862b71a485a0 Mon Sep 17 00:00:00 2001 From: sa-linetco Date: Mon, 26 Aug 2019 16:38:50 +0200 Subject: [PATCH 4/6] fix unit for mac timeout --- lib/builder/mac.js | 2 +- lib/builder/win.js | 2 +- test/test-ping.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/builder/mac.js b/lib/builder/mac.js index 58ddfff..74ab136 100644 --- a/lib/builder/mac.js +++ b/lib/builder/mac.js @@ -72,7 +72,7 @@ builder.getResult = function (target, config) { ret = ret.concat([ '-W', - util.format('%d', _config.timeout), + util.format('%d', _config.timeout * 1000), ]); } diff --git a/lib/builder/win.js b/lib/builder/win.js index 8d6a5d2..8507b36 100644 --- a/lib/builder/win.js +++ b/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 duration for ping command to exit * @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 diff --git a/test/test-ping.js b/test/test-ping.js index c7971fd..170a492 100644 --- a/test/test-ping.js +++ b/test/test-ping.js @@ -192,7 +192,7 @@ describe('ping timeout and deadline options', function () { }).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('4700'); expect(pingArgs[pingArgs.indexOf('-t') + 1]).to.equal('83'); }.bind(this)); }); From 5a4a33f2f1790c5c0e1554a7b1f013aa24195faa Mon Sep 17 00:00:00 2001 From: sa-linetco Date: Mon, 26 Aug 2019 16:39:14 +0200 Subject: [PATCH 5/6] correct windows configuration object documentation --- lib/builder/win.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/builder/win.js b/lib/builder/win.js index 8507b36..045d35f 100644 --- a/lib/builder/win.js +++ b/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 - Timeout in seconds for 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 From 5be9f2cb4bec037e29b1f36c9c08f8fda5684f85 Mon Sep 17 00:00:00 2001 From: Mond WAN Date: Tue, 27 Aug 2019 23:05:17 +0800 Subject: [PATCH 6/6] Update README.md Address the discussions on `timeout` and `deadline` in the README --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 60d821b..5d23edb 100644 --- a/README.md +++ b/README.md @@ -155,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