Updated unit tests for request timeout and slight refactoring

appcoreopc · Dec 13, 2016 · f3bc41c · f3bc41c
1 parent 1ed15e1
commit f3bc41c
Show file tree

Hide file tree

Showing 8 changed files with 135 additions and 76 deletions.
diff --git a/lib/http/request.js b/lib/http/request.js
@@ -169,16 +169,12 @@ module.exports = (function() {
       self.emit('error', {}, response);
     });
 
-    this.request.setTimeout(this.timeout, function(){
+    this.request.setTimeout(this.timeout, function() {
+      self.request.abort();
+
       if (self.retryAttempts) {
-        Logger.info('retrying on http timeout');
         self.retryAttempts = self.retryAttempts - 1;
-        if (this.socket.unref) {
-          this.socket.unref();
-        }
         self.send();
-      } else {
-        self.emit('error', {});
       }
     });
 

diff --git a/lib/index.js b/lib/index.js
@@ -118,8 +118,6 @@ Nightwatch.prototype.setOptions = function(options) {
   var useSSL           = this.options.useSsl || this.options.use_ssl;
   var proxy            = this.options.proxy;
   var timeoutOptions   = this.options.request_timeout_options || {};
-  var timeout          = timeoutOptions.timeout;
-  var retryAttempts    = timeoutOptions.retry_attempts;
 
   if (seleniumPort) {
     HttpRequest.setSeleniumPort(seleniumPort);
@@ -133,11 +131,11 @@ Nightwatch.prototype.setOptions = function(options) {
   if (proxy) {
     HttpRequest.setProxy(proxy);
   }
-  if (timeout) {
-    HttpRequest.setTimeout(timeout);
+  if (typeof timeoutOptions.timeout != 'undefined') {
+    HttpRequest.setTimeout(timeoutOptions.timeout);
   }
-  if (retryAttempts) {
-    HttpRequest.setRetryAttempts(retryAttempts);
+    if (typeof timeoutOptions.retry_attempts != 'undefined') {
+    HttpRequest.setRetryAttempts(timeoutOptions.retry_attempts);
   }
 
   if (typeof this.options.default_path_prefix == 'string') {

diff --git a/package.json b/package.json
@@ -41,8 +41,7 @@
     "mocha-lcov-reporter": "^1.2.0",
     "mock-spawn": "^0.2.1",
     "mockery": "~1.4.0",
-    "nock": "https://github.com/NickStefan/nock/tarball/set-allow-unmock",
-    "nodeunit": "latest"
+    "nock": "~0.45.0"
   },
   "bin": {
     "nightwatch": "./bin/nightwatch"

diff --git a/test/lib/mockserver.js b/test/lib/mockserver.js
@@ -59,6 +59,7 @@ function MockServer(options, callback) {
     req.on('data', function(chunk) {
       postdata += chunk;
     });
+
     req.on('end', function() {
       var item = nextInLine(req, postdata);
       var responsedata = '';
@@ -80,11 +81,23 @@ function MockServer(options, callback) {
         options.finishedCallback(req, res);
       }
 
-      res.end(responsedata, function() {
-        if (item && item.__once) {
-          removeMock(item);
-        }
-      });
+      if (item && item.__once) {
+        removeMock(item);
+      }
+
+      if (item && item.socketDelay) {
+        var timeoutId = setTimeout(function() {
+          res.end(responsedata);
+        }, item.socketDelay);
+
+        req.on('close', function(err) {
+          clearTimeout(timeoutId);
+        });
+
+        return;
+      }
+
+      res.end(responsedata);
     });
   });
 

diff --git a/test/lib/nocks.js b/test/lib/nocks.js
@@ -1,5 +1,4 @@
 var nock = require('nock');
-nock.setAllowUnmocked(true);
 
 module.exports = {
   createSession : function() {

diff --git a/test/src/http/testRequest.js b/test/src/http/testRequest.js
@@ -207,61 +207,6 @@ module.exports = {
         done();
       }).send();
 
-    },
-
-    testRequestTimeout: function (done) {
-      nock('http://localhost:4444')
-        .get('/wd/hub/123456/element')
-        .socketDelay(10000)
-        .reply(200, {});
-
-      var options = {
-        path: '/:sessionId/element',
-        method: 'GET',
-        sessionId: '123456'
-      };
-
-      HttpRequest.setTimeout(100);
-
-      var request = new HttpRequest(options);
-
-      request.on('error', function () {
-        assert.ok(true); // should trigger error on timeout
-        done();
-      }).send();
-
-    },
-
-    testRetryAttempts: function (done) {
-      nock('http://localhost:4444')
-        .get('/wd/hub/123456/element')
-        .socketDelay(10000)
-        .reply(500, {})
-        .get('/wd/hub/123456/element')
-        .socketDelay(100)
-        .reply(200, {})
-
-      var options = {
-        path: '/:sessionId/element',
-        method: 'GET',
-        sessionId: '123456'
-      };
-
-      HttpRequest.setTimeout(200);
-      HttpRequest.setRetryAttempts(1);
-
-      var request = new HttpRequest(options);
-
-      request
-      .on('success', function (result) {
-        assert.ok(true); // should succeed
-        done();
-      })
-      .on('error', function(){
-        // the nock library is merely invoking the timeout event, rather than actually timing out,
-        // so catch the first request that is supposed to delay 10000
-      }).send();
-
     }
   }
 

diff --git a/test/src/http/testRequestTimeout.js b/test/src/http/testRequestTimeout.js
@@ -0,0 +1,89 @@
+var common = require('../../common.js');
+var HttpRequest = common.require('http/request.js');
+var MockServer  = require('../../lib/mockserver.js');
+var Logger = common.require('util/logger');
+var assert = require('assert');
+
+module.exports = {
+  'test HttpRequestTimeout' : {
+    beforeEach : function(done) {
+      this.server = MockServer.init();
+      Logger.disable();
+      this.server.on('listening', function() {
+        done();
+      });
+    },
+
+    afterEach : function(done) {
+      this.server.close(function() {
+        done();
+      });
+    },
+
+    testRequestTimeout: function (done) {
+      MockServer.addMock({
+        url : '/wd/hub/123456/element',
+        response : '',
+        method: 'GET',
+        socketDelay : 400
+      }, true);
+
+      var options = {
+        path: '/:sessionId/element',
+        selenium_port: 10195,
+        method: 'GET',
+        sessionId: '123456'
+      };
+
+      HttpRequest.setTimeout(50);
+
+      var request = new HttpRequest(options);
+
+      request.on('error', function (err, res) {
+        assert.ok(res instanceof Error);
+        done();
+      }).on('success', function(result, response) {
+        done(new Error('Request should have timed out.'));
+      }).send();
+
+    },
+
+    testRetryAttempts: function (done) {
+      MockServer.addMock({
+        url : '/wd/hub/10000000/element',
+        response : '',
+        method: 'GET',
+        socketDelay : 200
+      }, true).addMock({
+        url : '/wd/hub/10000000/element',
+        response : '',
+        method: 'GET'
+      }, true);
+
+      var options = {
+        path: '/:sessionId/element',
+        selenium_port: 10195,
+        method: 'GET',
+        sessionId: '10000000'
+      };
+
+      HttpRequest.setTimeout(50);
+      HttpRequest.setRetryAttempts(1);
+
+      var request = new HttpRequest(options);
+      assert.equal(request.retryAttempts, 1);
+
+      request
+        .on('error', function(res, err) {
+          assert.strictEqual(request.retryAttempts, 0);
+          assert.ok(err instanceof Error); // the 'socket hang up' error, caused by aborting the request
+        })
+        .on('success', function (result) {
+          assert.strictEqual(request.retryAttempts, 0);
+          done();
+        })
+        .send();
+    }
+  }
+
+};
diff --git a/test/src/index/testNightwatchIndex.js b/test/src/index/testNightwatchIndex.js
@@ -291,6 +291,26 @@ module.exports = {
       eq(client.options.end_session_on_fail, false);
     },
 
+    testSetRequestTimeoutOptions: function () {
+      var client = Nightwatch.createClient({
+        request_timeout_options: {
+          timeout : 10000,
+          retry_attempts : 3
+        }
+      });
+
+      assert.deepEqual(client.options.request_timeout_options, {
+        timeout : 10000,
+        retry_attempts : 3
+      });
+
+      var common = require('../../common.js');
+      var HttpRequest = common.require('http/request.js');
+      var request = new HttpRequest({});
+      assert.equal(request.timeout, 10000);
+      assert.equal(request.retryAttempts, 3);
+    },
+
     'test session response with status success and no sessionId': function (done) {
       MockServer.addMock({
         url : '/wd/hub/session',