From c3181ef8261332fe93cc313e2b2d540ef4b9a5c6 Mon Sep 17 00:00:00 2001
From: Simon Woolf <simon@ably.io>
Date: Thu, 8 Oct 2015 10:49:10 +0100
Subject: [PATCH] Connection#ping: support no cb, return responseTime, add test

---
 common/lib/client/connection.js           |  2 +
 common/lib/transport/connectionmanager.js | 10 ++--
 spec/realtime/connection.test.js          | 63 +++++++++++++++++++++++
 3 files changed, 72 insertions(+), 3 deletions(-)
 create mode 100644 spec/realtime/connection.test.js

diff --git a/common/lib/client/connection.js b/common/lib/client/connection.js
index 709817ace6..7eec708677 100644
--- a/common/lib/client/connection.js
+++ b/common/lib/client/connection.js
@@ -35,6 +35,8 @@ var Connection = (function() {
 
 	Connection.prototype.ping = function(callback) {
 		Logger.logAction(Logger.LOG_MINOR, 'Connection.ping()', '');
+		if(typeof(callback) !== 'function')
+			callback = function() {};
 		this.connectionManager.ping(null, callback);
 	};
 
diff --git a/common/lib/transport/connectionmanager.js b/common/lib/transport/connectionmanager.js
index b85737c77c..cdce4f0bb8 100644
--- a/common/lib/transport/connectionmanager.js
+++ b/common/lib/transport/connectionmanager.js
@@ -957,12 +957,16 @@ var ConnectionManager = (function() {
 				callback(new ErrorInfo('Timedout waiting for heartbeat response', 50000, 500));
 			};
 
+			var pingStart = Date.now();
+
 			var onHeartbeat = function () {
 				clearTimeout(timer);
-				callback(null);
+				var responseTime = Date.now() - pingStart;
+				callback(null, responseTime);
 			};
 
 			var timer = setTimeout(onTimeout, Defaults.sendTimeout);
+
 			transport.once('heartbeat', onHeartbeat);
 			transport.ping();
 			return;
@@ -978,11 +982,11 @@ var ConnectionManager = (function() {
 		 * but ensure that we retry if the transport is superseded before we complete */
 		var completed = false, self = this;
 
-		var onPingComplete = function(err) {
+		var onPingComplete = function(err, responseTime) {
 			self.off('transport.active', onTransportActive);
 			if(!completed) {
 				completed = true;
-				callback(err);
+				callback(err, responseTime);
 			}
 		};
 
diff --git a/spec/realtime/connection.test.js b/spec/realtime/connection.test.js
new file mode 100644
index 0000000000..7042ea1426
--- /dev/null
+++ b/spec/realtime/connection.test.js
@@ -0,0 +1,63 @@
+"use strict";
+
+define(['ably', 'shared_helper'], function(Ably, helper) {
+	var exports = {},
+		closeAndFinish = helper.closeAndFinish,
+		monitorConnection = helper.monitorConnection;
+
+	exports.setupConnection = function(test) {
+		test.expect(1);
+		helper.setupApp(function(err) {
+			if(err) {
+				test.ok(false, helper.displayError(err));
+			} else {
+				test.ok(true, 'app set up');
+			}
+			test.done();
+		});
+	};
+
+	exports.connectionPing = function(test) {
+		test.expect(1);
+		var realtime;
+		try {
+			realtime = helper.AblyRealtime();
+			realtime.connection.on('connected', function() {
+				realtime.connection.ping();
+				test.ok(true, 'check that ping without did not raise exception');
+				closeAndFinish(test, realtime);
+			});
+			monitorConnection(test, realtime);
+		} catch(e) {
+			test.ok(false, 'test failed with exception: ' + e.stack);
+			closeAndFinish(test, realtime);
+		}
+	};
+
+	exports.connectionPingWithCallback = function(test) {
+		test.expect(2);
+		var realtime;
+		try {
+			realtime = helper.AblyRealtime();
+			realtime.connection.on('connected', function() {
+				realtime.connection.ping(function(err, responseTime){
+					if(err) {
+						test.ok(false, helper.displayError(err));
+						closeAndFinish(test, realtime);
+						return;
+					}
+					test.equal(typeof responseTime, "number", 'check that a responseTime returned');
+					test.ok(responseTime > 0, 'check that responseTime was +ve');
+					closeAndFinish(test, realtime);
+				});
+			});
+			monitorConnection(test, realtime);
+		} catch(e) {
+			test.ok(false, 'test failed with exception: ' + e.stack);
+			closeAndFinish(test, realtime);
+		}
+	};
+
+
+	return module.exports = helper.withTimeout(exports);
+});