-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat(protractor): add support for promises in configs #1040
Conversation
Would love to have this feature in please as we'd love to be able to dynamically assign hosts and proxy servers into the config. |
Could someone take a look at this? |
key, | ||
type; | ||
|
||
for (key in config) { |
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 this is a bad idea - if we extend onPrepare to be allowed to return a promise, this would try to wait for that too, for example. Let's be more specific about exactly what we're waiting on.
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.
To clarify - only waiting for seleniumAddress seems reasonable. Not sure why you would asynchronously pick a browser, or anything else.
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.
Dynamic proxy support (port assignment) would be for browsers for capabilities. Should we limit the change to only seleniumAddress, capabilities and multiCapabilities then?
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'm being dense - what does port assignment have to do with the browser? Could you give an example of the config you'd like to be able to write?
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.
Sure:
...
baseConfig.multiCapabilities = [{
browserName: 'firefox' ,
version: 'latest' ,
//To run the browser tests through a proxy server, use the proxy variable
//Specify the proxyType as 'manual' to use proxy or 'direct' to skip proxy
proxy: { //would be an async call to start a proxy(s) which would return the host/port; the proxies can have different rewrite rules, header rewrites that need to happen, etc that is specific for the tests
proxyType: 'manual',
httpProxy: 'localhost:10000',
sslProxy: 'localhost:10000'
},
shardTestFiles: true , // set if specs should be shard across the webdriver instances (of the same set of capabilities)
maxInstances: 3 // # simultaneous webdriver instances
}];
We have a service that dynamically assigns a proxy host and port and would need to feed that into the capability object along with another service for getting a seleniumAddress to use.
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.
Ah that proxy. Thanks.
If this isn't being used for just seleniumAddress
, I'm concerned about the confusion that it will only work with the hosted provider. It would be hard to debug why your capabilities promise doesn't resolve if you're testing on saucelabs, for example.
So, I propose only waiting for seleniumAddress
here, and adding the ability for capabilities
to be a promise in runner.js here - https://github.com/angular/protractor/blob/master/lib/runner.js#L221
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.
Sounds good - the proxy additions can wait, but seleniumAddress
is a must-have. We'll update the PR next week as my colleague (Sam) is out for the week. Thanks!
Change driverProviders/hosted to resolve promise values in configuration to allow async jobs in setup. Primarily, this would be for a network call to acquire a selenium host or to start a proxy server.
@juliemr I've made those adjustments according to your suggestions (and rebased the changes into a single commit) |
driver = self.driverprovider_.getDriver(); | ||
return q.fcall(function() {}); |
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.
You need to do this before getting the driver, or else it fails. I'll fix it for you.
Made the changes I mentioned and merged as 316961c |
👍 |
@samlecuyer Can you explain why |
@hankduan |
@samlecuyer Follow up. Is there a reason that you need the promises (capabilities and seleniumAddress) to resolve after i.e. do you have to wait for some condition before those promises can be resolved? |
Please make note of change happening in #1629.
|
Change driverProviders/hosted to resolve promise values
in configuration to allow async jobs in setup. Primarily,
this would be for a network call to acquire a selenium host
or to start a proxy server.