Skip to content

Add unit tests for keyboard API #452

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.

Sign up for GitHub

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 4 commits into from
Oct 17, 2019
Merged

Add unit tests for keyboard API #452

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
052cccd
Merge pull request #1 from appium/master
manoj9788 Oct 16, 2019
75afed6
Add Unit tests for Keyboard API
manoj9788 Oct 17, 2019
b7099df
incorporating review comments
manoj9788 Oct 17, 2019
4e827bd
change per review comment
manoj9788 Oct 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions test/unit/webdriver/device/keyboard_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,69 @@ def test_hide_keyboard(self):
appium_command('/session/1234567890/appium/device/hide_keyboard')
)
assert isinstance(driver.hide_keyboard(), WebDriver)

@httpretty.activate
def test_press_keycode(self):
driver = android_w3c_driver()
httpretty.register_uri(
httpretty.POST,
appium_command('/session/1234567890/appium/device/press_keycode'),
body='{"value": "86"}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line is unnecessary, so delete this line.
This body simulates the value which appium server returns.

Copy link
Member Author

@manoj9788 manoj9788 Oct 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I'm confused.
"This body simulates the value which appium server returns."
my previous commit had exactly that...
@KazuCocoa ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@KazuCocoa please have a check again.

As tips, you will pass tests when both L42 is deleted and L42 is body={}.

Copy link
Member

Choose a reason for hiding this comment

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

@ki4070ma I don't understand why don't we need to mock the response? I would agree on @KazuCocoa @manoj9788 way of writing it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hum, in my opinion, its value (body={blabla}) doesn't impact to any other codes and result.

Copy link
Member

Choose a reason for hiding this comment

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

@ki4070ma Feel free to approve it. I think this is the right way to mock it.

Copy link
Member

Choose a reason for hiding this comment

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

Either is fine to add a response or not as a client-side check. (It depends on the case if the client would like to check internal parsing against the response.)
btw, the return value by the server is like { value: '' }, i remember.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, guys! so many confusions on this.
I think mocking and validating the request is always the way to go and doesn't make sense to rely on the server having the values and it wouldn't impact if there are any bug from the server-side since this is a completely client-side unit tests.

Copy link
Member

Choose a reason for hiding this comment

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

I'm merging this @KazuCocoa.

Copy link
Collaborator

@ki4070ma ki4070ma Oct 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

ok, thanks for many your comments.

[JFYI]

btw, the return value by the server is like { value: '' }, i remember.

Seems right.

[debug] [WD Proxy] Got response with status 200: {"sessionId":"80c32258-b7fa-4f1e-bc5b-b76b11d9a082","value":null}

[HTTP] --> POST /wd/hub/session/b7a19413-0f4f-4a52-ab52-b9aa630cffb8/appium/device/press_keycode
[HTTP] {"keycode":86}
[debug] [W3C (b7a19413)] Calling AppiumDriver.pressKeyCode() with args: [86,null,null,null,null,"b7a19413-0f4f-4a52-ab52-b9aa630cffb8"]
[debug] [WD Proxy] Matched '/appium/device/press_keycode' to command name 'pressKeyCode'
[debug] [WD Proxy] Proxying [POST /appium/device/press_keycode] to [POST http://localhost:8200/wd/hub/session/80c32258-b7fa-4f1e-bc5b-b76b11d9a082/appium/device/press_keycode] with body: {"keycode":86,"metastate":null,"flags":null}
[debug] [WD Proxy] Got response with status 200: {"sessionId":"80c32258-b7fa-4f1e-bc5b-b76b11d9a082","value":null}
[debug] [W3C (b7a19413)] Responding to client with driver.pressKeyCode() result: null
[HTTP] <-- POST /wd/hub/session/b7a19413-0f4f-4a52-ab52-b9aa630cffb8/appium/device/press_keycode 200 28 ms - 14

)
driver.press_keycode(86)
d = get_httpretty_request_body((httpretty.last_request()))
assert d['keycode'] == 86

@httpretty.activate
def test_long_press_keycode(self):
driver = android_w3c_driver()
httpretty.register_uri(
httpretty.POST,
appium_command('/session/1234567890/appium/device/long_press_keycode'),
body='{"value": "86"}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

)
driver.long_press_keycode(86)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check returned value type

d = get_httpretty_request_body((httpretty.last_request()))
assert d['keycode'] == 86

@httpretty.activate
def test_keyevent(self):
driver = android_w3c_driver()
httpretty.register_uri(
httpretty.POST,
appium_command('/session/1234567890/appium/device/keyevent'),
body='{keycode: 86}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

Copy link
Member

Choose a reason for hiding this comment

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

The body should be JSON format

)
assert isinstance(driver.keyevent(86), WebDriver)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert isinstance(driver.keyevent(86), WebDriver)
assert isinstance(driver.keyevent(86), WebDriver)
d = get_httpretty_request_body((httpretty.last_request()))
assert d['keyevent'] == 86
assert d.get('metastate') is None


@httpretty.activate
def test_press_keycode_with_flags(self):
driver = android_w3c_driver()
httpretty.register_uri(
httpretty.POST,
appium_command('/session/1234567890/appium/device/press_keycode'),
body='{keycode: 86, metastate: 2097153, flags: 44}'
Copy link
Member

Choose a reason for hiding this comment

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

The body should be JSON format

)
# metastate is META_SHIFT_ON and META_NUM_LOCK_ON
# flags is CANCELFLAG_CANCELEDED, FLAG_KEEP_TOUCH_MODE, FLAG_FROM_SYSTEM
assert isinstance(
driver.press_keycode(
86, metastate=[
0x00000001, 0x00200000], flags=[
0x20, 0x00000004, 0x00000008]), WebDriver)

Copy link
Collaborator

@ki4070ma ki4070ma Oct 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Add check arguments which are passed to appium server.

Suggested change
d = get_httpretty_request_body((httpretty.last_request()))
assert d['keycode'] == 86
assert d['metastate'] == [0x00000001, 0x00200000]
assert d['flags'] == [0x20, 0x00000004, 0x00000008]

@httpretty.activate
def test_long_press_keycode_with_flags(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

driver = android_w3c_driver()
httpretty.register_uri(
httpretty.POST,
appium_command('/session/1234567890/appium/device/long_press_keycode'),
body='{keycode: 86, metastate: 2097153, flags: 44}'
Copy link
Member

Choose a reason for hiding this comment

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

The body should be JSON format

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks Kazu. I understand. Pretty much all the unit tests out there don't have this JSON format do you think all those needs change as well?
All the other tests just use body and send key,val params directly. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

It depends on what the test check.
I mean:

res = driver.do_some_command
assert res == 'some result' # Check the response value. In this case, we must set the return value.

Maybe, almost cases are not necessary since the client simply returns the result without modification inside the client. As a client, what request body has is important. So, the request check is necessary though.

Btw, the above body should be {"value: ''"} as the response.

)
# metastate is META_SHIFT_ON and META_NUM_LOCK_ON
# flags is CANCELFLAG_CANCELEDED, FLAG_KEEP_TOUCH_MODE, FLAG_FROM_SYSTEM
assert isinstance(
driver.long_press_keycode(
86, metastate=[
0x00000001, 0x00200000], flags=[
0x20, 0x00000004, 0x00000008]), WebDriver)