Fix tests

* Go back to previous behaviour of continuing to emit ERROR states if it continues to fail
 * Don't set a timer if the timeout is zero
 * Change test to assert the continued-error behaviour, not exactly multiple syncs failing
 * Update other tests to fail the keepalive requests where appropriate

lib/sync.js

@@ -856,10 +856,14 @@ SyncApi.prototype._startKeepAlives = function(delay) {
         clearTimeout(this._keepAliveTimer);
     }
     var self = this;
+    if (delay > 0) {
         self._keepAliveTimer = setTimeout(
             self._pokeKeepAlive.bind(self),
             delay
         );
+    } else {
+        self._pokeKeepAlive();
+    }
     if (!this._connectionReturnedDefer) {
         this._connectionReturnedDefer = q.defer();
     }
@@ -899,17 +903,18 @@ SyncApi.prototype._pokeKeepAlive = function() {
             // responses fail, this will mean we don't hammer in a loop.
             self._keepAliveTimer = setTimeout(success, 2000);
         } else {
-            // If we haven't already marked this sync
-            // connection as gone-away, do so now and
-            // emit an error.
-            if (!self._syncConnectionLost) {
-                self._syncConnectionLost = true;
-                self._updateSyncState("ERROR", { error: err });
-            }
             self._keepAliveTimer = setTimeout(
                 self._pokeKeepAlive.bind(self),
                 5000 + Math.floor(Math.random() * 5000)
             );
+            // If we haven't already marked this sync
+            // connection as gone-away, do so now and
+            // emit an error.
+            // Note we do this after setting the timer:
+            // this lets the unit tests advance the mock
+            // clock when the get the error.
+            self._syncConnectionLost = true;
+            self._updateSyncState("ERROR", { error: err });
         }
     });
 };

spec/unit/matrix-client.spec.js

@@ -51,9 +51,10 @@ describe("MatrixClient", function() {
         // }
         // items are popped off when processed and block if no items left.
     ];
+    var accept_keepalives;
     var pendingLookup = null;
     function httpReq(cb, method, path, qp, data, prefix) {
-        if (path === KEEP_ALIVE_PATH) {
+        if (path === KEEP_ALIVE_PATH && accept_keepalives) {
             return q();
         }
         var next = httpLookups.shift();
@@ -143,8 +144,10 @@ describe("MatrixClient", function() {
         client._http.authedRequest.andCallFake(httpReq);
         client._http.authedRequestWithPrefix.andCallFake(httpReq);
         client._http.requestWithPrefix.andCallFake(httpReq);
+        client._http.request.andCallFake(httpReq);
 
         // set reasonable working defaults
+        accept_keepalives = true;
         pendingLookup = null;
         httpLookups = [];
         httpLookups.push(PUSH_RULES_RESPONSE);
@@ -329,12 +332,19 @@ describe("MatrixClient", function() {
         it("should transition ERROR -> PREPARED after /sync if prev failed",
         function(done) {
             var expectedStates = [];
+            accept_keepalives = false;
             httpLookups = [];
             httpLookups.push(PUSH_RULES_RESPONSE);
             httpLookups.push(FILTER_RESPONSE);
             httpLookups.push({
                 method: "GET", path: "/sync", error: { errcode: "NOPE_NOPE_NOPE" }
             });
+            httpLookups.push({
+                method: "GET", path: KEEP_ALIVE_PATH, error: { errcode: "KEEPALIVE_FAIL" }
+            });
+            httpLookups.push({
+                method: "GET", path: KEEP_ALIVE_PATH, data: {}
+            });
             httpLookups.push({
                 method: "GET", path: "/sync", data: SYNC_DATA
             });
@@ -354,10 +364,14 @@ describe("MatrixClient", function() {
         });
 
         it("should transition SYNCING -> ERROR after a failed /sync", function(done) {
+            accept_keepalives = false;
             var expectedStates = [];
             httpLookups.push({
                 method: "GET", path: "/sync", error: { errcode: "NONONONONO" }
             });
+            httpLookups.push({
+                method: "GET", path: KEEP_ALIVE_PATH, error: { errcode: "KEEPALIVE_FAIL" }
+            });
 
             expectedStates.push(["PREPARED", null]);
             expectedStates.push(["SYNCING", "PREPARED"]);
@@ -394,13 +408,17 @@ describe("MatrixClient", function() {
             client.startClient();
         });
 
-        it("should transition ERROR -> ERROR if multiple /sync fails", function(done) {
+        it("should transition ERROR -> ERROR if keepalive keeps failing", function(done) {
+            accept_keepalives = false;
             var expectedStates = [];
             httpLookups.push({
                 method: "GET", path: "/sync", error: { errcode: "NONONONONO" }
             });
             httpLookups.push({
-                method: "GET", path: "/sync", error: { errcode: "NONONONONO" }
+                method: "GET", path: KEEP_ALIVE_PATH, error: { errcode: "KEEPALIVE_FAIL" }
+            });
+            httpLookups.push({
+                method: "GET", path: KEEP_ALIVE_PATH, error: { errcode: "KEEPALIVE_FAIL" }
             });
 
             expectedStates.push(["PREPARED", null]);