Skip to content

update selenium-webdriver 3.0 and add patches to work with Appium #383

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

Merged
merged 12 commits into from
Dec 13, 2016

Conversation

KazuCocoa
Copy link
Member

@KazuCocoa KazuCocoa commented Oct 15, 2016

This PR is for selenium-webdriver 3.0.0 and 3.0.1.
Over 3.0.2, we need #413 .

motivation

Using selenium-webdriver 3.0 instead of selenium-webdriver 2.x to catch up with the latest maintained library.

What I do

Fix some issues I encountered issues I ran my test suite with selenium-webdriver 3.0.

We should use legacy Bridge to compatible with Appium server. W3CBridge, used in Selenium3 Ruby client by default, differs paths between selenium-webdriver 2.x and selenium-webdriver 3.x. So, we encounter some no route error if we use W3CBridge. For instance, the following paths are changed paths between selenium-webdriver 2.x and selenium-webdriver 3.x.

  • selenium-webdriver 2.x with Bridge
        command :setWindowSize, :post, 'session/:session_id/window/:window_handle/size'
        command :executeScript, :post, 'session/:session_id/execute'
        command :executeAsyncScript, :post, 'session/:session_id/execute_async'
  • selenium-webdriver 3.0 with W3CBridge
        command :getWindowSize, :get, 'session/:session_id/window/size'
        command :getPageSource, :get, '/session/:session_id/source'
        command :executeScript, :post, 'session/:session_id/execute/sync'
        command :executeAsyncScript, :post, 'session/:session_id/execute/async'

Fixed issues

  1. selenium-webdriver 3.0 client freeze Selenium::WebDriver::SearchContext::FINDERS. But ruby_lib extends the attribute. So, I've added search context for Appium .
  2. Selenium::Client is removed from selenium-webdriver. So, re-define iOS error in ruby_lib.
  3. Not supported and not found routes error with W3CBridge

Routes


Appium server's routes are based on mobile-json-wire-protocol. So, the commands differ from w3c one.

@KazuCocoa KazuCocoa changed the title Be able to use selenium-webdriver 3.0 and update gems available selenium-webdriver 3.0 and update gems Oct 15, 2016
@KazuCocoa
Copy link
Member Author

KazuCocoa commented Oct 15, 2016

This PR may solve Bump selenium-webdriver dependency #382

@KazuCocoa KazuCocoa changed the title available selenium-webdriver 3.0 and update gems [WIP]available selenium-webdriver 3.0 and update gems Oct 16, 2016
@KazuCocoa KazuCocoa changed the title [WIP]available selenium-webdriver 3.0 and update gems [WIP]update selenium-webdriver 3.0 and add patches to work with Appium Oct 18, 2016
@KazuCocoa KazuCocoa changed the title [WIP]update selenium-webdriver 3.0 and add patches to work with Appium update selenium-webdriver 3.0 and add patches to work with Appium Oct 18, 2016
@KazuCocoa
Copy link
Member Author

KazuCocoa commented Oct 19, 2016

I update commits and this PR works fine in my test suites.
What I do in this PR is described in description.

An important point in this PR is that using Bridge instead of W3CBridge to send previous version's commands to Appium server because Appium Server raise no route error for W3C commands .

@jlipps Could you check the patch policy?
If Appium server will support W3C command completely in the future, then we should remove patch_webdriver_driver, I already commend as TODO.

If using Bridge is OK, I'd like to review this PR's code and merge this.

@sofaking
Copy link
Contributor

Wouldn't it be more pragmatic to just wait until selenium 3 routs will be implemented in Appium server ? This PR obviously introduce some unnecessary complexity while I'm not sure what exactly Selenium 3.0 brings to the table.

@KazuCocoa
Copy link
Member Author

KazuCocoa commented Oct 19, 2016

Yes, I agree... > This PR obviously introduce some unnecessary complexity...
So, if Appium has decided to continue to use selenium-webdriver 2.x, I'd like to close this PR.
(I found introducing selenium3.0 in Java client, so I just tried to this PR)

@KazuCocoa
Copy link
Member Author

@jlipps what do you think ? => #383 (comment)

@jsf-clabot
Copy link

jsf-clabot commented Nov 1, 2016

CLA assistant check
All committers have signed the CLA.

@jlipps
Copy link
Member

jlipps commented Nov 3, 2016

We will certainly be supporting the se3-style clients in the Appium server. So I think this patch is relevant. But I'm not sure exactly when this will happen. @KazuCocoa do you have a link that explains the differences between what the se3 clients send and what the Appium server expects?

@KazuCocoa
Copy link
Member Author

KazuCocoa commented Nov 5, 2016

# ".switch_to.alert" calls getAlertText so use bridge directly
driver.send(:bridge).acceptAlert
# ".switch_to.alert" calls alert_text so use bridge directly
driver.send(:bridge).accept_alert
Copy link
Member Author

Choose a reason for hiding this comment

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

Some methods in selenim-webdriver 3.0 are renamed.

@@ -552,7 +552,7 @@ def _by_json(opts)
JS

res = execute_script element_or_elements_by_type
res ? res : fail(Selenium::Client::CommandError, 'mainWindow is nil')
res ? res : fail(Appium::Ios::CommandError, 'mainWindow is nil')
Copy link
Member Author

Choose a reason for hiding this comment

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

Selenium::Client were removed

@KazuCocoa
Copy link
Member Author

@jlipps ^ the above comments is what you wanted ? Could you check again ? 🙇

@KazuCocoa
Copy link
Member Author

Use ::Selenium::WebDriver::Remote::Capabilities.new to define capability to remove patch.

@KazuCocoa KazuCocoa modified the milestone: v9.1 Dec 11, 2016
@KazuCocoa
Copy link
Member Author

Update this description

@KazuCocoa
Copy link
Member Author

according to the "Bump selenium-webdriver dependency #382", someone uses appium_lib with selenium-webdriver. So, they need appium_lib's dependencies over selenium-webdriver3.

So, I'd like to merge this after self-review for them.

@jlipps
Copy link
Member

jlipps commented Dec 12, 2016

👍

@KazuCocoa KazuCocoa mentioned this pull request Dec 12, 2016
@KazuCocoa KazuCocoa merged commit fbcaa62 into appium:master Dec 13, 2016
@KazuCocoa KazuCocoa deleted the update_gems branch December 13, 2016 23:12
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.

4 participants