wix / Detox

Gray box end-to-end testing and automation framework for mobile apps
https://wix.github.io/Detox/
MIT License
11.23k stars 1.92k forks source link

Add Support for Device Shake Action #551

Closed grabbou closed 6 years ago

grabbou commented 6 years ago

Hey,

I am working on https://github.com/facebook/react-native/pull/17806 where I am attempting to remove Appium and start using Detox for internal tests of React Native as well as the release process.

The problem I am facing right now is lack of the ability to open up Dev Settings (in order to test whether Chrome Debugger or Live Reload is working).

I've done some research and found that one of the ways to do it would be to expose an action on device, called toggleDevSettings that could execute show(https://github.com/facebook/react-native/blob/e8eec24706e792314ee574bbf7f7c0066c4f3a7a/React/DevSupport/RCTDevMenu.m#L292) method which is exposed on RCTDevMenu native module.

Alternatively, we could go towards React Native agnostic solution and implement the ability to trigger shake gesture.

I am open to collaborating on this feature as I need it to move my PR forward.

grabbou commented 6 years ago

cc: @rotemmiz as we discussed this on Slack

LeoNatan commented 6 years ago

I think a device.shake() API is the correct way to go. iOS simulator has this option in the menu, so it could be possible to use that somehow. If not, it should be possible to implement using some Earl Grey trick or private API directly.

LeoNatan commented 6 years ago

Added support for shake in iOS native. Need JS support from mr. RMM @rotemmiz

grabbou commented 6 years ago

Thanks @LeoNatan, any ideas when Android support might land?

grabbou commented 6 years ago

How would you attempt using that feature w/o a JS interface? Any workaround I can do to use it right now?

LeoNatan commented 6 years ago

I cannot say when Android support might be added.

Regarding a workaround, I am not sure. The shake event that triggers the menu is generated from native. You could drill down in the JS API and somehow call that action directly, but that like implementing the actual JS API (if you do, please submit PR 😄).

grabbou commented 6 years ago

Haha, okay, I'll try to play around with it and I'll get you some details back.

Is there anyone I can ping re: Android?

On Mon, 5 Feb 2018 at 15:36 Leo Natan notifications@github.com wrote:

I cannot say when Android support might be added.

Regarding a workaround, I am not sure. The shake event that triggers the menu is generated from native. You could drill down in the JS API and somehow call that action directly, but that like implementing the actual JS API (if you do, please submit PR 😄).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/wix/detox/issues/551#issuecomment-363102287, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWcxuVEYl6J9xEpMWabsj6FtSUULKmpks5tRxHhgaJpZM4R1eUH .

rotemmiz commented 6 years ago

Android, me.

LeoNatan commented 6 years ago

@rotemmiz According to this, you can fake it with adb. That's the hardware menu key.

rotemmiz commented 6 years ago

adb shell input keyevent 82 is not exactly shake, it's keycode for menu button. It works, but this is not "shake"

LeoNatan commented 6 years ago

True, it would be added as a separate action.

LeoNatan commented 6 years ago

For shake, I see some need to telnet into the simulator and change the accelerator. 😓

Why it's always harder on Android? 😂

grabbou commented 6 years ago

If the name of method was open dev tools, then you could use the key press as Android implementation j guess 👍

On Mon, Feb 5, 2018, 4:20 PM Leo Natan notifications@github.com wrote:

For shake, I see some need to telnet into the simulator and change the accelerator. 😓😂

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/wix/detox/issues/551#issuecomment-363115938, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWcxvdAlq8cnImXOlJjUd9WW-8sxe92ks5tRxw0gaJpZM4R1eUH .

rotemmiz commented 6 years ago

We already have a pretty good telnet client, not gonna be very hard I hope.

Android has no shake property, it's not a thing in the official API... There is one good library that does that. They do though support mocking every hardware/software key available (KEYCODE_MENU, etc). Not everything is harder, but if we impose iOS APIs on Android that's what we get.

LeoNatan commented 6 years ago

So why not keep shake to iOS and menu for Android?

rotemmiz commented 6 years ago

Not the same thing, it will not work the same outside of react native

LeoNatan commented 6 years ago

But seems to me like those are two separate functionalities unrelated to each other. We could add shake now for iOS and menu only for Android (no menu on iOS), and then add shake later for Android (when necessary).

rotemmiz commented 6 years ago

I'm starting to think we need another abstraction layer, for react native related operations, that will use different driver APIs under the hood 😕

LeoNatan commented 6 years ago

I disagree. Detox is React Native neutral, and should remain that way.

LeoNatan commented 6 years ago

(reloadReactNative is also problematic)

LeoNatan commented 6 years ago

Further, dev menu is a debug feature, so it makes even less sense in adding it as official API for device. Normally, users run in release.

rotemmiz commented 6 years ago

That's exactly my point, we don't want to add per platform react native specific behaviors, so we create a dedicated API for react native apps (reloadReactNaive will be added there as well), and native apps will not have these options as part of the API. reloadReactNative or openDevMenu are only RN options.

Native APIs will include the available native commands for each platform.

I will try to draft something

LeoNatan commented 6 years ago

Other than this narrow use-case of testing the tools themselves, why should the dev menu be exposed? I dislike adding support for the kitchen sink. On iOS, support for shake is something that is useful beyond dev menu. It's a used gesture for undo, and is often used. So adding support for that has merit. So is the menu button for Android. I just don't see it for RN.

LeoNatan commented 6 years ago

With these two APIs, for this narrow use-case, a workaround can be added to ask if(iOS) shake() else menu() inside @grabbou 's test..

Salakar commented 6 years ago

Not sure if this is the best place to chime in but I'd also like to have the ability to be able to control the Dev Settings in RN through detox - at least the ability to toggle remote debugging and to reload react native.


@LeoNatan I have a bit of a weirder use case here though than just tooling tests: whilst detox is great for controlling all things device/UI (which i'm internally still using) I needed a way of testing native modules e2e without extensive tests in native obj-c/java code or without having the tests in-app.

image

As you can see in the above screenshot, this is just standard Node.js + mocha tests but I can now require native modules and use them fully - no mocking, bundle runs in-process automatically (as long as remote debugging is enabled and I have the ability to reload react native - this is where detox comes in).

I plan to bundle this at some point into an NPM package to ease native module testing as there's only a handful of native modules doing testing well/extensively that I could see - I see this as a first step towards simplifying the testing process for native module developers.


@rotemmiz: That's exactly my point, we don't want to add per platform react native specific behaviors, so we create a dedicated API for react native apps (reloadReactNaive will be added there as well), and native apps will not have these options as part of the API. reloadReactNative or openDevMenu are only RN options.

Agree on this.


@grabbou usage without a JS interface currently implemented - this could be done with a custom action I'd imagine, something I covered here with an example: https://github.com/wix/detox/issues/207#issuecomment-355796998 (it's a slightly hacky but if it helps...)

LeoNatan commented 6 years ago

@Salakar I think we should move the discussion to #207. I would like to hear more how you have implemented your solution and perhaps discuss how to add the functionality in Detox.

LeoNatan commented 6 years ago

Device shake has landed for iOS.

grabbou commented 6 years ago

Great. Been waiting for this for so long! Thanks, looks like it's time to prototype some tests this week.

On Tue, 20 Mar 2018 at 16:28 Leo Natan notifications@github.com wrote:

Device shake has landed for iOS.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/wix/detox/issues/551#issuecomment-374640205, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWcxmsx4ulE2No5-iPWgXgxUjBGAtQYks5tgSAJgaJpZM4R1eUH .