Add unit tests for keyboard API #452

manoj9788 · 2019-10-17T08:16:30Z

No description provided.

test: add unit test unlock (appium#450)

manoj9788 · 2019-10-17T08:58:55Z

KazuCocoa · 2019-10-17T09:35:31Z

test/unit/webdriver/device/keyboard_test.py

+        httpretty.register_uri(
+            httpretty.POST,
+            appium_command('/session/1234567890/appium/device/press_keycode'),
+            body='{keycode: 86}'


Here can be json.dumps(dict(keycode=86)) to buld JSON or write JSON like '{"keycode": "86"}'

do you think the below is good?
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"}' ) driver.press_keycode('86') d = get_httpretty_request_body((httpretty.last_request())) assert d['keycode'] == 86

The above test passes

@KazuCocoa Done.

test/unit/webdriver/device/keyboard_test.py

ki4070ma · 2019-10-17T16:32:59Z

test/unit/webdriver/device/keyboard_test.py

+        httpretty.register_uri(
+            httpretty.POST,
+            appium_command('/session/1234567890/appium/device/press_keycode'),
+            body='{"value": "86"}'


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

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

@KazuCocoa please have a check again.

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

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

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

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

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.

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.

I'm merging this @KazuCocoa.

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

ki4070ma · 2019-10-17T16:33:18Z

test/unit/webdriver/device/keyboard_test.py

+        httpretty.register_uri(
+            httpretty.POST,
+            appium_command('/session/1234567890/appium/device/long_press_keycode'),
+            body='{"value": "86"}'


Same as above.

ki4070ma · 2019-10-17T16:33:49Z

test/unit/webdriver/device/keyboard_test.py

+        httpretty.register_uri(
+            httpretty.POST,
+            appium_command('/session/1234567890/appium/device/keyevent'),
+            body='{keycode: 86}'


Same as above.

ki4070ma · 2019-10-17T16:38:55Z

test/unit/webdriver/device/keyboard_test.py

+            appium_command('/session/1234567890/appium/device/keyevent'),
+            body='{keycode: 86}'
+        )
+        assert isinstance(driver.keyevent(86), WebDriver)


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

ki4070ma · 2019-10-17T16:43:15Z

test/unit/webdriver/device/keyboard_test.py

+            appium_command('/session/1234567890/appium/device/long_press_keycode'),
+            body='{"value": "86"}'
+        )
+        driver.long_press_keycode(86)


Check returned value type

ki4070ma · 2019-10-17T16:46:17Z

test/unit/webdriver/device/keyboard_test.py

+                86, metastate=[
+                    0x00000001, 0x00200000], flags=[
+                    0x20, 0x00000004, 0x00000008]), WebDriver)
+


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]

ki4070ma · 2019-10-17T16:46:39Z

test/unit/webdriver/device/keyboard_test.py

+                    0x20, 0x00000004, 0x00000008]), WebDriver)
+
+    @httpretty.activate
+    def test_long_press_keycode_with_flags(self):


Same as above.

SrinivasanTarget · 2019-10-17T17:05:27Z

test/unit/webdriver/device/keyboard_test.py

+        httpretty.register_uri(
+            httpretty.POST,
+            appium_command('/session/1234567890/appium/device/press_keycode'),
+            body='{"value": "86"}'


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

manoj9788 · 2019-10-17T17:34:30Z

Thanks @SrinivasanTarget

KazuCocoa

just left JSON format comments

KazuCocoa · 2019-10-18T04:05:54Z

test/unit/webdriver/device/keyboard_test.py

+        httpretty.register_uri(
+            httpretty.POST,
+            appium_command('/session/1234567890/appium/device/keyevent'),
+            body='{keycode: 86}'


The body should be JSON format

KazuCocoa · 2019-10-18T04:06:07Z

test/unit/webdriver/device/keyboard_test.py

+        httpretty.register_uri(
+            httpretty.POST,
+            appium_command('/session/1234567890/appium/device/press_keycode'),
+            body='{keycode: 86, metastate: 2097153, flags: 44}'


The body should be JSON format

KazuCocoa · 2019-10-18T04:06:16Z

test/unit/webdriver/device/keyboard_test.py

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


The body should be JSON format

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?

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.

manoj9788 added 2 commits October 16, 2019 21:14

Merge pull request #1 from appium/master

052cccd

test: add unit test unlock (appium#450)

Add Unit tests for Keyboard API

75afed6

manoj9788 mentioned this pull request Oct 17, 2019

Add unit tests #283

Closed

KazuCocoa reviewed Oct 17, 2019

View reviewed changes

manoj9788 added 2 commits October 17, 2019 17:54

incorporating review comments

b7099df

change per review comment

4e827bd

ki4070ma reviewed Oct 17, 2019

View reviewed changes

SrinivasanTarget approved these changes Oct 17, 2019

View reviewed changes

SrinivasanTarget merged commit 1993d98 into appium:master Oct 17, 2019

KazuCocoa reviewed Oct 18, 2019

View reviewed changes

-        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

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

Add unit tests for keyboard API #452

Add unit tests for keyboard API #452

Uh oh!

Conversation

manoj9788 commented Oct 17, 2019

Uh oh!

manoj9788 commented Oct 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manoj9788 Oct 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manoj9788 Oct 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ki4070ma Oct 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ki4070ma Oct 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manoj9788 commented Oct 17, 2019

Uh oh!

KazuCocoa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

manoj9788 Oct 17, 2019 •

edited

Loading

manoj9788 Oct 17, 2019 •

edited

Loading

ki4070ma Oct 17, 2019 •

edited

Loading

ki4070ma Oct 17, 2019 •

edited

Loading