Skip to content

Add ignore_case argument to element_should_contain
 #1034

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

Closed
wants to merge 4 commits into from

Conversation

SidePlace
Copy link

No description provided.

@SidePlace
Copy link
Author

Trying to help on #849
Forgot to mention bug ID in the commit message

@aaltat
Copy link
Contributor

aaltat commented Jan 25, 2018

Code looks OK, but PR is missing test. PR are not merged without test backing them up.

@SidePlace
Copy link
Author

Hi @aaltat I just pushed my tests for it. Let me know if I'm doing something improperly.

@aaltat
Copy link
Contributor

aaltat commented Jan 26, 2018

We have had lately problems with the Travis CI, sometimes the test execution stops. It seems that this PR is also suffering from the same problem. I did restart the two failed runs and lets see how it goes.

Copy link
Contributor

@aaltat aaltat left a comment

Choose a reason for hiding this comment

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

There are two things to fix.

@@ -0,0 +1,19 @@
*** Settings ***
Copy link
Contributor

Choose a reason for hiding this comment

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

There already is a existing suite for this keyword, in ./test/acceptance/keywords/content_assertions.robot You have two options:

  1. Move tests from here to the content_assertions suite
  2. Move the Element Should Contain keyword test all in same and new suite. Also change the suite name to describe the whole keyword, not just the new feature.

Test Teardown Close All Browsers

*** Variables ***
${ISSUE_URL} https://github.com/robotframework/SeleniumLibrary/issues/849
Copy link
Contributor

Choose a reason for hiding this comment

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

Using links to external resources are not allowed. The acceptance test runner starts small http server. which must be used to serve the required html files. Please take an example from the ./test/acceptance/keywords/content_assertions.robot suite.

The test logic is OK,

@aaltat
Copy link
Contributor

aaltat commented Jan 27, 2018

If you want credit for your work, please add your name and short description what you have done in the https://github.com/robotframework/SeleniumLibrary/blob/master/CHANGES.rst

@aaltat
Copy link
Contributor

aaltat commented Feb 12, 2018

Closing on favour of #1043.

@aaltat aaltat closed this Feb 12, 2018
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.

3 participants