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 1 commit
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
39 changes: 26 additions & 13 deletions src/Selenium2Library/keywords/_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -602,41 +602,54 @@ def page_should_not_contain_image(self, locator, message='', loglevel='INFO'):
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))
return str(count)
if xpath.startswith('//'):
xpath = "xpath=" + xpath

if xpath.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.

The detection of the xpath locator prefix is both case- and space-sensitive. This is not consistent with S2L.

count = len(self._element_find(xpath, False, False))
return str(count)
else:
raise ValueError("Element locator must be xpath and start with xpath= or //")

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, locator, expected_xpath_count, message='', loglevel='INFO'):
Copy link
Member Author

Choose a reason for hiding this comment

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

I left the argument in Get Matching Xpath Count as xpath but changed the argument in Xpath Should Match X Times to locator. Here I changed it so one wouldn't get an Robot Framework positional argument error. If, as in one of the acceptance tests, you wrote

  Xpath Should Match X Times xpath=//input[@type="text"] ${1}

and the argument was xpath, Robot Framework would think you were placing a named/optional argument, xpath, before a positional argument of expected_xpath_count.

They don't have to be the same. For Get Matching Xpath Count since there is only one argument, which I thought xpath was more descriptive, I left it. To avoid the unintentional error I described above I changed the argument to locator under Xpath Should Match X Times.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to avoid an issue with the named argument processing of RF, I would suggest calling the argument xpath_, rather than locator.
Adding a trailing underscore is the PEP 8 suggestion for when there is a conflict with Python Keywords.
BuiltIn's Sleep and Get Time keywords use a trailing underscore on the time argument.

"""Verifies that the page contains the given number of elements located by the given `locator`.

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 `locator` 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 locator.startswith('//'):
locator = "xpath=" + locator

if locator.startswith('xpath='):
actual_xpath_count = len(self._element_find(locator, False, False))
else:
raise ValueError("Element locator must be xpath and start with xpath= or //")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a backwards incompatible change. Previously, the XPath did not have to start with //. For example:
${count}= | Get Matching Xpath Count | (//a)
I doubt many tests out there use parentheses in this context, though.
Another possible breakage is leading space in the XPath. That is working now, but will fail after this change.


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)
%(locator, 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, locator))

# 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
... ValueError: Element locator must be xpath and start with xpath= or //
... 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