8000 fix: Handling of dictionary-values in WebElement.get_attribute() by hanneshauer · Pull Request #521 · appium/python-client · GitHub
[go: up one dir, main page]
Skip to content

fix: Handling of dictionary-values in WebElement.get_attribute()#521

Merged
mykola-mokhnach merged 1 commit intoappium:masterfrom
hanneshauer:master
Apr 23, 2020
Merged

fix: Handling of dictionary-values in WebElement.get_attribute()#521
mykola-mokhnach merged 1 commit intoappium:masterfrom
hanneshauer:master

Conversation

@hanneshauer
Copy link
Contributor
@hanneshauer hanneshauer commented Apr 21, 2020 •
edited
Loading

Changes

  • Add additional type check to get_attribute() to avoid mishandling of dictionaries received from an Appium server.

Problem

get_attribute() in WebElement currently cannot receive dictionary values.

E.g. when calling get_attribute('rect') on an element the Appium iOS Server returns a dictionary of coordinates and size which the Python client cannot handle:

>>> buttons[2].get_attribute('rect')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.7/site-packages/appium/webdriver/webelement.py", line 66, in get_attribute
    attributeValue = unicode(attributeValue)
NameError: name 'unicode' is not defined

(Note that the NameError results from me using Python 3 in which unicode() is not available. I am not sure whether this should be considered an issue as well or not. The core issue is that this code path shouldn't be reached in this case anyways.)

When the client receives a return value from the Appium server but it isn't of type str it tries to convert it to one using unicode(); as far as I can tell this is because of the different string types available in Python 2. However this leads to an issues if the value received isn't a str but a dict in which case the client still tries to convert it even though it isn't supposed to be handled as a string at all (calling unicode() on a dict in Python 2 would also lead to undesired behaviour because the value would effectively be converted to a string as well).

Proposed Fix

The proposed fix adds a check and immediately returns the value if it is a dictionary. This avoids undesired behaviour (the NameError seen above when using Python 3, or the value being converted to a string when using Python 2) but leaves the existing conversion in place in case the value is neither a dictionary nor a str.

I have further added a test case to the WebElement unit test that checks whether get_attribute() correctly receives and passes on received dictionaries.

I have run both $ tox and $ pytest test/unit and did not see any testing regressions due to the proposed change.

@mykola-mokhnach
Copy link
Contributor
mykola-mokhnach commented Apr 21, 2020

Could you also address the unicode issue?
Something like

try:
    return unicode(x)
except NameError:
    return str(x)

@hanneshauer
Copy link
Contributor Author
hanneshauer commented Apr 21, 2020

I wasn't sure how to best address the Python2/3 discrepancies, thanks for your pointer; I've added a try-catch like you suggested.

@hanneshauer
Copy link
Contributor Author
hanneshauer commented Apr 22, 2020

I've updated my PR branch with the additional changes yesterday while GitHub was experiencing some technical issues and while the commit shows up in my branch it is not being reflected here even though the page says pushing should suffice to add them to the PR - not sure if this is due to the timing of my push or something else.

@mykola-mokhnach
Copy link
Contributor
mykola-mokhnach commented Apr 22, 2020

Try to squash your commit

@hanneshauer
fix: Handling of dictionary-values in WebElement.get_attribute()
e5909ed 
fix: Catch NameError caused by missing unicode()-function and instead use str() when using Python 3
@mykola-mokhnach mykola-mokhnach merged commit a2e21d1 into appium:master Apr 23, 2020
ki4070ma added a commit that referenced this pull request Apr 26, 2020
@ki4070ma @dependabot-preview
@KazuCocoa @mykola-mokhnach @iamnrupesh @Akulavenkatesh @hanneshauer
Rebase python3 branch with master (#522)
78926d2 
* Update pytest-cov requirement from ~=2.6 to ~=2.8 (#489)

Updates the requirements on [pytest-cov](https://github.com/pytest-dev/pytest-cov) to permit the latest version.
- [Release notes](https://github.com/pytest-dev/pytest-cov/releases)
- [Changelog](https://github.com/pytest-dev/pytest-cov/blob/master/CHANGELOG.rst)
- [Commits](pytest-dev/pytest-cov@v2.6.0...v2.8.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* Update autopep8 requirement from ~=1.4 to ~=1.5 (#490)

Updates the requirements on [autopep8](https://github.com/hhatto/autopep8) to permit the latest version.
- [Release notes](https://github.com/hhatto/autopep8/releases)
- [Commits](hhatto/autopep8@v1.4...v1.5)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* Update tox-travis requirement from ~=0.11 to ~=0.12 (#491)

Updates the requirements on [tox-travis](https://github.com/tox-dev/tox-travis) to permit the latest version.
- [Release notes](https://github.com/tox-dev/tox-travis/releases)
- [Changelog](https://github.com/tox-dev/tox-travis/blob/master/HISTORY.rst)
- [Commits](tox-dev/tox-travis@0.11...0.12)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* Update tox requirement from ~=3.6 to ~=3.14 (#494)

Updates the requirements on [tox](https://github.com/tox-dev/tox) to permit the latest version.
- [Release notes](https://github.com/tox-dev/tox/releases)
- [Changelog](https://github.com/tox-dev/tox/blob/master/docs/changelog.rst)
- [Commits](tox-dev/tox@3.6.0...3.14.3)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* chore: Fix find_by_images_tests.py (#495)

* chore: Fix find_by_images_tests.py

* Add installation opencv4nodejs

* Fix typo

* Add taking screen record to find_by_image_test

* Fix errors on the emulator

* Remove unused imports

* feat: Add viewmatcher (#480)

* Add android view matcher as strategy locator

* Add docstring

* Add functional test

* Remove find_elements_by_android_data_matcher

* Fix docstring

* tweak docstring

* Bump 0.50

* Update changelog for 0.50

* Fix flaky functional tests (#473)

* Run all tests

* Fix apk file path

* Skip find_element_by_image test cases

* Skip context switching test

* Skip multi tap test on CI

* Change strategy for waiting element

* Add functions for same steps

* Restore unexpected changes

* Fix touch_action_tests

* Fix

* Fix
Fix test_driver_swipe

* fix

* Create _move_to_[target_view]

* [test_driver_swipe] Add wait

* feat: Add idempotency key header to create session requests (#514)

* feat: Override send_keys without file upload function (#515)

* add send_keys_direct

* override send_keys

* tune

* add unittest instead of functional test

* tweak syntax

* Bump 0.51

* Update changelog for 0.51

* test: Fix test_clear flaky functional test (#519)

* test: Add unit test for set_value (setImmediateValue) (#518)

* chore: Fix int - str comparison error in ios desired capabilities (#517)

if number >= PytestXdistWorker.COUNT:

TypeError: '>=' not supported between instances of 'int' and 'str'

2. Updated test case path and iPhone model in Readme file

* fix: Handling of dictionary-values in WebElement.get_attribute() (#521)

* Bump 0.52

* Update changelog for 0.52

* Fix mypy error

* tweak

* Add wait to test

* Skip tap_twice test

* review comments

* Remove unnecessary import

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Kazuaki Matsuo <fly.49.89.over@gmail.com>
Co-authored-by: Mykola Mokhnach <mokhnach@gmail.com>
Co-authored-by: Nrupesh Patel <nrupesh.patel2912@gmail.com>
Co-authored-by: Venkatesh <venkatesh@poshmark.com>
Co-authored-by: Hannes Hauer <hanneshauer@beeware.at>
ki4070ma added a commit that referenced this pull request Apr 30, 2020
@ki4070ma @dependabot-preview
@KazuCocoa @mykola-mokhnach @iamnrupesh @Akulavenkatesh @hanneshauer
feat: Merge python3 branch to master (#526)
9c597a3 
* Drop py2 support (#478)

* Drop py2 support

* Support 3.7+

* Add explicit type declarations (#482)

* Fixed mypy warning: touch_action.py

* Fixed mypy warning: multi_action.py

* Fixed mypy warning: extensions/android

* Fixed mypy warning: extensions/search_context

* Updated

* Revert some changes to run unit test

* Review comments

* Updates

* Updates

* Add mypy check to ci.sh

* Add mypy to Pipfile

* Updates

* Update README

* Revert unexpected changes

* Updates Dict

* Revert unexpected changes

* Updates

* Review comments

* Review comments

* tweak

* Restore and modify changes

* Fix wrong return type

* Add comments

* Revert unexpected changes

* Fix mypy error

* updates

* Add mypy to pre-commit (#485)

* chore: Applied some py3 formats (#486)

* Removed unused import

* Removed unnecessary codes

* Applied f'' format instead ''.format()

* Fixes

* tweak

* chore: Fix mypy errors under test folder (#487)

* Fix mypy errors under test folder

* Add mypy check for test folder to pre-commit

* Add mypy check to ci

* chore: Remove unittest dependency (#488)

* Removed unnecessary codes from calling super

* Removed unittest dependency

* Upgrade the dependencies to the latest

* Removed unused args

* Review comments

* Update mock requirement from ~=3.0 to ~=4.0 (#502)

Updates the requirements on [mock](https://github.com/testing-cabal/mock) to permit the latest version.
- [Release notes](https://github.com/testing-cabal/mock/releases)
- [Changelog](https://github.com/testing-cabal/mock/blob/master/CHANGELOG.rst)
- [Commits](testing-cabal/mock@3.0.0...4.0.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* Add 'from' to except (#503)

* Update pre-commit requirement from ~=1.21 to ~=2.1 (#506)

Updates the requirements on [pre-commit](https://github.com/pre-commit/pre-commit) to permit the latest version.
- [Release notes](https://github.com/pre-commit/pre-commit/releases)
- [Changelog](https://github.com/pre-commit/pre-commit/blob/master/CHANGELOG.md)
- [Commits](pre-commit/pre-commit@v1.21.0...v2.1.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* doc: Add script to generate sphinx doc  (#508)

* Add quickstart template files

* Update conf file

* Update

* Update settings

* Change project name

* Add script to generate docs

* Changed header title

* Add new line to usage section

* Add py.typed file(PEP561)

* Replace \n with new line

* tweak

* Use sphinx format for tables

* Rebase python3 branch with master (#522)

* Update pytest-cov requirement from ~=2.6 to ~=2.8 (#489)

Updates the requirements on [pytest-cov](https://github.com/pytest-dev/pytest-cov) to permit the latest version.
- [Release notes](https://github.com/pytest-dev/pytest-cov/releases)
- [Changelog](https://github.com/pytest-dev/pytest-cov/blob/master/CHANGELOG.rst)
- [Commits](pytest-dev/pytest-cov@v2.6.0...v2.8.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* Update autopep8 requirement from ~=1.4 to ~=1.5 (#490)

Updates the requirements on [autopep8](https://github.com/hhatto/autopep8) to permit the latest version.
- [Release notes](https://github.com/hhatto/autopep8/releases)
- [Commits](hhatto/autopep8@v1.4...v1.5)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* Update tox-travis requirement from ~=0.11 to ~=0.12 (#491)

Updates the requirements on [tox-travis](https://github.com/tox-dev/tox-travis) to permit the latest version.
- [Release notes](https://github.com/tox-dev/tox-travis/releases)
- [Changelog](https://github.com/tox-dev/tox-travis/blob/master/HISTORY.rst)
- [Commits](tox-dev/tox-travis@0.11...0.12)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* Update tox requirement from ~=3.6 to ~=3.14 (#494)

Updates the requirements on [tox](https://github.com/tox-dev/tox) to permit the latest version.
- [Release notes](https://github.com/tox-dev/tox/releases)
- [Changelog](https://github.com/tox-dev/tox/blob/master/docs/changelog.rst)
- [Commits](tox-dev/tox@3.6.0...3.14.3)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

* chore: Fix find_by_images_tests.py (#495)

* chore: Fix find_by_images_tests.py

* Add installation opencv4nodejs

* Fix typo

* Add taking screen record to find_by_image_test

* Fix errors on the emulator

* Remove unused imports

* feat: Add viewmatcher (#480)

* Add android view matcher as strategy locator

* Add docstring

* Add functional test

* Remove find_elements_by_android_data_matcher

* Fix docstring

* tweak docstring

* Bump 0.50

* Update changelog for 0.50

* Fix flaky functional tests (#473)

* Run all tests

* Fix apk file path

* Skip find_element_by_image test cases

* Skip context switching test

* Skip multi tap test on CI

* Change strategy for waiting element

* Add functions for same steps

* Restore unexpected changes

* Fix touch_action_tests

* Fix

* Fix
Fix test_driver_swipe

* fix

* Create _move_to_[target_view]

* [test_driver_swipe] Add wait

* feat: Add idempotency key header to create session requests (#514)

* feat: Override send_keys without file upload function (#515)

* add send_keys_direct

* override send_keys

* tune

* add unittest instead of fu
8330
nctional test

* tweak syntax

* Bump 0.51

* Update changelog for 0.51

* test: Fix test_clear flaky functional test (#519)

* test: Add unit test for set_value (setImmediateValue) (#518)

* chore: Fix int - str comparison error in ios desired capabilities (#517)

if number >= PytestXdistWorker.COUNT:

TypeError: '>=' not supported between instances of 'int' and 'str'

2. Updated test case path and iPhone model in Readme file

* fix: Handling of dictionary-values in WebElement.get_attribute() (#521)

* Bump 0.52

* Update changelog for 0.52

* Fix mypy error

* tweak

* Add wait to test

* Skip tap_twice test

* review comments

* Remove unnecessary import

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Kazuaki Matsuo <fly.49.89.over@gmail.com>
Co-authored-by: Mykola Mokhnach <mokhnach@gmail.com>
Co-authored-by: Nrupesh Patel <nrupesh.patel2912@gmail.com>
Co-authored-by: Venkatesh <venkatesh@poshmark.com>
Co-authored-by: Hannes Hauer <hanneshauer@beeware.at>

* chore: Update readme and gitchangelog section role (#524) (#525)

* chore: tweak changelog filter

* address stoping Python 2 support

* 2 instead of 2.0...

* tweak readme

* Revert some unexpected changes

* review comments

* Changed bound for TypeVar

* Fix crashing ci

* Remove beta

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Kazuaki Matsuo <fly.49.89.over@gmail.com>
Co-authored-by: Mykola Mokhnach <mokhnach@gmail.com>
Co-authored-by: Nrupesh Patel <nrupesh.patel2912@gmail.com>
Co-authored-by: Venkatesh <venkatesh@poshmark.com>
Co-authored-by: Hannes Hauer <hanneshauer@beeware.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mykola-mokhnach mykola-mokhnach mykola-mokhnach approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hanneshauer @mykola-mokhnach
0