Detox
Detox copied to clipboard
Disable RN Timers and Animation synchronization to prevent tests from stalling
Is your feature request related to a problem? Please describe.
It's related to a problem we've been facing at Skyscanner. Some of our tests were stalling in Android because of ReactNativeTimersIdlingResource.isIdleNow() and AnimatedModuleIdlingResource.isIdleNow() never returning true in some cases. I couldn't identify exactly when this happens since some of the times this happened we didn't have any infinite animations running in that particular screen.
Describe the solution you'd like
We already have the disableSynchronization() function but that's only disabling the network synchronization. So my proposal is to add similar methods to disable RN timers synchronization and animation synchronization. You can see an already implemented version of this here: https://github.com/leanmazzu/Detox/commit/efb0dcfd799ffea3d91bd1c8e46ee84df3950dd6. This is what we currently have in place in our Skyscanner fork and it's working just fine for us.
Describe alternatives you've considered N/A
Additional context
This would be a non-breaking change as it's not modifying the current disableSynchronization() method or anything else.
Hmm disableSynchronization() should disable all synchronization. @rotemmiz @d4vidi Why is this not the case in Android?
@leanmazzu Feel free to submit a PR that implements this.
Cool will do @LeoNatan , just one question... the solution I proposed above is only adding the functionality to disable ReactNativeTimersIdlingResource and AnimatedModuleIdlingResource. That was enough for us but, do you think it's worth adding the functionality to disable ReactNativeUIModuleIdlingResource as well? Looks like it could help solve this issue https://github.com/wix/Detox/issues/1435
I think a disableSynchronization() option should disable everything, like it does on iOS. Waiting for @rotemmiz
It should, and if it doesn't we need to fix it. Also, we should probably add per IR disable mechanism (as much as we can)
We don't have that mechanism on iOS either for now. But at least the main disable should disable 😄
@leanmazzu We'd be happy to receive a pull request. Thank you!
Ok cool, I'll do this then:
disableSynchronization()will now disable all the IR.- There will be a
disablemethod for every IR so that they can be disabled/enabled separately.
Let me know if I'm missing something here. Otherwise, I'll send the PR implementing this as soon as it's ready. Thank you!
Sounds good. Thanks!
Sounds about right. Nevertheless @leanmazzu I'm highly intrigued with what your use cases are, I can't help but think whether possibly you're just struggling with an actual bug in one of the idling resources. Either way, 'love the Skyscanner app :)
Usually what we've found on iOS is that people disable sync when they have endless animations or some network. For network, we have a way to disable sync on iOS. Not sure if the same API works for Android (@rotemmiz ?). Regarding animations, we usually recommend mocking the animation calls to only run once or not at all during Detox tests.
Network disable/enable sync is implemented in Android, same API. https://github.com/wix/Detox/blob/master/detox/test/e2e/14.network.test.js#L29
@rotemmiz I meant this: https://github.com/wix/detox/blob/master/docs/APIRef.DeviceObjectAPI.md#deviceseturlblacklisturls
But I see setURLBlacklist() is used in the tests. 👍
Thank you all guys! The PR is ready to review (https://github.com/wix/Detox/pull/1521)
Sounds about right. Nevertheless @leanmazzu I'm highly intrigued with what your use cases are, I can't help but think whether possibly you're just struggling with an actual bug in one of the idling resources. Either way, 'love the Skyscanner app :)
@d4vidi in our case we have some screens with complex view hierarchies. We tried mocking all the animations, RN even provides a way of doing this. But that didn't work for us, as some components were not working at all with animations disabled. Anyways, I'm glad you like Skyscanner app :)
That RN feature is completely broken. On iOS, we force it disabled so that apps work. It's not intended for e2e testing.
It seems I am hitting the same problem, I get very randomly below errors after test job timeouts:
(node:19198) UnhandledPromiseRejectionWarning: Error: Looped for 1978 iterations over 60 SECONDS. The following Idle Conditions failed.
detox[22054] WARN: [Client.js/PENDING_REQUESTS] App has not responded to the network requests below: (id = 110) invoke: {"target":{"type":"Class","value":"com.wix.detox.espresso.EspressoDetox"},"method":"perform","args":[{"type":"Invocation","value":{"target":{"type":"Class","value":"androidx.test.espresso.Espresso"},"method":"onView","args":[{"type":"Invocation","value":{"target":{"type":"Class","value":"com.wix.detox.espresso.DetoxMatcher"},"method":"matcherForTestId","args":["back_button"]}}]}},{"type":"Invocation","value":{"target":{"type":"Class","value":"com.wix.detox.espresso.DetoxViewActions"},"method":"click","args":[]}}]} That might be the reason why the test "Teddy scenario Explore" has timed out.
Do you guys think these logs are related? This seems to happen after tapping buttons that trigger an animation
disableSynchronization does nothing at all for me, not even turning off network. I also tried setURLBlacklist or detoxURLBlacklistRegex without any results
I'm hitting a similar issue, UIManagerModule is always busy (because of a looping animation). Was about to implement disable myself but found this! Thanks @leanmazzu for the PR!
@maxime-helen I found it useful to check the instrumentation app logs:
adb logcat *:S Detox:V -T 50
If your app is stuck and those actions are timing out then check the logs for messages like "UIManagerModule is busy"
Will this be merged any time soon? Currently blocked on running android tests as well on RN 0.64. My tests start up by typing is about 1 character per 2 seconds, which causes timeouts. I also see a "UIManagerModule is busy" printed quite frequenty b/w characters appearing
@rotemmiz let's give this high prio, I don't think there's too much left to be done here
heads up: I'm not actively working on this as my team's priorities have shifted a little, especially since the branch in its current state works great for us (thanks @leanmazzu).
Is there any update on this? I'm finding since we upgraded our app to Wix3, this issue started reproducing. I get the Network is busy line when doing logcat, the waiting is a lot worse on Android than on iOS.
@leanmazzu hey, did you open a PR for this in the end? I am experiencing the same issue
@ronilitman he initially has, but it's become obsolete after requiring various changes. If you wanna take it we'd be happy to accept it.
Thank you all guys! The PR is ready to review (#1521)
Sounds about right. Nevertheless @leanmazzu I'm highly intrigued with what your use cases are, I can't help but think whether possibly you're just struggling with an actual bug in one of the idling resources. Either way, 'love the Skyscanner app :)
@d4vidi in our case we have some screens with complex view hierarchies. We tried mocking all the animations, RN even provides a way of doing this. But that didn't work for us, as some components were not working at all with animations disabled. Anyways, I'm glad you like Skyscanner app :)
These RN files are totally broken, they don't preserve animation behaviors (like call a callback or setting final values when they finish), I've made a patch to apply in my project for them (see the link), at least with it my tests on android were working again
@Grohden kudos for getting that fixed - even though that in essence, I advise not to disable animations in E2E-level testing. Rather, in our projects, we've force-disabled animations mocking altogether: https://github.com/facebook/react-native/issues/27010#issuecomment-573645965
I have this issue, my use case is as follows (and I'd happily accept guidance to perhaps mitigate it):
- Start a single-screen test-only app that just refreshes with status in order to run a big queue of tests, no UI interaction is needed
- Apparently (? open question) if there is no UI interaction the tests crawl because it appears that Detox requires UI interaction in order to move quickly?
- So we send a steady stream of "adb shell input tap" events to the emulator and things move along, but...
- If react-native catalyst instance goes down in between an android MotionEvent.ACTION_DOWN and MotionEvent.ACTION_UP, then it only registers the ACTION_UP, and that apparently leaves something on the queue as detected by UIModuleIdlingResource here, so that queue is permanently blocked (this could be a react-native bug as well then, I suppose)
- So now the app is stuck and never sends the testee-side login / ready event, so the test never starts
So in my use case, I don't care at all about synchronization, I'd like to disable it completely on Android, exactly as the API disableSynchronization would indicate.
@d4vidi are you still interested in someone picking up the android disableSynchronization PR? It seems at the least, disable should...disable all synch (not just network) ?
Is there some way to make detox pick up events and keep moving, without UI interaction? Perhaps I'm missing some timing config?
Is anyone familiar with react-native MotionEvent handling and why a broken pair of ACTION_DOWN/ACTION_UP events might jam up the UIManager queue?
Cheers
I am having tests that are failing in Android with UI Kitten modals. I think it is an infinite animation when I open modals
Is there any alternative to avoid detox stalling? I tried disableSynchronization() but this only works for iOS...
https://github.com/akveo/react-native-ui-kitten/issues/1250
@napsta32 depending on your needs, you can chop any part of Detox out. I completely cut off the UIManagerIdlingResource from being able to block testing for react-native-firebase because for my case I wasn't driving the UI, I was just running effectively headless tests yet the UIIdlingResource has some bug that causes it to halt anyway.
So you just cut it out as a patch-package patch like so https://github.com/invertase/react-native-firebase/blob/master/tests/patches/detox%2B16.7.2.patch
If you're driving the UI then this won't work, but it's an example of bending Detox to your will for your situation
Thanks @mikehardy
This solution is working for my UI tests (somehow)
I guess if I have any issues I will try to fix the disableSynchronization() UIManagerIdlingResource bug
There was a PR created (https://github.com/wix/Detox/pull/1521) to fix this a long time ago but for some reason was never merged. We have a similar issue where we have an animation loop (custom input blinking cursor) on certain screens that there is just no good way to test on those screens without hacks. Disable animation synchronization on that screen would be a huge help, works on iOS but not on android
@martintreurnicht , maybe I can look at #1521 next week, but if you feel like you can resurrect and finish the work, you are more than welcome to! 👍
@d4vidi, please advise if a better solution than #1521 would actually be working on debugSynchronization for Android? I'm not 100% sure what our stance on it is.
/cc @rotemmiz