Skip to content

bump selenium 3.14.1, call RemoteCommand without workaround #259

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 5 commits into from
Oct 15, 2018

Conversation

KazuCocoa
Copy link
Member

closes #258

selenium 3.14.1 has workaround, calling JavaScript, for get_attribute and is_displayed in W3C.
In this PR, I override them in webelement.py to remove the workaround for Appium

I tested on my locale for each command.

Processing selenium-3.14.1-py2.py3-none-any.whl
Installing selenium-3.14.1-py2.py3-none-any.whl to /usr/local/lib/python2.7/site-packages
writing requirements to /usr/local/lib/python2.7/site-packages/selenium-3.14.1-py2.7.egg/EGG-INFO/requires.txt
Adding selenium 3.14.1 to easy-install.pth file

Installed /usr/local/lib/python2.7/site-packages/selenium-3.14.1-py2.7.egg
Searching for urllib3==1.23
Best match: urllib3 1.23
Adding urllib3 1.23 to easy-install.pth file

Using /usr/local/lib/python2.7/site-packages
Finished processing dependencies for Appium-Python-Client==0.28
$ py.test test/functional/ios/appium_tests.py
====================================== test session starts =======================================
platform darwin -- Python 3.7.0, pytest-3.7.1, py-1.5.4, pluggy-0.7.1
rootdir: /Users/kazu/GitHub/python-client, inifile:
collected 11 items

test/functional/ios/appium_tests.py ...........                                            [100%]

================================== 11 passed in 178.38 seconds ===================================

resp = self._execute(RemoteCommand.GET_ELEMENT_ATTRIBUTE, {'name': name})
attributeValue = resp.get('value')
if attributeValue is not None:
if name != 'value' and attributeValue.lower() in ('true', 'false'):
Copy link
Contributor

Choose a reason for hiding this comment

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

this call is not very safe if the returned value is not a string

attributeValue = ''
resp = self._execute(RemoteCommand.GET_ELEMENT_ATTRIBUTE, {'name': name})
attributeValue = resp.get('value')
if attributeValue is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather do if attributeValue is None return None


"""

attributeValue = ''
Copy link
Contributor

@mykola-mokhnach mykola-mokhnach Oct 11, 2018

Choose a reason for hiding this comment

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

    resp = self._execute(RemoteCommand.GET_ELEMENT_ATTRIBUTE, {'name': name})
    attributeValue = resp.get('value')
    if attributeValue is None:
        return None
    # The standard requires attribute values to be of type String or None
    if not isinstance(attributeValue, basestring):
        attributeValue = unicode(attributeValue)
    if name != 'value' and attributeValue.lower() in ('true', 'false'):
        return attributeValue.lower()
    return attributeValue

if attributeValue is None:
return None

if not isinstance(attributeValue, str):
Copy link
Member Author

Choose a reason for hiding this comment

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

basestring and unicode don't work in Python3. str is probably enough in this case. (This work both 2 and 3.)

Copy link
Contributor

Choose a reason for hiding this comment

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

this coercion will throw an error in python2 if the value contains unicode chars

Copy link
Contributor

Choose a reason for hiding this comment

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

http://python-future.org/compatible_idioms.html, check basestring and unicode points

Copy link
Member Author

Choose a reason for hiding this comment

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

ah..
I see.. will fix later

@@ -20,8 +20,56 @@

from .mobilecommand import MobileCommand as Command

# Python 3 imports
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

huh, hacky

if not isinstance(attributeValue, str):
attributeValue = unicode(attributeValue)

if attributeValue.lower() in ('true', 'false'):
Copy link
Contributor

Choose a reason for hiding this comment

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

please add name != 'value' and

@KazuCocoa KazuCocoa merged commit 0add6f6 into appium:master Oct 15, 2018
@KazuCocoa KazuCocoa deleted the bump-minimum-selenium-version branch October 15, 2018 13:46
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.

Make sure isDisplayed and getAttribute
3 participants