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

[TIMOB-25742] Fixed adb issue where it fails to install to Android 4.1 devices #22

Merged
merged 2 commits into from Feb 2, 2018

Conversation

jquick-axway
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-25742

Summary:

  • Was failing to install because our adb install command was passing argument -d, which is only supported on Android 4.2 and higher.
  • Now checks the device's API Level and only adds the -d argument if supported on device.
  • Note that install was failing near the end of the install, once the APK had been finished copying over to the device. So, it's better to check device API Level in advance for best performance.

Test:
Run the test attached to TIMOB-25742.

lib/adb.js Outdated
}

// Set up the 'adb' arguments array.
let arguments = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arguments is a special word in JavaScript. Please use another variable name like args or something. Also, this should be const.

lib/adb.js Outdated
if (devices.filter(function (d) { return d.id == deviceId }).length != 1) return callback(new Error(__('device not found')));

devices = devices.filter(function (d) { return d.id == deviceId });
if (devices.length != 1) return callback(new Error(__('device not found')));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, we prefer !== over !=. Same goes for the == on the previous line.

lib/adb.js Outdated
if (deviceApiLevel >= 17) {
// Set the '-d' argument to install an older APK over a new one.
// Note: Only supported on Android 4.2 (API Level 17) and higher.
arguments.push('-d');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we keeping the -d flag anyways? According to the adb help screen, it says "allow version code downgrade (debuggable packages only)". Do we care?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume someone must have been burned by this at some point. Doesn't hurt to keep it. But I agree that this should be a rare event.

@jquick-axway
Copy link
Contributor Author

Updated PR based on feedback.

@cb1kenobi
Copy link
Contributor

APPROVED

@cb1kenobi cb1kenobi merged commit dc5dde9 into tidev:master Feb 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants