From eaa1ff868e9ffaa9af3a6886913b34fc983866e1 Mon Sep 17 00:00:00 2001 From: Sammy Jelin Date: Fri, 16 Dec 2016 22:53:26 -0800 Subject: [PATCH] feat(SELENIUM_PROMISE_MANAGER=0): Improve support for SELENIUM_PROMISE_MANAGER=0 There are three major ways this was done in this change: * In `callWhenIdle`, if `flow.isIdle` is not defined, we assume we are working with a `SimpleScheduler` instance, and so the flow is effectively idle. * In `wrapInControlFlow`, we use `flow.promise` to create a new promise if possible. Since `new webdriver.promise.Promise()` would have always made a `ManagedPromise`, but `flow.promise` will do the right thing. * In `wrapCompare`, we avoid the webdriver library entirely, and never instance any extra promises. Using `webdriver.promise.when` and `webdriver.promise.all` could have been a problem if our instance of `webdriver` had the control flow turned on, but another instance somewhere did not (or even the same instance, but just at a different point in time). I also added code to extract promises out of deferreds. This is needed for selenium 3.x, and Craig already implemented a solution in the beta branch (70c9f62af50018bea6ad326e12bacd9ca03e6ae5), but I was worried about what could happen if there were multiple versions of `selenium-webdriver` floating around. This change extracts promises without using `instanceof`. As an added bonus, it now works with `q`'s deferreds too! I also also fixed a minor bug where we weren't correctly checking for promises inside an array of expected results. Before we had ```js expected = Array.prototype.slice.call(arguments, 0) ... webdriver.promise.isPromise(expected) ``` I thought about it for a little while, and there's no way that's correct. `expected` is an `Array`, there's no way it has a `.then` function. Might close https://github.com/angular/jasminewd/issues/69 once we rebase `beta` against this, but I haven't checked. --- index.js | 113 +++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 89 insertions(+), 24 deletions(-) diff --git a/index.js b/index.js index ab1b10a..61909ce 100644 --- a/index.js +++ b/index.js @@ -54,7 +54,7 @@ function validateString(stringtoValidate) { * @param {!Function} fn The function to call */ function callWhenIdle(flow, fn) { - if (flow.isIdle()) { + if (!flow.isIdle || flow.isIdle()) { fn(); } else { flow.once(webdriver.promise.ControlFlow.EventType.IDLE, function() { @@ -84,7 +84,14 @@ function wrapInControlFlow(flow, globalFn, fnName) { var testFn = fn.bind(this); flow.execute(function controlFlowExecute() { - return new webdriver.promise.Promise(function(fulfill, reject) { + function newPromise(resolver) { + if (typeof flow.promise == 'function') { + return flow.promise(resolver); + } else { + return new webdriver.promise.Promise(resolver, flow); + } + } + return newPromise(function(fulfill, reject) { function wrappedReject(err) { var wrappedErr = new Error(err); reject(wrappedErr); @@ -106,7 +113,7 @@ function wrapInControlFlow(flow, globalFn, fnName) { fulfill(ret); } } - }, flow); + }); }, 'Run ' + fnName + description + ' in control flow').then( callWhenIdle.bind(null, flow, done), function(err) { if (!err) { @@ -193,6 +200,74 @@ global.expect = function(actual) { return originalExpect(actual); }; +/** + * Gets the underlying promise out of a deferred, does nothing to non-deferreds. + * + * @param {*} val The deferred (or not) + * @return {*} The underlying promise of val if it is a deferred, or val itself + * if it wasn't a deferred or was already a promise + */ +function extractPromiseFromDeferred(val) { + // This check isn't perfect, but it's much better than `instanceof Deferred`, + // which will fail if there's a version mismatch or if the deferred is from a + // different library. + if (val && !val.then && val.promise && (typeof val.fulfill == 'function') && + (typeof val.reject == 'function') && val.constructor && + val.constructor.name && (val.constructor.name.indexOf('defer') != -1)) { + return val.promise; + } else { + return val; + } +} + +/** + * Runs a callback synchronously against non-promise values and asynchronously + * against promises. Similar to ES6's `Promise.resolve` except that it is + * synchronous when possible and won't wrap the return value. + * + * This is not what you normally want. Normally you want the code to be + * consistently asynchronous, and you want the result wrapped into a promise. + * But because of webdriver's control flow, we're better off not introducing any + * extra layers of promises or asynchronous activity. + * + * @param {*} val The value to call the callback with. + * @param {!Function} callback The callback function + * @return {*} If val isn't a promise, the return value of the callback is + * directly returned. If val is a promise, a promise (generated by val.then) + * resolving to the callback's return value is returned. + */ +function maybePromise(val, callback) { + if (val && (typeof val.then == 'function')) { + return val.then(callback); + } else { + return callback(val); + } +} + +/** + * Like maybePromise() but for an array of values + * + * @param {!Array<*>} vals An array of values to call the callback with + * @param {!Function} callback the callback function + * @return {*} If nothing in vals is a promise, the return value of the callback + * is directly returned. Otherwise, a promise (generated by the .then + * functions in vals) resolving to the callback's return value is returned. + */ +function maybePromises(vals, callback) { + var resolved = new Array(vals.length); + function resolveAt(i) { + if (i >= vals.length) { + return callback(resolved); + } else { + return maybePromise(vals[i], function(val) { + resolved[i] = val; + return resolveAt(i+1); + }); + } + } + return resolveAt(0); +} + /** * Creates a matcher wrapper that resolves any promises given for actual and * expected values, as well as the `pass` property of the result. @@ -205,16 +280,14 @@ jasmine.Expectation.prototype.wrapCompare = function(name, matcherFactory) { matchError.stack = matchError.stack.replace(/ +at.+jasminewd.+\n/, ''); - if (!webdriver.promise.isPromise(expectation.actual) && - !webdriver.promise.isPromise(expected)) { - compare(expectation.actual, expected); - } else { - webdriver.promise.when(expectation.actual).then(function(actual) { - return webdriver.promise.all(expected).then(function(expected) { - return compare(actual, expected); - }); + expectation.actual = extractPromiseFromDeferred(expectation.actual); + expected = expected.map(extractPromiseFromDeferred); + + maybePromise(expectation.actual, function(actual) { + return maybePromises(expected, function(expected) { + return compare(actual, expected); }); - } + }); function compare(actual, expected) { var args = expected.slice(0); @@ -229,11 +302,7 @@ jasmine.Expectation.prototype.wrapCompare = function(name, matcherFactory) { var result = matcherCompare.apply(null, args); - if (webdriver.promise.isPromise(result.pass)) { - return webdriver.promise.when(result.pass).then(compareDone); - } else { - return compareDone(result.pass); - } + return maybePromise(result.pass, compareDone); function compareDone(pass) { var message = ''; @@ -268,13 +337,9 @@ jasmine.Expectation.prototype.wrapCompare = function(name, matcherFactory) { function defaultNegativeCompare() { var result = matcher.compare.apply(null, args); - if (webdriver.promise.isPromise(result.pass)) { - result.pass = result.pass.then(function(pass) { - return !pass; - }); - } else { - result.pass = !result.pass; - } + result.pass = maybePromise(result.pass, function(pass) { + return !pass; + }); return result; } }