Repository navigation
Change between 1.8.0 and 3.0.0 when using Wait Until Element Is Not Visible. In 3.0.0 element is consider visible if the element does not exist in the DOM. #1008
Description
When I have this:
*** Settings ***
Library Selenium2Library
*** Test Cases ***
Wait Untill Not Visible
Open Browser https://www.google.fi/ Chrome
Wait Until Page Contains Element id=lst-ib
Wait Until Element Is Not Visible id=foobarNotVisible
[Teardown] Close All Browsers
Then wen I run it with 1.8.0 release, then the test passes. But when I run the test with 3.0.0 release the test fails. More details in logs_with_differet_versions.zip. In the 1.8.0 the element is considered as not visible when the element does not exist in the DOM. But there is change in the 3.0.0 release and element is not considered as invisible if the element is not int he DOM.
It would be best consider element invisible also when the element is not in the DOM.
Activity
pekkaklarck commentedon Dec 4, 2017
Could you clarify?
pekkaklarck commentedon Dec 4, 2017
Read the discussion on the mailing list. Did I got it right that with earlier versions
Wait Until Element Is Not Visiblesucceeded if element didn't exist at all, but nowadays it needs to exist and be invisible? If yes, I would say the old behavior makes more sense. From the end user perspective it doesn't really matter is an element invisible or not there at all.Assuming I've understood this correctly, I would say we need 3.0.1 release with a fix soon but I don't think an immediate release is necessary. Better to wait at least few days to see are other similar issues encountered. The title and possibly also the description should also be updated to describe the problem a bit better.
aaltat commentedon Dec 4, 2017
Yes, you are correct.
I also agree that it should not matter that is element in the DOM or not. If the element is not in the DOM, it is not visible and keyword should not fail.
boakley commentedon Dec 4, 2017
I disagree. To be consistent with other keywords, I think if the element is not present in the DOM then the keyword should throw an error.
Consider the case where you have a typo in the locator. Do you want the keyword to just silently pretend the locator is there and not visible? I think if you give it an invalid locator, it should tell you.
boakley commentedon Dec 4, 2017
Perhaps that is true, but from the perspective of the test developer, that's a critical difference. If I have a typo in my locator, I don't want the keyword to silently accept the locator as valid. Think of the simple case of
id=progresinstead ofid=progress. If we silently assume that no element == invisible element, the test might continue on even though a progress indicator is still visible.geist-scm commentedon Dec 4, 2017
I like the behavior that if not in DOM than not visible. I was using it this way as it made some logic simpler.
For example on the embedded systems I test on some are not very powerful; it can take up to 10 secs to generate keys for https negotiation. This can causes delays like this:
In this case my logic waits an empirically determined time than checks to see if lost contact pop-up is there. This logic started failing on the faster embedded h/w with SeleniumLibrary 3.0.0 because the pop-up never shows up.
I moved to kw 'Wait Until Page Does Not Contain Element' for fix but I am concerned this will fail when I start testing on the slower h/w Because of the way our SPA GUI works; If a popup is used and then removed thru click or change in state (polling succeeded) its html is left in the DOM but marked so it is not visible. The next time some event causes a complete page redrawn the old pop-up html is removed.
So if Wait...Not Visible fails when not in DOM I will have to change logic to:
Workable but not as simple.
-Paul.
boakley commentedon Dec 4, 2017
geist-scm commentedon Dec 4, 2017
No. That reflects a bug in the code and I am not one that thinks the language should try to protect one against such things. (just my opinion, old UNIX guy ;-) )
In my case if I used the wrong locator and the Wait passed when it should not another error would happen since the element would still be there. In my case most are pop-ups that are modal.
aaltat commentedon Dec 4, 2017
To check that something is not there is generally a difficult problem. I understand both views and can think use cases for both implementations. Perhaps it would be best to make it configurable, should the keyword fail, if the element is not in the DOM.
geist-scm commentedon Dec 4, 2017
I had also thought about making it configurable but hesitated to suggest something that is more work. I like this idea.
pekkaklarck commentedon Dec 4, 2017
I'm in general fine with either behavior, but for backwards compatibility reasons think we should restore the old behavior. Most importantly the behavior needs to be documented.
I'm not really against configuration, but don't think it's
worth the effort. If the old behavior is the default, possible typos in locators aren't detected anyway. If you really want to test element exists but is invisible (are there use cases for this?) it's easy by using this kw in combination with
Page Should Contain Elementor similar. Again, documentation is the key.aaltat commentedon Dec 4, 2017
The #1009 proposes a quick fix to the problem, but I am not happy with all the code so it needs some more work. I did look also at the
Element Should Not Be Visiblekeyword and it does not suffer from the same problem. The #1009 does not make the behavior configurable, it restores the functionality in 1.8.0 and new creates tests to ensure that functionality works.I need to think little bit more is it worth making the behavior configurable or not. And then do the rest of the changes in the code.
@geist-scm could you test the PR to see does it solve your problem? You can checkout the PR locally and run
python setup.py install.HelioGuilherme66 commentedon Dec 4, 2017
Could we add an option, say
strict=Falseby default would keep old behaviour and if True would error if element not in DOM?geist-scm commentedon Dec 5, 2017
Tatu, Yes, I will test it.
geist-scm commentedon Dec 5, 2017
Tatu, pardon my ignorance, I have not done this before. I tried to follow the steps in PR locally but it states there should be a link 'command line instructions' in the merge box but I do not see that.
At this point I have cloned the repo locally but stuck at this point.
-Paul
pekkaklarck commentedon Dec 5, 2017
Did you checkout the PR? You can then install it by using
python setup.py install. I think you can install a PR using pip without even creaking a local clone. There ought to be instructions on the Internet somewhere.aaltat commentedon Dec 5, 2017
Pip is so versatile now days, @geist-scm you can install with pip by doing this:
pip install git+git://github.com/aaltat/SeleniumLibrary.git@fix_is_visible_keywordsPlease be patient with the installation, pip will checkout the repository to temp directory and SeleniumLibrary repo is quite big so it will take some time.geist-scm commentedon Dec 5, 2017
I used PIP as Tatu outlined and reverted my code in the two places I had 'fixed' back to Wail...Visible kw and it worked with SL 3.0.1.dev1 fix. Yea.
Pekka, my experience with GIT is still low and I never had to deal with a Pull Request, but I desired to learn. In my cloned repo of SL I can list all of the tags and all of the branches (both remote and local) but do not see what I would use with the checkout cmd. So how would I have completed the cmd to get the PR 1009:
git checkout ?
For reference here is the env I tested under showing the SL 3.0.1.dev1 code base:
Linux Dist Ubuntu 16.04.3 LTS
Linux Kernel 4.4.0-97-generic
Git commit eb4eb02
Virtualenv 15.0.1
Python Python 2.7.12
RF 3.0.2
RF Selenium 3.0.1.dev1
Selenium 3.8.0
Requests 2.18.4
EasySNMP 0.2.5
Poster 0.8.1
Chome 61.0.3163.100
Chrome Webdriver 2.33.506092
Thanks for your time,
Paul
aaltat commentedon Dec 5, 2017
Good that the fix did work. I think I will make the behaviour configurable in both
... Not Visiblekeywords, because there is need for it. Just need to think little bit how the code should look like and should the situations be logged differently.pekkaklarck commentedon Dec 5, 2017
Are you sure there's need for configuration? Nobody asked it earlier when non-existing elements were considered non-visible.
boakley commentedon Dec 5, 2017
SmashManiac commentedon Dec 8, 2017
This issue caused more than half of my test suite to suddenly fail. I have to revert to v1.8.0 as a workaround. A quick resolution would be appreciated.
geist-scm commentedon Dec 9, 2017
Not sure if Tatu Aaltat would mind but the PIP install of fix branch he outlined above worked for me. See comment 3 days ago (why are comments not labeled ;-) )
pip install git+git://github.com/aaltat/SeleniumLibrary.git@fix_is_visible_keywords
It installed RF Selenium 3.0.1.dev1
Fixes Wait Until Element Is Not Visible keyword (#1009)
[-]Regression between 1.8.0 and 3.0.0 when using Wait Until Element Is Not Visible[/-][+]Change between 1.8.0 and 3.0.0 when using Wait Until Element Is Not Visible. In 3.0.0 element is consider visible if the element does not exist in the DOM.[/+]