feat(protractor/runner): allow multiple browser in test

[hankduan]

continuation of #1537

[googlebot]

CLAs look good, thanks!

[hankduan]

fyi: the test is failing because I added a new 'interaction' link in the header. #1546 removes the test's dependency on the header (which can often change), and will fix the tests.

[jonhoo]

As a minor modification to this to mitigate the concerns about confusing browser instances I raised in #1537, could we consider making the indices strings instead of numbers? This code reads much better than the 0-indexed equivalent necessary given the current patch.

// ...
it('should receive email subscriptions', function(done) {
  browser.switchTo('inbox');
  browser.get('apps/email.html#/' + email());

  browser.switchTo('compose');
  browser.get('apps/email.html#/' + email() + '/compose');
  element(by.model('email.to')).sendKeys(email());
  element(by.id('sendEmail')).click();
  browser.switchTo('inbox');

  setTimeout(function() {
    expect(element.all(by.repeater('email in emails')).count()).toBe(1);
    done();
  }, 1000);
});
// ...

Also, could it be an idea to have all tests reset to drivers[0] before each test (that is, before beforeEach is called, so it can be overridden). That way, forgetting to reset the active browser driver at the end of a test won't modify the behaviour with later tests (imagine inserting a new test in the middle of a test file, and suddenly having the following test fail).

[jonhoo]

Maybe this could be done by allowing

browser.name(0, 'inbox');
// now browser.switchTo(0) is equivalent to browser.switchTo('inbox')

[hankduan]

@jonhoo I'm going to try a different approach that you may find easier to work with. I'll have an update soon.

[hankduan]

ok, this is kind of similar to your (@jonhoo) approach. But instead of passing runner into protractor, I added another global that allows the test to dynamically create multiple instances of protractor. The name of the global is up for discussion though (@juliemr ?). It's currently called createBrowser

[jonhoo]

Ah, yes, that seems a much better way of doing it.

The only comments I have are:

• getDriver should probably be renamed to getNewDriver so that the name accurately reflects what it does
• createBrowser looks weird as a global (as you allude to in your comment). Instead, how about having a fork method on browser? It could potentially even automatically navigate to the URL of the browser object being forked for convenience..

[juliemr]

I'd be fine with something like browser.fork, I think I'd vote for a more explicit browser.forkNewDriverInstance. I think we could also do a global getNewDriverInstance.

[hankduan]

take 3:

createBrowser is now browser.forkNewDriverInstance
getDriver is now getNewDriver

There are some issues with automatically going to the current browser's url.

1. some apps with auth can't access certain urls until some actions like signin are done (cookies won't be there for the new driver)
2. tests with mock modules might not work directly without mock modules injected before navigating.
3. some people simply might not want to.

Instead browser.forkNewDriverInstance takes two optional perimeters. First to navigate to url of previous browser. Second to copy mock modules.

[jonhoo]

Sure, that makes sense.
I think I'm now pretty comfortable with the API in its current state. Great work!

[juliemr]

Add a note that getNewDriver may be called in the middle of a test.

[juliemr]

Added some nits, but overall looks good!

Example is super thorough and useful.

[hankduan]

Added some comments on the comments.

[alon1980]

Hi, I was just looking for exactly this end 2 end feature and now i see that you are already done. Do you have an estimation when it should be part of the protractor package? can i just merge it as it is locally? I am an angular/protractor newbie so sorry if my questions are out of place.

Thanks!

[hankduan]

@juliemr Addressed all comments except two. Can you look at my comment regarding the bind and resetUrl?

@alon1980 I should be able to check it in today. Once I do that, you can checkout this repo to use the master branch. Otherwise the next release will probably be in 2 weeks or so.

[hankduan]

Addressed all comments

[hankduan]

merged with 0bbfd2b