-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(SELENIUM_PROMISE_MANAGER): Don't rely on webdriver.promise
fun…
#82
Conversation
Please change the commit message to mention "utility function which will be going away after the control flow has been fully deprecated." |
@juliemr done |
newPromise = function(resolver) { | ||
return new Promise(resolver); | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For code readability flow, could you change the above to:
if (!scheduler) {
scheduler = {
execute: function(fn) {
return Promise.resolve().then(fn);
}
}
}
if (typeof scheduler.promise == 'function') {
newPromise = scheduler.promise.bind(scheduler);
} else {
// what you have now...
}
} | ||
|
||
// Figure out how we're getting new promises | ||
var newPromise; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juliemr let me know what you think of this new logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like not using try/catch. Works for me!
Your scheduler must emit `"idle"` when it becomes idle. | ||
|
||
|
||
### Reset API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juliemr this was also added since your review
} | ||
|
||
// Figure out how we're getting new promises | ||
var newPromise; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like not using try/catch. Works for me!
|
||
jasminewd will automatically look for a `reset` function and call it when specs time out. This is | ||
useful so that if a spec executes a task that hangs, only that spec will timeout (as opposed to | ||
tying up the scheduler and causing all future specs to timeout). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And more generally, so that if a spec times out but has lingering commands, those get cleared instead of executing in random future tests.
…ctions While we support `SELENIUM_PROMISE_MANAGER=0` already, we rely on `SimpleScheduler` and some other utility functions which will be going away after the control flow has been fully deprecated. This commit allows jasminewd to work without those utility functions, and even allows people to pass jasminewd their own custom scheduler implementation. This does not fix our tests, which will also break when those utility functions go away. See angular#81 Closes angular#80
…ctions
While we support
SELENIUM_PROMISE_MANAGER=0
already, we rely onSimpleScheduler
and some otherutility functions which will be going away. This allows jasminewd to work without those utility
functions, and even allows people to pass jasminewd their own custom scheduler implementation.
This does not fix our tests, which will also break when those utility functions go away. See
#81
Closes #80