-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
Android API 32, 33 (v12): .tap() acts like tap+hold #3762
Comments
I'm also seeing this issue on Android API 33. Detox version: 20.1.0
UPDATE: |
@AdamTyler Thanks for the confirmation. I was trying to find the way of how theoretically I can overcome this with other tap methods and longPress() can help, but not for every element. If the element is listening for longPress() it will act like longPress() was fired even with longPress(1) . |
I am seeing the same issue across two different projects with:
|
@sergtimosh @AdamTyler @fossage We would very much appreciate a reproduction using DetoxTemplate so we could better inspect this. In essence, we've seen the various |
I can confirm I had the same issue.
I was using emulator with API 33 - that was causing all the trouble. |
@d4vidi just checked with DetoxTemplate and API 33 system image and it's failing there too. You can check the screencast below. I've just added 2s timeouts before and after tap() so it's clearly visible that element isn't released. |
I am seeing the same issue on Detox 20.1.1 + React Native 0.67.4 Also using API 31 on emulator made it work |
I'm triaging, and will report ASAP. |
I can approve this reproduces clearly with API v33. Presumably, something in Detox's tap-untap events injection fails to execute properly in a system running the recent API. Perhaps upgrading Espresso ( |
@d4vidi just tried to build my fork with Espresso upgraded to (3.5.1) and got build failure(the 3.4.0 builds fine). Anyway I don't feel enough competent to dig down into this.
|
@sergtimosh have you had any luck on this issue? I am running into the same problem. I can confirm that downgrading to 31 works for the emulator; however, I would like to run some of my tests on a physical android device that is running 32, and I am still seeing the tap holding. |
Apparently Espresso 3.5.1 comes with some API changes / deprecations removal. I doubt we would be able to get into this in the near future - would appreciate a more thorough attempt. |
We are likely to address this within the upcoming 1-2 months. |
Hello, thank you for maintaining Detox, it's a great library. Unfortunately, experiencing this as well on API 33. Is there any news on this? |
Blocking android on API 34 |
I have same on
This is very crucial bug! |
Also facing this issue. Any idea what might be causes the issue with the Android sdk? It seems like |
As pointed out by d4vid, the root cause seems to be linked to the custom DetoxMultiTap implementation. I've found a temporary solution by using the standard espresso Tap.SINGLE instead of DetoxMultiTap. Please refer to the attached patch file for details. However, this approach does have its drawbacks. As discussed in DetoxMultiTap, this change introduces some timing inconsistencies, particularly taps being misinterpreted as long taps. For my specific application, which doesn't utilize long tap handlers, the Tap.SINGLE solution is satisfactory for now. I intend to delve deeper into the DetoxMultiTap issues and try to resolve them when I can. However, I'm quite pressed for time at the moment. It would be greatly appreciated if someone else could also investigate this in the interim.
(Tested on Android 34) |
This patch seems to fix the issue. I updated Can someone confirm? Patch
|
@ericschaal (🏆 thanks for your patch), @gomesbreno, @sdurandeu, @mostafaberg, @AdamTyler, @isilher, @otech47, @markpar, @sunchanras, @fossage, @catanet, @d4vidi, @jeroen-van-dijk, @BBKolton, @AnisDerbel, @cortisiko, @FadiAboMsalam, @Rednegniw, @programystic-dev, @ShivamJoker, @dooleyb1, @cpham-groundswell, @sergtimosh, @arthwood, @zwerg44, @andac-ozcan, @otoniel-isidoro, @manuquentin, @codexico, @nateshmbhat, @vjdhanota, @raldred. The patched version is available as detox@20.13.1-smoke.0 on NPM. Could you please verify if the fix works and if there are any undesirable side effects? |
I'll give that a quick test on Monday morning and report back 👌 great work ! |
I tried it out and this issue is fixed for my tests and app. However, I'm now running into #2787. 😆 |
@noomorph Thanks for the support! We tested the patched version and it solved our issues! 🚀 |
Thanks for the feedback. |
This unfortunately does not seem to resolve things fully, might cause some confusion, as @markpar mentioned, the swiping does not work. In our case, we have a global scroller that handles all SectionList, Flatlist & Scrollview components, while looking for a component within any of those, we'll automatically scroll until found. this behavior is pretty much what makes/breaks the tests since lots of components are outside the screen during e2e tests. it does fix the tapping though, which is a plus, but I'm not sure if that's ideal 🤔 |
Disregard that comment. I was using an Android 11 phone and I thought I was on Android 13. 🤦 |
@mostafaberg AFAIU, tapping handlers and scrolling handlers are separate pieces of code, so I don't think we should wait for the scrolling fixes to release @ericschaal's patch. If anyone has time soon to play with the scrolling code and see what changes can fix it, you are welcome – Use |
You're 100% correct this does fix the tapping issue at least!, maybe leave a small "known issue" note in the release notes to avoid more tickets coming in later 😄 |
Okay, sure, I'll add that to release note. Thanks for the suggestion! Overall, I'd be glad to help further with Android 31+ bugs, and maybe eventually I'll do if my team continues to be unavailable for a long time, but as you can see sometimes these fixes are just ±10 lines of code, so I encourage everyone who's comfortable enough with Android to look into it. 🙏 Pull requests from the community remain our top 1 priority, I can assure you'll get reviews and assistance quickly. |
Will try to get more time to get this done, my initial approach was to upgrade espresso but I didn't get enough time as it was a bigger change than I could manage at the time :( , might as well find some smaller changes as you recommended and get things rolling 👍 |
✅ The fix is available in |
I was able to get scrolling working with an almost identical patch to what @ericschaal provided. Unfortunately, swiping is still not working. It might be fixable by implementing a custom swiper instead of relying on the built-in implementations Patchdiff --git a/node_modules/detox/android/detox/src/main/java/com/wix/detox/espresso/action/common/MotionEvents.kt b/node_modules/detox/android/detox/src/main/java/com/wix/detox/espresso/action/common/MotionEvents.kt
index d8fc451..502ad40 100644
--- a/node_modules/detox/android/detox/src/main/java/com/wix/detox/espresso/action/common/MotionEvents.kt
+++ b/node_modules/detox/android/detox/src/main/java/com/wix/detox/espresso/action/common/MotionEvents.kt
@@ -9,8 +9,37 @@ import androidx.test.espresso.action.MotionEvents
private val PRECISION = floatArrayOf(16f, 16f)
class MotionEvents {
- fun obtainMoveEvent(downEvent: MotionEvent, eventTime: Long, x: Float, y: Float): MotionEvent
- = MotionEvents.obtainMovement(downEvent.downTime, eventTime, floatArrayOf(x, y))!!
+ fun obtainMoveEvent(downEvent: MotionEvent, eventTime: Long, x: Float, y: Float): MotionEvent {
+ val pointerProperties = MotionEvent.PointerProperties().apply {
+ clear()
+ id = 0
+ toolType = MotionEvent.TOOL_TYPE_UNKNOWN
+ }
+ val pointerCoords = MotionEvent.PointerCoords().apply {
+ clear()
+ this.x = x
+ this.y = y
+ this.pressure = 0f
+ this.size = 1f
+ }
+
+ return MotionEvent.obtain(
+ downEvent.downTime,
+ eventTime,
+ MotionEvent.ACTION_MOVE,
+ 1, // pointerCounts
+ arrayOf(pointerProperties),
+ arrayOf(pointerCoords),
+ 0, // metaState
+ downEvent.buttonState,
+ downEvent.xPrecision,
+ downEvent.yPrecision,
+ 0, // deviceId
+ 0, // edgeFlags
+ downEvent.source,
+ 0
+ )
+ }
fun obtainDownEvent(x: Float, y: Float, precision: FloatArray = PRECISION)
= obtainDownEvent(x, y, precision, null) |
* feat: generate psk when creating community; add psk connection protector to libp2p * feat: add base64 encoded psk to invitation link * chore: refactor invitation url parsing functions * test: temporarily skip backward compatibility e2e test - psk changes are not backward compatible * chore: update detox version - fixes 'tap' wix/Detox#3762 * chore: increase timeout for multiple clients e2e test
## **Description** The purpose of this PR is to accomplish two things: bump detox version (this hasn't been done in a couple of months) Implement dynamic scroll in tests Previously, the use of APIs higher than 30 was hindered by an Android blocker on Detox's end. As mentioned in #6384 > There is currently an issue when running detox on android 12 and 13 (API 32/33) which prevents the tests from running. The issue is, the tap() action is treated like a tapAndHold() action. See the open issue in wix/detox wix/Detox#3762 With this new detox version bump, the blocker has been lifted, allowing us to utilize Android 31+. Furthermore, how we scroll to elements introduces flakiness. Instead of hardcoding a scroll speed in tests, we will now introduce dynamic scrolling. We will scroll on a view until the element is visible within the viewport. This was done using [whileElement](https://wix.github.io/Detox/docs/api/actions/#whileelementelement) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** Smoke run: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/42936d4e-1df8-40ca-bd88-4f408cdd655a Regression Run: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/cb83ae29-a0a4-49f5-a2f5-7c6683bf38e3 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Chris Wilcox <chris.wilcox@consensys.net>
What happened?
When detox tests executed against devices/emulators with API 33 level .tap() function acts like tap + hold(unreleased tap)
What was the expected behaviour?
tap should release
Was it tested on latest Detox?
Did your test throw out a timeout?
Help us reproduce this issue!
No response
In what environment did this happen?
Detox version: 20.1.0
React Native version: 0.68.2
Node version: 16.14.0
Device model: pixel 6a
Android version: 12
Test-runner (select one): jest
Device logs
another tap() example
The text was updated successfully, but these errors were encountered: