-
Notifications
You must be signed in to change notification settings - Fork 770
849 verify text case insensitive #1043
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
849 verify text case insensitive #1043
Conversation
…match to test it is false case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code generally looks good, expect the code block which we do not usually use. But there should be few more test done before the PR can be merged.
actual = self.find_element(locator).text | ||
actual = actual_before = self.find_element(locator).text | ||
expected_before = expected | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not in favor of code blocks. Please remove the empty line from this line and from lines 71 and 76.
@@ -56,35 +56,54 @@ def element_should_contain(self, locator, expected, message=None): | |||
The ``message`` argument can be used to override the default error | |||
message. | |||
|
|||
The ``ignore_case`` argument can be set to True to compare case | |||
insensitive, default is False. | |||
|
|||
Use `Element Text Should Be` if you want to match the exact text, | |||
not a substring. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In new line, add text: ignore_case
argument is new in SeleniumLibrary 3.1.
Please fix in keywords where the new argument is added.
""" | ||
actual = self.find_element(locator).text | ||
expected_before = expected | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not in favor of code blocks. Please remove the empty line from this line and from line 99.
Run Keyword And Expect Error | ||
... The text of element 'some_id' should have been 'inside' but it was 'This text is inside an identified element'. | ||
... Element Text Should Be some_id inside | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty line here is not needed.
@@ -119,15 +119,20 @@ Page Should Not Contain Element With Disabling Source Logging | |||
|
|||
Element Should Contain | |||
Element Should Contain some_id This text is inside an identified element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add line: | Element Should Contain | some_id | This text is inside an identified element | ignore_case=False |
|
||
Element Should Not Contain | ||
Element Should Not Contain some_id This text is not inside an identified element | ||
Element Should Not Contain some_id THIS TEXT is not inside an identified element ignore_case=False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please add line:
Element Should Not Contain | some_id | This text is not inside an identified element | ignore_case=True |
- Please also add new test where
Element Should Not Contain
whenignore_case
argument is used with True and False values but the keyword fails. In the test useRun Keyword And Expect Error
to verify that correct error is raised.
I have fixed the code according to suggestions and ready for final review. @aaltat |
Added code and tests to resolve #849