Protractor: Am I overusing promise.then()

Question

I'm working on an end-to-end test using Protractor. The part of the application I'm working on first uses ng-switch statements to show/hide questions in the registration process, one at a time. There's an animation between questions that gave me the hardest time. For example, attempting to load the page->go to next question->assert that an element exists was tough, among other things. The script would load the page, click the next button, then make the assert before the next slide was on screen.

What's worse is that for about half of a second between questions, both the old question and the new one existed on the DOM. The best non-sleep wait mechanism I could come up with was to do a browser.wait() that first waited for there to be two questions on the DOM, then chain another browser.wait() that waited for there to be only one question on the DOM again and then proceed from there. (this entire operation is wrapped into registerPage.waitForTransition() in the code)

The browser.wait()s were not always blocking long enough, so I ended up writing code that looks like this:

it('moves to previous question after clicking previous link', function() {
    var title;

    // Get the current slide title, then click previous, wait for transition,
    // then check the title again to make sure it changed

    registerPage.slideTitle.getText()
        .then(function(text) {
            title = text;
        })
        .then(registerPage.prevLink.click())
        .then(registerPage.waitForTransition())
        .then(function() {
            expect(registerPage.slideTitle.getText()).not.toBe(title);
        });
});

in order to ensure that each wait was properly completed before executing the next command. Now this works perfectly. What was happening before was that the tests would succeed 95% of the time, but would occasionally fire off the asserts or the next click action, etc. before the transition was actually 100% complete. That doesn't happen anymore, but I feel like this is almost OVERusing .then() on promises. But at the same time, it makes sense to force everything to occur sequentially since that's how interacting with a site actually works. Load the page, then wait for the next button to slide in, then make a selection, then click the next button, then wait for the next slide, etc.

Am I doing this in a completely bad-practice style or is this acceptable use of promises when using Protractor on an app with heavy animations?

I just read http://stackoverflow.com/questions/29331499/when-should-we-use-then-with-protractor-promise today and it gave me a strong understand of when to use promises. — KCaradonna, Mar 02 '16 at 20:39
Have you tried disabling all animations? (http://stackoverflow.com/questions/26584451/how-to-disable-animations-in-protractor-for-angular-js-application) — alecxe, Mar 02 '16 at 20:51
not sure about overusing. Misusing definitely. The argument(s) for `.then` should be a function. Your two middle then's wont wait for anything because of this — Jaromanda X, Mar 02 '16 at 20:54
Unless `registerPage.prevLink.click()` and `registerPage.waitForTransition()` return **functions**, you aren't waiting for them. — Madara's Ghost, Mar 03 '16 at 09:35
Thanks for all the input. the .click and .waitForTransition methods do indeed return a function that returns a promise! This code has since been reworked and now only includes a .then to assign the title text. Thanks again! — Richard Pressler, Mar 03 '16 at 17:19

Brine · Answer 1 · 2016-03-03T15:34:43.760

I like these kind of code-review-like questions, so thanks for posting.

I do think some of your .thens are unnecessary. The .click() and expect shouldn't need them, as they should be added to the controlFlow for you. The expect should also handle the promise for your getText().

The problem you're having would seem to be within your waitForTransition() method, operating outside the controlFlow. Depending on how you're handling the waits within this method, you may need to add it to the controlFlow yourself. Eg. are you calling non-webdriver commands? I've also had good luck with using Expected Conditions isClickable() in cases like these.

Additionally, I would also offload much of this code to your page object, especially when waiting is required. For example, if you add something like this to your page object:

registerPage:

this.getSlideTitleText = function() {
    return this.slideTitle.getText().then(function(text) {
        return text;
    });
};

this.clickPrevLink = function() {
    this.prevLink.click();
    return this.waitForTransition(); // fix this and the rest should work
};

then your test could be...

it('moves to previous question after clicking previous link', function() {
    var title = registerPage.getSlideTitleText();
    registerPage.clickPrevLink();

    expect(registerPage.getSlideTitleText()).not.toBe(title);
});

Through trial and tribulation, I did figure out the .then() usage part, and this code has since been fixed. However, you did solve the issue of my getSlideTitleText() method not working. I had this.getSlideTitleText = this.slideTitle.getText(); when it needs to be function() { return //handle promise } Thank you! — Richard Pressler, Mar 03 '16 at 17:17

Protractor: Am I overusing promise.then()

1 Answers1