Repository navigation
Conversation
|
Increasing minimum version of selenium and also pegging to a specific version? This does not just affect CI but also where this library is installed or did I miss something? |
|
Now selenium 2 library can only work with old Firefox driver which works only in firefox up to version 47.0.1 and selenium 2.53.6. We might need drop supporting it and switch to marionette(https://github.com/mozilla/geckodriver) |
|
Understandable but pip can handle having a minimum and maximum version (I.e. range of compatible versions). Usually we try to give flexibility to the end user. Also I think a big change like selenium version should be mentioned in commit message and PR message IMHO. |
|
It should be possible to modify existing functionality in Open Browser keyword to support Firefox with and without the marionette. Also it should be possible to make the change in such manner that the it doesn't cause changes to the keyword signature. I am thinking how to handle the change with the grid side. Should the keyword make some automatic setting with the desired capabilities or not. I am starting to think that we should not make any magic when user set the desired capabilities. Therefore I am against changing the minimum required selenium version. Also the selenium 3 should be only drop in replacement for selenium 2 and it doesn't cause changes to the selenium API. Of course we should recommended people to use latest selenium but we should not force it. |
1b1d828 to
ba0e431
Compare
aaltat
left a comment
aaltat
left a comment
There was a problem hiding this comment.
One think which I do not fully grasp with Travis, where are those browsers opened? Are they opened by Travis or are they used with SauceLabs? Where those remote_url and the desired_capabilities are coming from?
| global: | ||
| - secure: "QyY0j8C+hVhsGLv4YU0UMvLm5wBDg6xT5J0Ee77C/JAQ6vcdCMpTfWI8oDxTZb7pBhpDkYuNe747yCYF5PrdBYQbO08FilxctReuJfMXcH44rZC5q49mUmPamkvg1LkzIWuC1s9uKnjlvyDT5ojuPXZl2Vn0gIhSIg+yOyBkM1U=" | ||
| - secure: "HJaJTYauaBZODMDkbnRVsiCk+t2uVtztkkJ5V4dXSnE1uIBms2PVbufQwb+YNvzHkGheLtpnt/hLM1HhmSFgUtOCf+eumQ6kaLR2KRVEysXwWJzgZoLBidNBOy5rvAIsc9JJbDaIgYVGzyaJGTP9AMb8CyvIPE//lH10sf+GB7g=" | ||
| matrix: |
There was a problem hiding this comment.
Should we add some run with Python 3 too. Example have to runs with Chrome one with Python 2.7 and another one with Python 3.
There was a problem hiding this comment.
I can add it, but does s2l compatible with py3?
There was a problem hiding this comment.
No it is not yet compatible. But the aim for the 2.0 release, which hopefully is the next release, is to make it compatible with Python 3.
| @@ -1,3 +1,3 @@ | |||
| decorator >= 3.3.2 | |||
| selenium >= 2.32.0 | |||
| selenium == 2.53.6 | |||
There was a problem hiding this comment.
Despite my previous comments, I think this change is OK, specially with the selenium 3. Supporting old version of selenium would need extra runs from the CI and in the current model, we do not have resource to provide the support and we do not have resources to pay for the CI time (or actually pay for the cloud service to get all the browsers running)
There was a problem hiding this comment.
But instead selenium == 2.53.6 should we say selenium >= 2.53.6
| #- BROWSER=ie # will be added when we have completed SauceLabs CI | ||
| #- BROWSER=safari # will be added after #247 merged | ||
| global: | ||
| - secure: "QyY0j8C+hVhsGLv4YU0UMvLm5wBDg6xT5J0Ee77C/JAQ6vcdCMpTfWI8oDxTZb7pBhpDkYuNe747yCYF5PrdBYQbO08FilxctReuJfMXcH44rZC5q49mUmPamkvg1LkzIWuC1s9uKnjlvyDT5ojuPXZl2Vn0gIhSIg+yOyBkM1U=" |
There was a problem hiding this comment.
Are the Sauce Labs hash keys to access the account. You have the clear passwords saved in somewhere? Perhaps we should distribute the password with wider audience but not put the passwords in git repo.
Is there reason why the are two secure lines?
There was a problem hiding this comment.
Yes, these are encoded login and password for sauce labs. They can be securely stored in repo because they can be decoded only in travis env.
There was a problem hiding this comment.
OK, this is good to go then.
| self._current_browser()).\ | ||
| move_to_element(element).\ | ||
| move_by_offset(xoffset, yoffset)\ | ||
| .click().perform() |
There was a problem hiding this comment.
It could be cleaner to write:
action = ActionChains(self._current_browser())
action.move_to_element(element)
action.move_by_offset(xoffset, yoffset)
action.click().perform()
| def page_should_contain_image(self, locator, message='', loglevel='INFO'): | ||
| """Verifies image identified by `locator` is found from current page. | ||
|
|
||
| See `Page Should Contain Element` for explanation about `message` and |
There was a problem hiding this comment.
I think empty line should have not been removed.
| Create Webdriver Creates Functioning WebDriver | ||
| [Documentation] LOG 2:1 INFO REGEXP: Creating an instance of the \\w+ WebDriver | ||
| ... LOG 2:4 DEBUG REGEXP: Created \\w+ WebDriver instance with session id (\\w|-)+ | ||
| [Tags] Known Issue Chrome Known Issue Internet Explorer Known Issue Safari |
There was a problem hiding this comment.
Should we document the tags we use, so that people contributing could also use them right?
@aaltat, @HelioGuilherme66, @pekkaklarck please, have a look.