Skip to content

Commit

Permalink
feat(SELENIUM_PROMISE_MANAGER=0): Improve support for SELENIUM_PROMIS…
Browse files Browse the repository at this point in the history
…E_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
(70c9f62), 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<any>`, there's no way it has a `.then` function.

Might close angular#69 once we rebase `beta`
against this, but I haven't checked.
  • Loading branch information
sjelin committed Dec 17, 2016
1 parent ffae218 commit eaa1ff8
Showing 1 changed file with 89 additions and 24 deletions.
113 changes: 89 additions & 24 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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 = '';
Expand Down Expand Up @@ -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;
}
}
Expand Down

0 comments on commit eaa1ff8

Please sign in to comment.