feat(Android): optimize app installation. #3195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d4vidi
requested changes
Feb 2, 2022
There was a problem hiding this comment.
Hey hey,
this is going in a good direction but requires more work, still. We will run over this through several iterations.
Besides my comments, please be sure to also provide details regarding the positive impact this has over the overall execution time. I urge you to utilize the timeline artifact in order to be able to do that...
🙏🏻
detox/src/devices/common/drivers/android/tools/AppInstallHelper.js
Outdated
Show resolved
Hide resolved
detox/src/devices/common/drivers/android/tools/AppInstallHelper.js
Outdated
Show resolved
Hide resolved
detox/src/devices/common/drivers/android/tools/CryptoUtils.test.js
Outdated
Show resolved
Hide resolved
detox/src/devices/common/drivers/android/tools/AppInstallHelper.js
Outdated
Show resolved
Hide resolved
jonathanmos
changed the title
d4vidi
requested changes
Feb 22, 2022
detox/src/devices/common/drivers/android/tools/AppInstallHelper.test.js
Outdated
Show resolved
Hide resolved
detox/src/devices/common/drivers/android/tools/FileXfer.test.js
Outdated
Show resolved
Hide resolved
detox/src/devices/common/drivers/android/tools/HashFileXfer.test.js
Outdated
Show resolved
Hide resolved
d4vidi
reviewed
Feb 22, 2022
detox/src/devices/common/drivers/android/tools/HashHelper.test.js
Outdated
Show resolved
Hide resolved
d4vidi
reviewed
Feb 22, 2022
d4vidi
reviewed
Feb 22, 2022
detox/src/devices/common/drivers/android/tools/HashHelper.test.js
Outdated
Show resolved
Hide resolved
d4vidi
force-pushed
the
improve-apk-install-attempt2
branch
from
7168e33
to
0285e55
Compare
# Conflicts: # detox/src/Detox.js # detox/src/Detox.test.js # docs/APIRef.DetoxCLI.md
asafkorem
changed the title
|
Starting a review. |
asafkorem
requested changes
Jul 14, 2022
| @@ -28,6 +28,7 @@ describe("Test", () => { | |||
|
|
|||
| beforeAll(async () => { | |||
| await device.reloadReactNative(); | |||
| await device.resetAppState(); | |||
There was a problem hiding this comment.
why here? @noomorph / @jonathanmos
There was a problem hiding this comment.
@asafkorem it is not a test per se, it is a typing/compiler error check thing.
asafkorem
approved these changes
Jul 18, 2022
d4vidi
reviewed
Jul 19, 2022
d4vidi
reviewed
Jul 19, 2022
| } | ||
|
|
||
| async readFile(deviceId, filepath, silent = false) { | ||
| if (silent) { |
There was a problem hiding this comment.
This never seems to be called with silent=false. Is this really necessary?
@jonathanmos
There was a problem hiding this comment.
It's to allow for calling nonsilently in future. Atm it's called only from the optimized flow
d4vidi
reviewed
Jul 19, 2022
d4vidi
reviewed
Jul 19, 2022
d4vidi
reviewed
Jul 19, 2022
d4vidi
reviewed
Jul 19, 2022
d4vidi
reviewed
Jul 19, 2022
d4vidi
reviewed
Jul 19, 2022
d4vidi
reviewed
Jul 19, 2022
jonathanmos
added a commit
that referenced
this pull request
Jul 20, 2022
This reverts commit 9221257.
jonathanmos
added a commit
that referenced
this pull request
Jul 20, 2022
This reverts commit 1054887.
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Resolves #3175
Resolves #2537
Implemented only for Android.
resetAppStateto decide between clearing user data and reinstallation.behaviorconfig,optimizeAppInstallwith a corresponding CLI flag:--optimize-app-install.In terms of performance: uninstall/install takes 6-7s, whereas clearing app state takes 400-600ms.
This pull request fixes #2534 .