Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

visibility_of_all implies it only returns elements if all visible #2052

Merged
merged 2 commits into from
May 5, 2016
Merged

visibility_of_all implies it only returns elements if all visible #2052

merged 2 commits into from
May 5, 2016

Conversation

rouke-broersma
Copy link

@rouke-broersma rouke-broersma commented May 4, 2016

This is not what the function does.
visibility_of_any in my opinion correctly conveys the usage of this expected condition.

 it only returns elements if all elements are visible. This is not what the function does.
visibility_of_any in my opinion correctly conveys the usage of this expected condition.
@remcowesterhoud
Copy link
Contributor

If you're gonna change the name of this class you should also change the tests that use it.
You can find those at https://github.com/SeleniumHQ/selenium/blob/master/py/test/selenium/webdriver/common/webdriverwait_tests.py

 to use to visibility_of_any_elements_located
@@ -102,7 +102,7 @@ def __init__(self, locator):
def __call__(self, driver):
return _find_elements(driver, self.locator)

class visibility_of_all_elements_located(object):
class visibility_of_any_elements_located(object):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't call return a boolean?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is clearly not the author's intent and returning elements instead of a boolean is not out of line with the rest of the expected conditions.

See presence_of_element_located: https://github.com/Mobrockers/selenium/blob/f878b53d9dd66ef25444b82e50218acd36f6bc4e/py/selenium/webdriver/support/expected_conditions.py#L49
visibility_of_element_located:https://github.com/Mobrockers/selenium/blob/f878b53d9dd66ef25444b82e50218acd36f6bc4e/py/selenium/webdriver/support/expected_conditions.py#L61

etcetc

Or do you mean as a final condition if no visible elements van be found by the given locator?
In that case perhaps you're right yes.

@lukeis lukeis merged commit 27e31db into SeleniumHQ:master May 5, 2016
@rouke-broersma rouke-broersma deleted the patch-1 branch May 5, 2016 19:51
AutomatedTester pushed a commit that referenced this pull request May 18, 2016
)

* visibility_of_all implies

 it only returns elements if all elements are visible. This is not what the function does.
visibility_of_any in my opinion correctly conveys the usage of this expected condition.

* Fix visibility_of_all_elements_located test

 to use to visibility_of_any_elements_located
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants