Skip to content
This repository has been archived by the owner on Jul 29, 2024. It is now read-only.

expose driver functionality #8

Closed
geddski opened this issue Jun 14, 2013 · 8 comments
Closed

expose driver functionality #8

geddski opened this issue Jun 14, 2013 · 8 comments

Comments

@geddski
Copy link

geddski commented Jun 14, 2013

When wrapping a driver, shouldn't protractor expose the underlying driver methods as well as angular-specific ones? For example:

ptor.get("http://www.smashingmagazine.com");
ptor.getTitle().then(function(title) {
  expect(title).to.equal('Smashing Magazine — For Professional Web Designers and Developers');
  done();
});    

getTitle is a method on the wrapped driver, but not available after being wrapped.

@juliemr
Copy link
Member

juliemr commented Jun 14, 2013

I'd considered this and wasn't sure what would be most intuitive to people - truly wrapping the driver, or keeping both around. I take this as a vote for the former, and I think I prefer that as well. I'd love to hear others' votes, too.

@ghost ghost assigned juliemr Jun 14, 2013
@geddski
Copy link
Author

geddski commented Jun 16, 2013

I agree, I think keeping both around would be good. That way some e2e tests could simply use the driver for non-angular pages.

Currently when I try to use them side by side like so:

ptor.get("http://www.smashingmagazine.com");    
driver.getTitle().then(function(title) {...

It errors out:

UnknownError: Error Message => 'Detected a page unload event; asynchronous script execution does not work across page loads.'

After we fix that, maybe we just add a driver property to the ptor instance so it doesn't have to be passed around manually:

ptor.get("http://www.smashingmagazine.com");    
ptor.driver.getTitle().then(function(title) {...

Thoughts?

EDIT: looks like there already is a driver property on the instance. So just fixing the other issue may be sufficient.

@juliemr
Copy link
Member

juliemr commented Jun 20, 2013

Heya,

Added full wrapping on the 'wrapfix' branch with 40d8a7a. Does this look sane (and fix your issue?)

@tbosch
Copy link

tbosch commented Jun 24, 2013

Hi Julie,
I don't know webdriver in detail, but maybe you also need to preserve the this pointer in the webdriver functions to point to the webdriver instance, e.g.

// Mix all other driver functionality into Protractor.
for (var foo in webdriver) {
   if(!this[foo]) {
     this[foo] = webdriver[foo].bind(webdriver);
   }
}

Furthermore, for chaining functions, your mixed in functions should return the protractor instance, and not the webdriver instance, e.g.

// Mix all other driver functionality into Protractor.
for (var foo in webdriver) {
   if(!this[foo]) {
     mixin(this, webdriver, foo);
   }
}

function mixin(self, webdriver, fnName) {
    self[fnName] = function() {
        var res = webdriver[fnName].apply(this, arguments);
        if (res===webdriver) {
            res = self;
        }
        return res;
    }
}

Does this make sense?
Tobias

@juliemr
Copy link
Member

juliemr commented Jun 24, 2013

Tobias - thanks for the comments, can't believe I forgot to fix the binding.

However, on second thought, it might be best to really just delegate to webdriver, since there aren't any non-function members of the webdriver object that I want to copy to the protractor object. For example:

for (var foo in webdriver) {
  if(!this[foo] && typeof webdriver[foo] == "function") {
    this[foo] = function() {
      return webdriver[foo](webdriver, arguments);
    };
  }
}

Any comments on this style?

@juliemr
Copy link
Member

juliemr commented Jun 24, 2013

Alternative B: Decorate the webdriver instance, instead of creating a separate protractor object.

Protractor.wrapDriver = function(webdriver) {
  webdriver.extend(Protractor.prototype);
};

@juliemr
Copy link
Member

juliemr commented Jun 25, 2013

Implemented Tobias's suggestion (minus chaining, since there is no chaining in webdriverjs) with 1ef76dc

@juliemr
Copy link
Member

juliemr commented Jun 25, 2013

Merged into master. Thanks for the comments, everyone!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants