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

add appium prefix in create session and fix find_elements for W3C #196

Merged
merged 7 commits into from
Jan 23, 2018

Conversation

KazuCocoa
Copy link
Member

@KazuCocoa KazuCocoa commented Dec 30, 2017

What do I do in this PR

  • Add appium: prefix for capabilities except for items defined by W3C
  • Add forceMjsonwp capability to force request format MJSONWP format.
    • By default, the client send {'firstMatch': [{}], 'alwaysMatch': always_match} style
    • We can remove this process after finishing migrating from MJSONWP to W3C completely.
  • Change some logic from WebDriver's w3c spec
  • Change some test cases to work the latest appium server(1.7.2)

  • Run with W3C
[debug] [MJSONWP] Responding to client with driver.getSize() result: {"width":93,"height":33}
[HTTP] <-- GET /wd/hub/session/a92ddc72-1b53-47b1-a34b-0a55bdca5ca4/element/4680CB2D-24F2-4CC7-844D-917A6D607AE4/size 200 206 ms - 85
[HTTP] --> POST /wd/hub/session {"capabilities":{"firstMatch":[{}],"alwaysMatch":{"appium:deviceName":"iPhone 6s","platformName":"iOS","appium:platformVersion":"10.3","appium:app":"/Users/kazuaki/GitHub/python-client/test/apps/UICatalog.app.zip","appium:automationName":"XCUITest","appium:allowTouchIdEnroll":true}},"desiredCapabilities":{"deviceName":"iPhone 6s","platformName":"iOS","platformVersion":"10.3","app":"/Users/kazuaki/GitHub/python-client/test/apps/UICatalog.app.zip","automationName":"XCUITest","allowTouchIdEnroll":true}}
[debug] [MJSONWP] Calling AppiumDriver.createSession() with args: [{"deviceName":"iPhone 6s","platformName":"iOS","platformVersion":"10.3","app":"/Users/kazuaki/GitHub/python-client/test/apps/UICatalog.app.zip","automationName":"XCUITest","allowTouchIdEnroll":true},null,{"firstMatch":[{}],"alwaysMatch":{"appium:deviceName":"iPhone 6s","platformName":"iOS","appium:platformVersion":"10.3","appium:app":"/Users/kazuaki/GitHub/python-client/test/apps/UICatalog.app.zip","appium:automationName":"XCUITest","appium:allowTouchIdEnroll":true}}]
[debug] [BaseDriver] Event 'newSessionRequested' logged at 1515072504260 (22:28:24 GMT+0900 (JST))
[Appium] Creating new XCUITestDriver (v2.63.0) session
[Appium] Capabilities:
[Appium]   deviceName: iPhone 6s
[Appium]   platformName: iOS
[Appium]   platformVersion: 10.3
[Appium]   app: /Users/kazuaki/GitHub/python-client/test/apps/UICatalog.app.zip
[Appium]   automationName: XCUITest
[Appium]   allowTouchIdEnroll: true
[BaseDriver] The following capabilities were provided, but are not recognized by appium: allowTouchIdEnroll.
[BaseDriver] Session created with session id: 422d1dd0-5ff8-4778-b0db-2d19e4d6a44c
  • set 'forceMjsonwp': True
[HTTP] <-- DELETE /wd/hub/session/7d0e0141-8bff-48ea-99cd-e8bae0f3362d 200 279 ms - 76
[HTTP] --> POST /wd/hub/session {"desiredCapabilities":{"deviceName":"iPhone 6s","platformName":"iOS","platformVersion":"10.3","app":"/Users/kazuaki/GitHub/python-client/test/apps/UICatalog.app.zip","automationName":"XCUITest","allowTouchIdEnroll":true}}
[debug] [MJSONWP] Calling AppiumDriver.createSession() with args: [{"deviceName":"iPhone 6s","platformName":"iOS","platformVersion":"10.3","app":"/Users/kazuaki/GitHub/python-client/test/apps/UICatalog.app.zip","automationName":"XCUITest","allowTouchIdEnroll":true},null,null]
[debug] [BaseDriver] Event 'newSessionRequested' logged at 1515075790469 (23:23:10 GMT+0900 (JST))
[Appium] Creating new XCUITestDriver (v2.63.0) session
[Appium] Capabilities:
[Appium]   deviceName: iPhone 6s
[Appium]   platformName: iOS
[Appium]   platformVersion: 10.3
[Appium]   app: /Users/kazuaki/GitHub/python-client/test/apps/UICatalog.app.zip
[Appium]   automationName: XCUITest
[Appium]   allowTouchIdEnroll: true
[BaseDriver] The following capabilities were provided, but are not recognized by appium: allowTouchIdEnroll.
[BaseDriver] Session created with session id: 3a5d313d-e482-4292-a273-c741d406fbcf

related

always_match = {}
if caps.get('proxy') and caps['proxy'].get('proxyType'):
caps['proxy']['proxyType'] = caps['proxy']['proxyType'].lower()
for k, v in caps.items():
Copy link
Contributor

Choose a reason for hiding this comment

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

-> iteritems

Copy link
Member Author

@KazuCocoa KazuCocoa Jan 4, 2018

Choose a reason for hiding this comment

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

With Python 3.x, items() looks enough as an iterator and iteritems is deprecated.

for k, v in caps.items():
if v and k in _OSS_W3C_CONVERSION:
always_match[_OSS_W3C_CONVERSION[k]] = v.lower() if k == 'platform' else v
if k in _W3C_CAPABILITY_NAMES or ':' in k:
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have a constant for ':' ?

new_opts = copy.deepcopy(moz_opts)
new_opts['profile'] = profile
always_match['moz:firefoxOptions'] = new_opts
return {"firstMatch": [{}], "alwaysMatch": always_match}
Copy link
Contributor

Choose a reason for hiding this comment

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

PEP recommends to use single quotes by default for string constants

@@ -48,6 +98,43 @@ def __init__(self, command_executor='http://127.0.0.1:4444/wd/hub',
By.ANDROID_UIAUTOMATOR = MobileBy.ANDROID_UIAUTOMATOR
By.ACCESSIBILITY_ID = MobileBy.ACCESSIBILITY_ID


Copy link
Contributor

Choose a reason for hiding this comment

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

one line space is enough between class methods

@KazuCocoa KazuCocoa changed the title [WIP]add appium prefix in create session and fix find_elements for W3C add appium prefix in create session and fix find_elements for W3C Jan 4, 2018
@dpgraham
Copy link
Contributor

dpgraham commented Jan 4, 2018

@mykola-mokhnach Doesn't the Java client send the capabilities like this:

{
  "desiredCapabilities": {...},
  "capabilities": {
    "alwaysMatch:" {...},
    "firstMatch": [{...}, ...],
  }
}

I think the safest way to send capabilities is to use both protocols and then parse the response to determine if the session is MJSONWP or W3C. And then eventually MJSONWP will be deprecated.

@SrinivasanTarget
Copy link
Member

@dpgraham

{
  "desiredCapabilities": {...},
  "capabilities": {
    "desiredCapabilities": {...},
    "firstMatch": [{...}]
  }
}

This is how we send currently in java-client.

@jlipps
Copy link
Member

jlipps commented Jan 4, 2018

Yes, the client should send both types of capabilities as a compatibility measure. @SrinivasanTarget that payload doesn't look correct, @dpgraham's does though.

@KazuCocoa
Copy link
Member Author

KazuCocoa commented Jan 5, 2018

📝
In Python and Ruby client, they send the following format by default and they change their behaviour whether W3C or OSS depending on the response.

{
  "desiredCapabilities": {...},
  "capabilities": {
    "alwaysMatch:" {...},
    "firstMatch": [{...}, ...],
  }
}

So, if the response is like the below, it comes from Ruby client's test code, the clients work as the OSS(MJSONWP) driver.

      response = {
        status: 0, # To make bridge.dialect == :oss
        sessionId: '1234567890',
        value: {
          capabilities: {
            device: 'iphone',
            browserName: 'UICatalog',
            sdkVersion: '10.3.1',
            CFBundleIdentifier: 'com.example.apple-samplecode.UICatalog'
          }
        }
      }.to_json

And if the response is like the below, it comes from Ruby client's test code, the drivers work as the W3C driver.

      response = {
        value: {
          sessionId: '1234567890',
          capabilities: {
            device: 'iphone',
            browserName: 'UICatalog',
            sdkVersion: '10.3.1',
            CFBundleIdentifier: 'com.example.apple-samplecode.UICatalog'
          }
        }
      }.to_json

The above's response depends on the response. Meanwhile, Appium 1.7.2-beta3~5 support both formats. In the environment, clients should send only { "desiredCapabilities": {...} } to get MJSONWP style's response. (Finally, W3C create session was disabled for Appium 1.7.2 though.)

Thus, I define forceMjsonwp to manage the request format to handle the response. We can remove the flag safely in the future because each client sends { "desiredCapabilities": {...}, "capabilities": { ... } } format by default.


BTW, I found some missing commands and locators in W3C module. I'm not sure Java Client, but I think we also cover them for W3C.

@SrinivasanTarget
Copy link
Member

SrinivasanTarget commented Jan 5, 2018

Yes, the client should send both types of capabilities as a compatibility measure

👍

that payload doesn't look correct

Yes @jlipps Looks like a bug. Will fix in upcoming release. @KazuCocoa looking at your links java-client also misses few of them. Will figure out those first. Thanks for letting us know.

@TikhomirovSergey FYI

@KazuCocoa
Copy link
Member Author

📝
I'll merge this after self-review again, later.

@KazuCocoa KazuCocoa merged commit f37733d into appium:master Jan 23, 2018
@KazuCocoa KazuCocoa deleted the add_appium_prefix branch January 24, 2018 14:13
@NozomiIto
Copy link

I'm waiting for this fix to be released! (since now python client does not work with Appium1.8.0-beta maybe due to this problem)

@dpgraham
Copy link
Contributor

@NozomiIto This probably won't fix the problem. Do you have error logs?

@NozomiIto
Copy link

@dpgraham Thanks.

Appium server 1.8.0-beta log:

https://gist.github.com/NozomiIto/3c12eefec788a9d6a87bfdf5d3d82f8d

Python client0.26 log:
https://gist.github.com/NozomiIto/81b2795e6772e1f5b2a63a5246a650f8

My Python code:

from appium import webdriver
import time

desired_caps = {}
desired_caps['platformName'] = 'iOS'
desired_caps['platformVersion'] = '11.2'
desired_caps['deviceName'] = 'iPhone 8'
desired_caps['automationName'] = 'XCUITest'
desired_caps['bundleId'] = 'com.apple.camera'
desired_caps['showXcodeLog'] = 'true'
driver = webdriver.Remote('http://localhost:4723/wd/hub', desired_caps)

But when I install Python client from the master HEAD, this error is resolved.
I thought this problem is similar to appium/appium#10055 and this fix would resolve the problem.

@dpgraham
Copy link
Contributor

@NozomiIto Looks like you were right, and it is a client issue. It's sending incomplete capabilities. However, the next upcoming beta will have a workaround for that.

Appium on the master branch currently has that fix and we'll probably publish a new beta later today or tomorrow.

@NozomiIto
Copy link

@dpgraham Thank you so much!

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.

6 participants