angular / protractor

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

0.24.0 breaks custom matchers returning a promise #910

Closed andreev-artem closed 10 years ago

andreev-artem commented 10 years ago

Before 0.24 I had the following custom matcher

toHaveClass: function(cls) {
    var elemHtml;
    this.message = function() {
        return "Expected '" + elemHtml + "' to have class '" + cls + "'.";
    };

    this.actual.getInnerHtml().then(function(html){
        elemHtml = html;
    });

    return this.actual.getAttribute('class').then(function(classes){
        return classes.split(' ').indexOf(cls) !== -1;
    });
},

which could be used like

var paginationWidget = {
    backBtn: element(by.css('.m-pagination-links ul li:first-child'))
};

expect(paginationWidget.backBtn).toHaveClass('disabled');

Now due introduced changes to ElementFinder paginationWidget.backBtn is resolved to null and I'm getting null for this.actual.

I guess that example should not work either.

Is there any way to get it working in 0.24.0?

Related question: are there any plans to have ability to extend ElementFinder and ElementArrayFinder with our own DSL? It would be good to have ability to add methods like hasClass(), getVal() (instead of getAttribute('value')), etc

juliemr commented 10 years ago

Thanks for the note. This is definitely not ideal and we're working on the best way to address this in new ElementFinder-land.

Droogans commented 10 years ago

So, should we all stick with an earlier version in the meantime?

andreev-artem commented 10 years ago

I guess that is because that promise resolving. Can we resolve using WebElement instead of null?

For now we are using the following workaround:

e2e.expect = function(actual){
    jasmine.getEnv().currentSpec.actualSrc = actual;
    return global.expect(actual);
};

toHaveClass: function(cls) {
    // workaround against https://github.com/angular/protractor/issues/910
    if(this.actual === null) {
        this.actual = jasmine.getEnv().currentSpec.actualSrc;
    }

    var elemHtml;
    this.message = function() {
        return "Expected '" + elemHtml + "' to have class '" + cls + "'.";
    };

    this.actual.getInnerHtml().then(function(html){
        elemHtml = html;
    });

    return this.actual.getAttribute('class').then(function(classes){
        return classes.split(' ').indexOf(cls) !== -1;
    });
}

var paginationWidget = {
    backBtn: element(by.css('.m-pagination-links ul li:first-child'))
};

e2e.expect(paginationWidget.backBtn).toHaveClass('disabled');
Droogans commented 10 years ago

I've been trying to figure out what's been breaking my tests lately, and I think this may be the culprit.

Currently, I'm having issues with a helper functions file, which for some tasks, simply finds and returns an element to the caller. The result is null. If there could be a way to not fulfill the promise with null right away, and instead return the raw element, I think my problems would go away.

hankduan commented 10 years ago

Can you share your use case? The reason why we decided to return null instead of the raw webelement is because the raw webelement came directly from webdriver and will not sync with angular.

i.e. andreev-artem had a legitimate use case for passing an elementFinder into an expect function (which calls then), but I can see that people who do not know the difference would do this: element(by.x).then(function(elem) { elem.click(); elem.clear(); elem.getText()}); which is different from element(by.x).click(), clear(), getText() because the former does not sync with angular and could cause hard to debug race conditions (which people will think of as flakes)

If you actually want to grab a web element, you can do element(by.x).getWebElement(). (but normally you should never need to do this).

We're still trying to come up with a solution for andreev-artem's (and likewise) problem though.

Droogans commented 10 years ago

Is there anyway you could email me? I would like to do a screenshare of some interesting things I've found.

I've replaced $ and $$ with findElement and findElements, and my tests are passing.

Thanks @hankduan!

hankduan commented 10 years ago

Issue was with doing:

$(css).then(function(elem) { // do something with elem (i.e. elem.click(), which is null })

Fixed by doing $(css).click() directly

hankduan commented 10 years ago

@andreev-artem https://github.com/angular/protractor/pull/936 should resolve your problem

andreev-artem commented 10 years ago

@hankduan, thanks!