is_element_visible: Check for StaleElementReferenceException

[tony]

Robust, consistent handling for is_element_visible_by_? and is_element_not_visible_by_?. We don't want either to blink at stale references.

Prior art: consistent with #396, the function can continue to poll "freshly" without interruption by stale objects

Fixes #934

[tony]

@jsfehler this can be reran now. This should fix the test

[tony]

@jsfehler

Anything more you'd like to see for this?

rationale:

• consistency
• fixes issues of StaleElementReferenceException burping unexpectedly
• is_element_visible is is_element_not_visible isn't checking whether something is stale - it's checking whether the element does / doesn't exist

my personal rationale is the same as when #396 did it's stale checks. just making sure the ones have it too

[tony]

Another friendly hello! :)

And a rebase! 🙌

[tony]

Rebased again!

[tony]

This avoids the stale exception when checking for an element's visibility. Right now, we correctly do this in is_text_present via #396, in my change it's added to is_element_visible too.

Anyone around? Can someone please take a look?

[jsfehler]

@tony There's a few problems with this PR:
1 - You're missing unit tests for this PR. There aren't any tests to make sure is_element_visible / is_element_not_visible doesn't choke on those exceptions.
2 - There's no point letting ValueError escape here. ValueError shouldn't be raised by find_by. Unless you have a specific scenario in mind that I haven't thought of.

[tony]

@jsfehler thank you for the response!

1. In Check for StaleElementReferenceException #396, a similar PR that got merged, I don't see any tests. How would you suggest to write a test that raises StaleElementReferenceException?
2. Fixed!

[jsfehler]

@tony Having re-read this PR, I'm not sure these changes will actually have any effect.

find_by already catches NoSuchElementException and StaleElementReferenceException. I can't think of a scenario in which these checks are not redundant.

Do you have a particular scenario where is_element_visible or is_element_not_visible are currently failing?

[tony]

@jsfehler Here's the traceback: #934 (as of 0.14.0)

I agree with you, maybe I'm misunderstanding something?

I reviewed the code and StaleElementReferenceException is ultimately caught.

I haven't gotten this since, should I close this?

[jsfehler]

@tony It looks like the call to Webdriver's is_displayed() method is what's causing your failure. The best way to handle this is isolating the part that can be flaky:

while time.time() < end_time:
    is_visible = False
    element = finder(selector, wait_time=wait_time)
    if element:
        try:
            is_visible = element.visible
        except StaleElementReferenceException:
            pass
    if is_visible:
        return True

return False

[jsfehler]

Closing this due to #1064

The new behaviour should be sufficient in this case.