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

Consider making a library of retry helper methods for situations outside of Angular's control #1557

Closed
juliemr opened this issue Nov 22, 2014 · 18 comments

Comments

@juliemr
Copy link
Member

juliemr commented Nov 22, 2014

I'm not sure if these belong in Protractor core or not, but let's consolidate discussion under this issue.

This would be functions like:

waitUntilElementPresent(elem);
waitUntilClickable(elem);
waitUntilUrlIs('foo.com');

Needing these can be red flags, because they mean that synchronization isn't working - but that may be true for websites outside of Angular (e.g. login pages).

@marklagendijk
Copy link
Contributor

Hey Julie,
You might remember me. I talked to you at ngEurope about whether we should start using Protractor in our company (Connectis, company for online identity / authorizations management).
When I first started to use Protractor I quickly ran into a problem, which made me dislike Protractor. Here is what happened.

I started to write tests for our application. The first part of the flow (the loging in part) is outside of Angular. Somewhere I read that to make Protractor work with stuff outside of Angular you have to use:

beforeEach(function(){
  browser.ignoreSynchronization = true;
});

Because the example said that you have to put in in the beforeEach I assumed that the property would automatically be reset to false. So I wrote my test like this:

describe('logging in', function(){
  beforeEach(function(){
    browser.ignoreSynchronization = true;
  });
  // Logging in flow
});

describe('Angular application', function(){
  // Angular application tests
});

This problem got me quite a headache, because my Angular tests where not synchronized anymore. The only way I managed to solve it is by writing functions like waitUntilElementPresent.
Luckily I later found out my mistake, and could clean up my tests. With this problem fixed, I really like Protractor again :).

To prevent any other people for making the same mistake it might be good to make very clear that:

  • You have to set browser.ignoreSynchronization to false again as soon as your tests enter your Angular application.
  • Make very clear that the helper functions which you propose, should only be used when testing stuff outside of Angular. This could be done, for example, by putting them in a seperate namespace like browser.noAngular (but then a better name).

When that would be taken care of, I would really think that these utils might be handy to have.

Mark

@jnizet
Copy link
Contributor

jnizet commented Dec 10, 2014

I'm in favor of such additions. In particular, even in pure Angular apps, I had some synchronization issues due to pure CSS animations. For example, angular-ui-bootstrap has modal dialogs that appear using a sliding CSS animation, and tests randomly fail when trying to locate an element in the modal because the element to locate is not clickable/visible yet due to the animation.
The trick I used was to disable the animation by having additional CSS in the HTML page used by e2e tests, but this is painful, and specific to that case.
Accordions have the same problem.

@elgalu
Copy link
Contributor

elgalu commented Dec 13, 2014

@marklagendijk where exactly in the official docs is suggested to put ignoreSynchronization in beforeEach? You might have read it in one of my StackOverflow answers, if so, sorry about that I've just updated them to remove that comment.

For the browser.noAngular idea note there is browser.driver already, maybe the name by itself doesn't say much about the intent.

@hankduan
Copy link
Contributor

hankduan commented Jan 9, 2015

Didn't make a waitFor for each condition, but wrote conditions that you can plug into browser.wait instead: #1703

@davidadas
Copy link

Honestly, I think only the waitUntilClickable need be considered. The issue with elements not immediately being clickable at (x, y) due to non-ngAnimate animations can get frustrating (especially with the odd work-arounds that become of it).

The wait method that was added is okay, but it would be nice if Protractor could wait until an element is clickable from within the click method if not otherwise available. Then let it throw an error if the element is still un-clickable after the designated timeout

@bitwit
Copy link

bitwit commented Mar 17, 2015

+1 that the frameworks couple use some sort of isElementClickable.

We have an angular application that is only a part of a greater non-angular application, so we keep ignoreSynchronization = true. As a result, the ng-animate animations seem to bust tests and I need to add arbitrary browser.sleeps to account for transition times

@hankduan
Copy link
Contributor

It's already there. It's called elementToBeClickable

This is straight from the example:

* @example
 * var EC = protractor.ExpectedConditions;
 * var button = $('#xyz');
 * var isClickable = EC.elementToBeClickable(button);
 *
 * browser.get(URL);
 * browser.wait(isClickable, 5000); //wait for an element to become clickable
 * button.click();

@hankduan
Copy link
Contributor

fyi, the API names are make consistent with the java/python selenium expected conditions (although I think some could sound a little weird)

Closing this issue since it's already addressed and released.

@bitwit
Copy link

bitwit commented Mar 17, 2015

@hankduan Thank you, great to hear! Could you please point to the documents you extracted them from though? I'm trying your example and wanted to read deeper (it's not working out of the box, but I'm inclined to blame myself first).

@hankduan
Copy link
Contributor

Unfortunately it's not part of the website (note to self: add to API page), but you can read about it in the src https://github.com/angular/protractor/blob/master/lib/expectedConditions.js

What's not working? (fyi, your protractor needs to be version 1.7.0 or above)

@bitwit
Copy link

bitwit commented Mar 17, 2015

Thanks again, a little more tweaking and I got it running. ^Handy read too

@hankduan
Copy link
Contributor

fyi: issue for adding expected conditions to website: #1936

@davidadas
Copy link

Indeed, that expected condition is inherited from the native Selenium suite (or...at least it was last I checked...).

Let me take a dive into the source here though. I've been assuming (due to the speed at which my tests fail to quick hide/show animations) that the wait for clickability is not baked into the Protractor WebElement click method.

If it's not though, and you guys feel it's an acceptable enhancement to the project, I'd be happy to put in a PR :)

@davidadas
Copy link

...and if it is baked in, and my tests have been failing for some other dumb reason, then I'll happily flog the developer who made me look like an idiot ;)

@joshmgrant
Copy link

I would also second @juliemr's and @davidadas's suggestion in adding waitFor functionality into click() functions, as well as sendKeys(), clear() and others.

@jlin412
Copy link

jlin412 commented Aug 6, 2015

+1

On Thu, Aug 6, 2015 at 3:13 PM, joshin4colours notifications@github.com
wrote:

I would also second @juliemr https://github.com/juliemr's and @davidadas
https://github.com/davidadas's suggestion in adding waitFor
functionality into click() functions, as well as sendKeys(), clear() and
others.


Reply to this email directly or view it on GitHub
#1557 (comment)
.

@c-mac
Copy link

c-mac commented Aug 7, 2015

+1!

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

No branches or pull requests

10 participants