angular / protractor

E2E test framework for Angular apps
http://www.protractortest.org
MIT License
8.75k stars 2.31k forks source link

seleniumAddress cannot currently be a promise, but should be able to be #883

Closed samlecuyer closed 10 years ago

samlecuyer commented 10 years ago

HostedDriverProvider.prototype.setupEnv returns a promise, so it should be able to accept this.config_.seleniumAddress as a promise.

https://github.com/angular/protractor/blob/master/lib/driverProviders/hosted.dp.js#L25

It would be great if instead of just returning a promise on a no-op, we could check to see if something like q.isPromise(this.config_.seleniumAddress) is truthy and then return that (updating this.config_ in the process), otherwise do what we're doing already.

The use case is that the selenium hubs can sometimes be dynamically allocated based on environmental variables, so we'd like to -- rather than run a bunch of setup tasks -- call a library that could return a promise of a seleniumAddress.

Maybe someone like @juliemr or @hankduan could take a look at this?

emkay commented 10 years ago

:+1:

arjansingh commented 10 years ago

That seems fair. Using a promise would also be a cleaner and more usable interface. :+1:

rynodivino commented 10 years ago

:+1:

gtg092x commented 10 years ago

:+1:

samlecuyer commented 10 years ago

Something like this should be sufficient:

diff --git a/lib/driverProviders/hosted.dp.js b/lib/driverProviders/hosted.dp.js
index 7f20315..35f7541 100644
--- a/lib/driverProviders/hosted.dp.js
+++ b/lib/driverProviders/hosted.dp.js
@@ -21,8 +21,11 @@ var HostedDriverProvider = function(config) {
  *     ready to test.
  */
 HostedDriverProvider.prototype.setupEnv = function() {
-  util.puts('Using the selenium server at ' + this.config_.seleniumAddress);
-  return q.fcall(function() {});
+  var config = this.config_;
+  return q.when(config.seleniumAddress, function(resolvedAddress) {
+    util.puts('Using the selenium server at ' + config.seleniumAddress);
+    config.seleniumAddress = resolvedAddress; 
+  });
 };

 /**

Any chance of that?

juliemr commented 10 years ago

Feature added in 316961c6a5d7410d73a2784a9622b106008b0930, it'll be out in the next version.