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

"Reusing" element promises leads to StaleElementReferenceError #1903

Closed
tullmann opened this issue Mar 5, 2015 · 9 comments
Closed

"Reusing" element promises leads to StaleElementReferenceError #1903

tullmann opened this issue Mar 5, 2015 · 9 comments

Comments

@tullmann
Copy link
Contributor

tullmann commented Mar 5, 2015

I'm trying to test how a page changes in response to user input. As such, the DOM is mutating, and I believe these mutations make any element promise from before the change suspect. Given that mutating the DOM is the Angular Way, and ExpectedConditions are supposed to help test for an element's visibility/presence, it seems like EC should be hiding this problem?

However, I'm not clear if holding on to such promises across DOM mutations is just "bad form" (and I should always re-"locate" any elements that I think may have changed/moved) or if Protractor should implicitly re-locate elements for me. (Those both seem problematic, so I think this is an EC issue?)

The Selenium page on the exception is useful/frustrating reading: http://docs.seleniumhq.org/exceptions/stale_element_reference.jsp
It really seems like the case of navigating to a different page should be separated from the case of an element being destroyed and re-created which should also be separated from the case of an element being removed from the DOM. (So maybe this is a Selenium bug?)

Here is a simplified test case that demonstrates a variant on my problem using the protractor API page. Note that the test catches exceptions from the ExpectedConditions, but I'm just doing that to count the failures (I don't think every use of ExpectedConditions should do this).

describe('demo EC bug', function() {

  var inputbox = $('#searchInput');
  var title = $('.api-title');

  it('force a StaleElementReferenceError', function() {
    browser.get('/protractor/#/api', 90 * 1000);

    expect(title.getText()).toBe('Protractor API Docs');

    // Get an array of element promises, one for each "<li>" on the page (the
    // doc links on the left side).
    var deferred = protractor.promise.defer();
    element.all(by.tagName('li')).then(function(elements) {
      var elProms = [];
      elements.forEach(function(elProm) {
        elProms.push(elProm);
      });
      deferred.fulfill(elProms);
    });

    // Tell controlFlow about the array generation promise
    protractor.promise.controlFlow().await(deferred);

    var failureCt = 0;

    // Once the array is done, put some text into the search box.  This
    // will cause most of the "<li>" elements to disappear, then when the
    // ExpectedConditions operate on the element, a
    // StaleElementReferenceError will get thrown (on some elements).
    deferred.promise.then(function(elements) {
      console.log('ELEMENTS:', elements.length);

      // This should cause all of the <li> elements on the page to
      // disappear: I believe this is effectively invalidating many of the
      // element promises in the "elements" array.
      inputbox.sendKeys('something unique and a bit long');

      elements.forEach(function(element) {
        var EC = protractor.ExpectedConditions;
        var mkProm = EC.visibilityOf(element);
        mkProm().then(function(visible) {
          console.log('Element visibility: ', visible);
        }, function(err) {
          // console.log('Element ERR: ', err.message);
          expect(err.message).toContain('stale element reference: element is not attached');
          failureCt++;
        });
      });
    }).then(function() {
      expect(failureCt).toBe(0);
    });
  });
});
@tullmann
Copy link
Contributor Author

tullmann commented Mar 5, 2015

I should've mentioned that I'm running protractor from master, and using Chrome v40.0.2214.115 on Linux. (If I run the test with Firefox v34, the same basic thing happens, but the exception message is slightly different. So my expect check is wrong with Firefox -- the test is still demonstrating the correct thing, though.)

@hankduan
Copy link
Contributor

hankduan commented Mar 6, 2015

You've hit a bug with isPresent. See reduced case below:

it('demonstrates isPresent bug', function() {
  browser.get('http://angular.github.io/protractor/#/api', 90 * 1000);

  element.all(by.tagName('li')).then(function(elements) {
    var disappearingElem = elements[100];
    expect(disappearingElem.isPresent()).toBeTruthy();
    $('#searchInput').sendKeys('something unique and a bit long')
    expect(disappearingElem.isPresent()).toBeFalsy(); // This will fail
  });
});

I would like to point it out that part of this bug is because of your usage, as I'm not sure why you have

var deferred = protractor.promise.defer();
element.all(by.tagName('li')).then(function(elements) {
  var elProms = [];
  elements.forEach(function(elProm) {
    elProms.push(elProm);
  });
  deferred.fulfill(elProms);
});
deferred.promise.then(...)

instead of the following which is shorter and much more readable:

var elemArrFinder = element.all(by.tagName('li'));
elemArrFinder.then(...)

That being said, it's still a bug nonetheless, and the fix is here: #1907

@hankduan hankduan self-assigned this Mar 6, 2015
@tullmann
Copy link
Contributor Author

tullmann commented Mar 6, 2015

Ah, that fix makes sense! Thanks for tracking this down.

Yeah, the weird usage in my example is because its reduced from some real code I have. In my real code I'm wrapping all the Modal dialogs that are on the screen, and my specs use my "page object"-like wrapper for the Modal dialogs, so I need (? I think?) the explicit promise and the forEach so I can return an array of ModalPage objects from a promise.

@hankduan
Copy link
Contributor

hankduan commented Mar 6, 2015

Not 100% sure what your set up is that requires explicit forEach and recapturing all the resolved elementFinders that you can't do with filter/map/reduce/get, but that is not really in scope of this bug. If you want suggestions over how to reduce your test to use only elementArrayFinder, post on stackoverflow and I could help you there.

@juliemr
Copy link
Member

juliemr commented Mar 16, 2015

As the fix from #1907 is in, I believe we can close this.

@juliemr juliemr closed this as completed Mar 16, 2015
@gkohen
Copy link
Contributor

gkohen commented Apr 15, 2015

@juliemr, I'm running the latest 2.0 with the fix mentioned by @hankduan. I have the following piece to make sure an element is not visible or does not exist:

var cssSelector = '.my-class';
browser.wait(ExpectedConditions.or(
                  ExpectedConditions.stalenessOf($(cssSelector)),
                  ExpectedConditions.invisibilityOf($(cssSelector))),
      5000);

I intermittently get a NoSuchElementError: No element found using locator: By.cssSelector('.my-class')
It seems like the or is not short circuiting if the first condition is true since the first condition should check if the elements are even there. Am I missing how multi condition should be used?

@hankduan
Copy link
Contributor

This should be how it's used (in fact, the invisibilityOf already checks for staleness, so you could just do:

browser.wait(ExpectedConditions.invisibilityOf($(cssSelector)), 5000);

If it's not working, then there is possibly a bug. Can you open a new issue with some code to reproduce this?

@gkohen
Copy link
Contributor

gkohen commented Apr 15, 2015

@hankduan invisibilityOf checks the status on the element like so:

invisibilityOf-->visibilityOf-->presenceOf

it seems like we don't check for stalenessOf anywhere in the chain.

@hankduan
Copy link
Contributor

stalenessof == not(presenseOf)

hankduan added a commit to hankduan/protractor that referenced this issue Jul 20, 2015
This was a regression caused by angular#1903 and
angular@2a765c7,
but was causing ElementArrayFinder.prototype.count to be slow because of
the extranous checks that must be performed. However, the checks could be
delayed into the isPresent check, which will mitigate this issue
hankduan added a commit that referenced this issue Jul 21, 2015
The bug fix for #1903
(2a765c7)
was causing ElementArrayFinder.prototype.count to be slow because of
the extranous checks that must be performed. However, the checks could be
delayed into the isPresent check, which will mitigate this issue

fixes(#2359)
@hankduan hankduan removed their assignment Nov 4, 2015
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

4 participants