-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat(launcher): allow multicapabilities to take array of promises #1629
Conversation
CLAs look good, thanks! |
Tests? |
@@ -104,6 +104,9 @@ exports.config = { | |||
// https://code.google.com/p/selenium/wiki/DesiredCapabilities | |||
// | |||
// In addition, you may specify count, shardTestFiles, and maxInstances. | |||
// | |||
// Capabilities can be a promise, which is resolved immediately after | |||
// `beforeLaunch` is run, and before any driver set up. |
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.
maybe "Capabilities can be a promise, which will be resolved before any driver set up. "
Since we don't guarantee that it runs after beforeLaunch
.
e.g.
capabilities = q.delay(500).then(function() { return {} })
won't wait until beforeLaunch to run, it'll start as soon as the config file is loaded.
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.
We actually do because I specifically resolve it after beforeLaunch
. (i.e. the configParser doesn't resolve it). This is because some users might want to do some set up during beforeLauncher
.
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 think I was unclear - what I'm trying to say is that the user can't assume that their code won't be run after beforeLaunch
. Maybe I'm overthinking it.
Please update the commit message to include basic usage and note the breaking change (for the current very small set of users relying on the way config promises work at the moment). |
4dff3ed
to
8e19a49
Compare
All comments addressed |
8e19a49
to
8c10f3c
Compare
var env = require('./environment.js'); | ||
var q = require('q'); | ||
|
||
// Tests for an Angular app where ng-app is not on the body. |
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.
outdated comments?
LGTM |
Enables adding `getMultiCapabilities: function(){}` to your configuration file. The function returns either multiCapabilities or a promise of a multiCapabilities that is resolved after `afterLaunch` and before driver set up. If this is specified, both capabilities and multiCapabilities will be ignored. Also allows specifying `seleniumAddress` in the capabilities/multiCapabilities object, which will override the global `seleniumAddress`. This allows you to use a different `seleniumAddress` per capabilities. Breaking Changes: `capabilities` can no longer be a promise. Use getMultiCapabilities if you need to return a promise. `seleniumAddress` can no longer be a promise. Likewise, use getMultiCapabilities.
8c10f3c
to
8fa5e02
Compare
merged in 9c9ed31 |
Confirmed, works for me (example here). Thank you very much, guys! |
Enables adding
getMultiCapabilities: function(){}
to your configuration file.The function returns either multiCapabilities or a promise of a
multiCapabilities that is resolved after
afterLaunch
and before driver set up.If this is specified, both capabilities and multiCapabilities will be ignored.
Also allows specifying
seleniumAddress
in the capabilities/multiCapabilitiesobject, which will override the global
seleniumAddress
. This allows you touse a different
seleniumAddress
per capabilities.Breaking Changes:
capabilities
can no longer be a promise. Use getMultiCapabilities if you needto return a promise.
seleniumAddress
can no longer be a promise. Likewise, use getMultiCapabilities.