[js] Changed webdriver.CommandExecutor (and its various implementatio…

…ns) to

return promises instead of using callback passing. The previous API was
inconsistent with the rest of the webdriver.js library.

javascript/node/selenium-webdriver/CHANGES.md

@@ -4,6 +4,8 @@
     `promise.Promise#thenCatch()` is not yet deprecated, but it simply
     delegates to `catch`.
 * Changed some `io` operations to use native promises.
+* Changed `webdriver.CommandExecutor#execute()` and `HttpClient` to return
+    promises instead of using callback passing.
 * Rewrote the `error` module to export an Error subtype for each type of error
     defined in the [W3C WebDriver spec](https://w3c.github.io/webdriver/webdriver-spec.html#handling-errors).
     For the export types, the `code` property is now the string code used by

javascript/node/selenium-webdriver/executors.js

@@ -37,10 +37,8 @@ var HttpClient = require('./http').HttpClient,
 var DeferredExecutor = function(delegate) {
 
   /** @override */
-  this.execute = function(command, callback) {
-    delegate.then(function(executor) {
-      executor.execute(command, callback);
-    }, callback);
+  this.execute = function(command) {
+    return delegate.then(executor => executor.execute(command));
   };
 };
 

javascript/node/selenium-webdriver/http/index.js

@@ -64,7 +64,7 @@ var HttpClient = function(serverUrl, opt_agent, opt_proxy) {
 
 
 /** @override */
-HttpClient.prototype.send = function(httpRequest, callback) {
+HttpClient.prototype.send = function(httpRequest) {
   var data;
   httpRequest.headers['Content-Length'] = 0;
   if (httpRequest.method == 'POST' || httpRequest.method == 'PUT') {
@@ -93,19 +93,23 @@ HttpClient.prototype.send = function(httpRequest, callback) {
     options.agent = this.agent_;
   }
 
-  sendRequest(options, callback, data, this.proxy_);
+  var proxy = this.proxy_;
+  return new Promise(function(fulfill, reject) {
+    sendRequest(options, fulfill, reject, data, proxy);
+  });
 };
 
 
 /**
  * Sends a single HTTP request.
  * @param {!Object} options The request options.
- * @param {function(Error, !webdriver.http.Response=)} callback The function to
- *     invoke with the server's response.
+ * @param {function(!webdriver.http.Response)} onOk The function to call if the
+ *     request succeeds.
+ * @param {function(!Error)} onError The function to call if the request fails.
  * @param {string=} opt_data The data to send with the request.
  * @param {string=} opt_proxy The proxy server to use for the request.
  */
-var sendRequest = function(options, callback, opt_data, opt_proxy) {
+var sendRequest = function(options, onOk, onError, opt_data, opt_proxy) {
   var host = options.host;
   var port = options.port;
 
@@ -127,7 +131,7 @@ var sendRequest = function(options, callback, opt_data, opt_proxy) {
       try {
         var location = url.parse(response.headers['location']);
       } catch (ex) {
-        callback(Error(
+        onError(Error(
             'Failed to parse "Location" header for server redirect: ' +
             ex.message + '\nResponse was: \n' +
             new HttpResponse(response.statusCode, response.headers, '')));
@@ -148,7 +152,7 @@ var sendRequest = function(options, callback, opt_data, opt_proxy) {
         headers: {
           'Accept': 'application/json; charset=utf-8'
         }
-      }, callback, undefined, opt_proxy);
+      }, onOk, onError, undefined, opt_proxy);
       return;
     }
 
@@ -157,21 +161,21 @@ var sendRequest = function(options, callback, opt_data, opt_proxy) {
     response.on('end', function() {
       var resp = new HttpResponse(response.statusCode,
           response.headers, body.join('').replace(/\0/g, ''));
-      callback(null, resp);
+      onOk(resp);
     });
   });
 
   request.on('error', function(e) {
     if (e.code === 'ECONNRESET') {
       setTimeout(function() {
-        sendRequest(options, callback, opt_data, opt_proxy);
+        sendRequest(options, onOk, onError, opt_data, opt_proxy);
       }, 15);
     } else {
       var message = e.message;
       if (e.code) {
         message = e.code + ' ' + message;
       }
-      callback(new Error(message));
+      onError(new Error(message));
     }
   });
 

javascript/node/selenium-webdriver/http/util.js

@@ -33,21 +33,16 @@ var base = require('../lib/_base'),
 /**
  * Queries a WebDriver server for its current status.
  * @param {string} url Base URL of the server to query.
- * @param {function(Error, *=)} callback The function to call with the
- *     response.
+ * @return {!webdriver.promise.Promise.<!Object>} A promise that resolves with
+ *     a hash of the server status.
  */
-function getStatus(url, callback) {
+function getStatus(url) {
   var client = new HttpClient(url);
   var executor = new Executor(client);
   var command = new Command(CommandName.GET_SERVER_STATUS);
-  executor.execute(command, function(err, responseObj) {
-    if (err) return callback(err);
-    try {
-      checkResponse(responseObj);
-    } catch (ex) {
-      return callback(ex);
-    }
-    callback(null, responseObj['value']);
+  return executor.execute(command).then(function(responseObj) {
+    checkResponse(responseObj);
+    return responseObj['value'];
   });
 }
 
@@ -61,9 +56,7 @@ function getStatus(url, callback) {
  * @return {!webdriver.promise.Promise.<!Object>} A promise that resolves with
  *     a hash of the server status.
  */
-exports.getStatus = function(url) {
-  return promise.checkedNodeCall(getStatus.bind(null, url));
-};
+exports.getStatus = getStatus;
 
 
 /**
@@ -75,15 +68,15 @@ exports.getStatus = function(url) {
  */
 exports.waitForServer = function(url, timeout) {
   var ready = promise.defer(),
-      start = Date.now(),
-      checkServerStatus = getStatus.bind(null, url, onResponse);
+      start = Date.now();
   checkServerStatus();
   return ready.promise;
 
-  function onResponse(err) {
-    if (!ready.isPending()) return;
-    if (!err) return ready.fulfill();
+  function checkServerStatus() {
+    return getStatus(url).then(ready.fulfill, onError);
+  }
 
+  function onError() {
     if (Date.now() - start > timeout) {
       ready.reject(
           Error('Timed out waiting for the WebDriver server at ' + url));
@@ -109,18 +102,16 @@ exports.waitForServer = function(url, timeout) {
 exports.waitForUrl = function(url, timeout) {
   var client = new HttpClient(url),
       request = new HttpRequest('GET', ''),
-      testUrl = client.send.bind(client, request, onResponse),
       ready = promise.defer(),
       start = Date.now();
   testUrl();
   return ready.promise;
 
-  function onResponse(err, response) {
-    if (!ready.isPending()) return;
-    if (!err && response.status > 199 && response.status < 300) {
-      return ready.fulfill();
-    }
+  function testUrl() {
+    client.send(request).then(onResponse, onError);
+  }
 
+  function onError() {
     if (Date.now() - start > timeout) {
       ready.reject(Error(
           'Timed out waiting for the URL to return 2xx: ' + url));
@@ -132,4 +123,12 @@ exports.waitForUrl = function(url, timeout) {
       }, 50);
     }
   }
+
+  function onResponse(response) {
+    if (!ready.isPending()) return;
+    if (response.status > 199 && response.status < 300) {
+      return ready.fulfill();
+    }
+    onError();
+  }
 };

javascript/node/selenium-webdriver/test/http/http_test.js

@@ -99,16 +99,15 @@ describe('HttpClient', function() {
     agent.maxSockets = 1;  // Only making 1 request.
 
     var client = new HttpClient(server.url(), agent);
-    return promise.checkedNodeCall(client.send.bind(client, request))
-        .then(function(response) {
-          assert.equal(200, response.status);
-          assert.equal(
-            'application/json; charset=utf-8', response.headers['accept']);
-          assert.equal('Bar', response.headers['foo']);
-          assert.equal('0', response.headers['content-length']);
-          assert.equal('keep-alive', response.headers['connection']);
-          assert.equal(server.host(), response.headers['host']);
-        });
+    return client.send(request).then(function(response) {
+      assert.equal(200, response.status);
+      assert.equal(
+        'application/json; charset=utf-8', response.headers['accept']);
+      assert.equal('Bar', response.headers['foo']);
+      assert.equal('0', response.headers['content-length']);
+      assert.equal('keep-alive', response.headers['connection']);
+      assert.equal(server.host(), response.headers['host']);
+    });
   });
 
   test.it('can use basic auth', function() {
@@ -117,65 +116,59 @@ describe('HttpClient', function() {
 
     var client = new HttpClient(url.format(parsed));
     var request = new HttpRequest('GET', '/protected');
-    return promise.checkedNodeCall(client.send.bind(client, request))
-      .then(function(response) {
-        assert.equal(200, response.status);
-        assert.equal('text/plain', response.headers['content-type']);
-        assert.equal('Access granted!', response.body);
-      });
+    return client.send(request).then(function(response) {
+      assert.equal(200, response.status);
+      assert.equal('text/plain', response.headers['content-type']);
+      assert.equal('Access granted!', response.body);
+    });
   });
 
   test.it('fails requests missing required basic auth', function() {
     var client = new HttpClient(server.url());
     var request = new HttpRequest('GET', '/protected');
-    return promise.checkedNodeCall(client.send.bind(client, request))
-      .then(function(response) {
-              assert.equal(401, response.status);
-              assert.equal('Access denied', response.body);
-            });
+    return client.send(request).then(function(response) {
+      assert.equal(401, response.status);
+      assert.equal('Access denied', response.body);
+    });
   });
 
   test.it('automatically follows redirects', function() {
     var request = new HttpRequest('GET', '/redirect');
     var client = new HttpClient(server.url());
-    return promise.checkedNodeCall(client.send.bind(client, request))
-        .then(function(response) {
-          assert.equal(200, response.status);
-          assert.equal('text/plain', response.headers['content-type']);
-          assert.equal('hello, world!', response.body);
-        });
+    return client.send(request).then(function(response) {
+      assert.equal(200, response.status);
+      assert.equal('text/plain', response.headers['content-type']);
+      assert.equal('hello, world!', response.body);
+    });
   });
 
   test.it('handles malformed redirect responses', function() {
     var request = new HttpRequest('GET', '/badredirect');
     var client = new HttpClient(server.url());
-    return promise.checkedNodeCall(client.send.bind(client, request))
-        .thenCatch(function(err) {
-          assert.ok(/Failed to parse "Location"/.test(err.message),
-              'Not the expected error: ' + err.message);
-        });
+    return client.send(request).then(assert.fail, function(err) {
+      assert.ok(/Failed to parse "Location"/.test(err.message),
+          'Not the expected error: ' + err.message);
+    });
   });
 
   test.it('proxies requests through the webdriver proxy', function() {
     var request = new HttpRequest('GET', '/proxy');
     var client = new HttpClient(
         'http://another.server.com', undefined, server.url());
-    return promise.checkedNodeCall(client.send.bind(client, request))
-        .then(function(response) {
-           assert.equal(200, response.status);
-           assert.equal('another.server.com', response.headers['host']);
-        });
+    return client.send(request).then(function(response) {
+       assert.equal(200, response.status);
+       assert.equal('another.server.com', response.headers['host']);
+    });
   });
 
   test.it(
       'proxies requests through the webdriver proxy on redirect', function() {
     var request = new HttpRequest('GET', '/proxy/redirect');
     var client = new HttpClient(
         'http://another.server.com', undefined, server.url());
-    return promise.checkedNodeCall(client.send.bind(client, request))
-        .then(function(response) {
-          assert.equal(200, response.status);
-          assert.equal('another.server.com', response.headers['host']);
-        });
+    return client.send(request).then(function(response) {
+      assert.equal(200, response.status);
+      assert.equal('another.server.com', response.headers['host']);
+    });
   });
 });

javascript/node/selenium-webdriver/test/http/util_test.js

@@ -65,60 +65,59 @@ describe('selenium-webdriver/http/util', function() {
   });
 
   describe('#getStatus', function() {
-    it('should return value field on success', function(done) {
-      util.getStatus(baseUrl).then(function(response) {
+    it('should return value field on success', function() {
+      return util.getStatus(baseUrl).then(function(response) {
         assert.equal('abc123', response);
-      }).thenFinally(done);
+      });
     });
 
-    it('should fail if response object is not success', function(done) {
+    it('should fail if response object is not success', function() {
       status = 1;
-      util.getStatus(baseUrl).then(function() {
+      return util.getStatus(baseUrl).then(function() {
         throw Error('expected a failure');
       }, function(err) {
         assert.equal(status, err.code);
         assert.equal(value, err.message);
-      }).thenFinally(done);
+      });
     });
 
     it('should fail if the server is not listening', function(done) {
       killServer(function(e) {
         if(e) return done(e);
 
         util.getStatus(baseUrl).then(function() {
-          throw Error('expected a failure');
+          done(Error('expected a failure'));
         }, function() {
           // Expected.
-        }).thenFinally(done);
+          done();
+        });
       });
     });
 
-    it('should fail if HTTP status is not 200', function(done) {
+    it('should fail if HTTP status is not 200', function() {
       status = 1;
       responseCode = 404;
-      util.getStatus(baseUrl).then(function() {
+      return util.getStatus(baseUrl).then(function() {
         throw Error('expected a failure');
       }, function(err) {
         assert.equal(status, err.code);
         assert.equal(value, err.message);
-      }).thenFinally(done);
+      });
     });
   });
 
   describe('#waitForServer', function() {
-    it('resolves when server is ready', function(done) {
+    it('resolves when server is ready', function() {
       status = 1;
       setTimeout(function() { status = 0; }, 50);
-      util.waitForServer(baseUrl, 100).
-          then(function() {}).  // done needs no argument to pass.
-          thenFinally(done);
+      return util.waitForServer(baseUrl, 100);
     });
 
-    it('should fail if server does not become ready', function(done) {
+    it('should fail if server does not become ready', function() {
       status = 1;
-      util.waitForServer(baseUrl, 50).
-          then(function() { done('Expected to time out'); },
-               function() { done(); });
+      return util.waitForServer(baseUrl, 50).
+          then(function() {throw Error('Expected to time out')},
+               function() {});
     });
 
     it('can cancel wait', function(done) {