Skip to content
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

Detox failed to install apk files #274

Closed
paynd opened this issue Sep 12, 2017 · 15 comments
Closed

Detox failed to install apk files #274

paynd opened this issue Sep 12, 2017 · 15 comments

Comments

@paynd
Copy link

paynd commented Sep 12, 2017

Hi there!

Description

I'm trying to add detox for android into existing project. We already got ios tests on detox running.
I found that both apk files (app-release.apk & app-release-androidTest.apk) are failing to install.

detox verb 5: stdout: [ 99%] /data/local/tmp/app-release.apk  
detox verb 5: stdout: [100%] /data/local/tmp/app-release.apk  
detox verb 5: stdout: /Users/paynd/Repo/Work/tipsi-react-app/android/app/build/outputs/apk/app-release.apk: 1 file pushed. 27.4 MB/s (12748911 bytes in 0.443s)  
detox verb 5: stdout: Error: Unknown option: -g

full log here

So, it looks like problem are here:
adb -s emulator-5554 install -r -g /Users/paynd/Repo/Work/tipsi-react-app/android/app/build/outputs/apk/app-release-androidTest.apk

If I change a line in EmulatorDriver.js
From: this.adbCmd(deviceId, 'install -r -g ' + binaryPath))
To: this.adbCmd(deviceId, 'install -rg ' + binaryPath))
Installation succeed.

Steps to Reproduce

Node, Device, Xcode and macOS Versions

  • Node: v7.10.0
  • Device: android emulator 5.1.1 (api 22)
  • Xcode: 8.3.3 (8E3004b)
  • macOS: 10.12.6 (16G29)

@simonracz @rotemmiz

@rotemmiz
Copy link
Member

Use an API 24 emulator , I think it only works on newer OS versions

@paynd
Copy link
Author

paynd commented Sep 12, 2017

@rotemmiz thanks! It works fine on API 24 emulator.
Any idea why it wont work on api 22?
Looks like -g standard adb install option

 install [-lrtsdg] PACKAGE
 install-multiple [-lrtsdpg] PACKAGE...
     push package(s) to the device and install them
     -l: forward lock application
     -r: replace existing application
     -t: allow test packages
     -s: install application on sdcard
     -d: allow version code downgrade (debuggable packages only)
     -p: partial application install (install-multiple only)
     -g: grant all runtime permissions

@heuism
Copy link

heuism commented Sep 26, 2017

Can I may ask that with this thread. Does this mean that Detox already support Android? Thanks.

@simonracz
Copy link
Contributor

@heuism
Yes. Please take a look at here for how to install and current caveats.

@Crash--
Copy link
Contributor

Crash-- commented Dec 13, 2017

We just encountered this error. Removing -g works, but I'm wondering :

  • Is it intentional ? Does Detox only support API >= 25 (Android 7), I personally hope that it (will) supports at least Android 4.4+
  • Can we had this to the readme or android start guide?

@simonracz
Copy link
Contributor

@Crash-- , I think you are right. This is something we could fix.

API lvl 23 introduced runtime permissions. If I remember correctly adb -g has issues with that, namely it ignores that parameter. So, that's a different topic.

However, we should be able to support API lvl 22 and lower and API lvl 24 and higher.

I'll take a look at this when I'll have time to work on detox again.

@simonracz simonracz self-assigned this Dec 13, 2017
@simonracz simonracz reopened this Dec 13, 2017
@simonracz
Copy link
Contributor

I spent a small time on this.

API lvl 22 device accepts adb -rg ... command, but fails the adb -r -g ... command.

API lvl 24+ device accepts adb -r -g ... command, but fails the adb -rg ... command.

Great. :(

  1. So we need to know the API lvl of the emulator, before issuing the adb command. (I am not sure yet, whether we can get this from telnet.)
  2. Issue different adb command based on that.

@simonracz
Copy link
Contributor

At least the latest RN versions doesn't need the -g flag anymore as this landed.

I mean, the main reason to call -g was the SYSTEM_ALERT_WINDOW permission for RN.
It still could be useful to call it.

@simonracz
Copy link
Contributor

Update: adb shell getprop ro.build.version.sdk works to get the API lvl

@simonracz
Copy link
Contributor

It accidentally works for API lvl 23 too. This whole adb parameter issue is weird.

@simonracz
Copy link
Contributor

This bothered me a lot even after the fix. So I made some research.

First weirdness, is that apparently there are multiple adbs in Android. And they diverged. The one that matters more to Google is obviously not the one provided for the users, but the one on the devices itself.
https://github.com/aosp-mirror/platform_system_core/blob/master/adb/Android.mk#L42

The command line parsers code is 2000+ LOC. Wow. There are tools that generate this code. But this was written by hand.

And here is the line, that causes this weird issue :
https://github.com/aosp-mirror/platform_system_core/blob/master/adb/commandline.cpp#L1908

On install command adb simply copies the arguments and sends them as it is. As the receiving end can be any old or new code, it breaks.

@rotemmiz
Copy link
Member

can AndroidDriver still support all of them if we just have different handling for different API levels ? similar to what you started doing

@simonracz
Copy link
Contributor

I don't get your question. What else are left to do here?

I tested it in the detox/test app with API levels from 21 - 26. It worked.

@rotemmiz
Copy link
Member

Nevermind, your last comment answered my question :)

@simonracz
Copy link
Contributor

simonracz commented Jan 12, 2018

:D Reading back this thread, I didn't communicate well the stages.

@wix wix locked and limited conversation to collaborators Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants