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

Allow locator strategy of xpath= for matching xpath keywords. #527

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
Release Notes
=============

1.8 (unreleased)
----------------
- Allow xpath= locator strategy for mmatching xpath keywords [emanlove]

1.7.4
-------------------
- Reverted 'Press Keys' because of backwards incompatible changes [zephraph]
Expand Down
33 changes: 20 additions & 13 deletions src/Selenium2Library/keywords/_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -599,44 +599,51 @@ def page_should_not_contain_image(self, locator, message='', loglevel='INFO'):

# Public, xpath

def get_matching_xpath_count(self, xpath):
def get_matching_xpath_count(self, xpath_):
"""Returns number of elements matching `xpath`

One should not use the xpath= prefix for 'xpath'. XPath is assumed.
As of release 1.8 `Get Matching Xpath Count` allows the
argument `xpath` to start with the xpath= locator strategy.

Correct:
Example:
| count = | Get Matching Xpath Count | //div[@id='sales-pop']
Incorrect:
| count = | Get Matching Xpath Count | xpath=//div[@id='sales-pop']

If you wish to assert the number of matching elements, use
`Xpath Should Match X Times`.
"""
count = len(self._element_find("xpath=" + xpath, False, False))
if not xpath_.lower().lstrip().startswith('xpath='):
xpath_ = "xpath=" + xpath_

count = len(self._element_find(xpath_, False, False))
return str(count)

def xpath_should_match_x_times(self, xpath, expected_xpath_count, message='', loglevel='INFO'):
"""Verifies that the page contains the given number of elements located by the given `xpath`.
def xpath_should_match_x_times(self, xpath_, expected_xpath_count, message='', loglevel='INFO'):
"""Verifies that the page contains the given number of elements located by the given `xpath_`.

One should not use the xpath= prefix for 'xpath'. XPath is assumed.
As of release 1.8 `Xpath Should Match X Times` allows the
argument `xpath_` to start with the xpath= locator strategy.

Correct:
Example:
| Xpath Should Match X Times | //div[@id='sales-pop'] | 1
Incorrect:
| Xpath Should Match X Times | xpath=//div[@id='sales-pop'] | 1

See `Page Should Contain Element` for explanation about `message` and
`loglevel` arguments.
"""
actual_xpath_count = len(self._element_find("xpath=" + xpath, False, False))
if not xpath_.lower().lstrip().startswith('xpath='):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is good enough. But wanted to point out that the prefixes as seen in UT test_find_with_sloppy_prefix wouldn't all work here. lstrip would not remove spaces in the prefix or before the equals sign. ElementFinder._parse_locator partitions on = and strips both parts. The prefix is not space-sensitive because it does a lookup in a dict that ignores space.
I think this is good enough but wanted to point out that its still not the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just a note that I think my code should be fixed as @ombre42 pointed out before you merge @aaltat. I should be able to look at it sometime this week or if anyone else does please feel free.

Choose a reason for hiding this comment

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

Just to think we are that close to support locators with the strategy in, I'm getting sick of removing it from my locator vars to verify the correct number of elements are present 😄
All joking aside, what is needed to get the PR merged? This will be a really helpful improvement; thanks a lot in advance 👍

xpath_ = "xpath=" + xpath_

actual_xpath_count = len(self._element_find(xpath_, False, False))

if int(actual_xpath_count) != int(expected_xpath_count):
if not message:
message = "Xpath %s should have matched %s times but matched %s times"\
%(xpath, expected_xpath_count, actual_xpath_count)
%(xpath_, expected_xpath_count, actual_xpath_count)
self.log_source(loglevel)
raise AssertionError(message)
self._info("Current page contains %s elements matching '%s'."
% (actual_xpath_count, xpath))
% (actual_xpath_count, xpath_))

# Public, custom
def add_location_strategy(self, strategy_name, strategy_keyword, persist=False):
Expand Down
6 changes: 5 additions & 1 deletion test/acceptance/keywords/content_assertions.robot
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,12 @@ Xpath Should Match X Times
[Setup] Go To Page "forms/login.html"
Xpath Should Match X Times //input[@type="text"] 1
Xpath Should Match X Times //input[@type="text"] ${1}
Xpath Should Match X Times xpath=//input[@type="text"] ${1}
Run Keyword And Expect Error
... Xpath //input[@type="text"] should have matched 2 times but matched 1 times
... InvalidSelectorException: Message: The given selector id=//input[@type="text"] is either invalid or does not result in a WebElement. *
... Xpath Should Match X Times id=//input[@type="text"] 2
Run Keyword And Expect Error
... Xpath xpath=//input[@type="text"] should have matched 2 times but matched 1 times
... Xpath Should Match X Times //input[@type="text"] 2

Locator Should Match X Times
Expand Down
2 changes: 2 additions & 0 deletions test/acceptance/keywords/elements.robot
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ Get Matching XPath Count
[Documentation] Get Matching XPath Count
${count}= Get Matching XPath Count //a
Should Be Equal ${count} 19
${count}= Get Matching XPath Count xpath=//a
Should Be Equal ${count} 19
${count}= Get Matching XPath Count //div[@id="first_div"]/a
Should Be Equal ${count} 2

Expand Down