You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2020/10/05 08:12:34 UTC
[GitHub] [cordova-android] raphinesse opened a new pull request #1082: refactor(lib/run): simplify code to determine install/run target
raphinesse opened a new pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
This change simplifies the code that determines on which target (device or emulator) the app should be installed and run on.
This refactoring was triggered during a bigger refactoring (still WIP) that has the goal of doing something similar to https://github.com/apache/cordova-electron/pull/142 here.
### Description
<!-- Describe your changes in detail -->
The changes made here should not change the observable behavior of the code beyond changing what is logged to the console. The changes included are:
- factor out `resolveInstallTarget` from `lib/run.run`
- improve unit tests for the `lib/run` module
- convert `resolveInstallTarget` to async/await
- DRY logic in `resolveInstallTarget`
The logic for selecting a install/run target is unchanged. Targets are tried to resolve in this order:
- A connected device matching the given ID (if target ID given)
- Any connected device (if `--emulator` option was not given)
- A started emulator matching the given ID (if target ID given)
- An existing emulator matching the given ID (will be started) (if target ID given)
- An existing emulator that best matches the app's target API (will be started) (if `--device` option was not given)
### Testing
<!-- Please describe in detail how you tested your changes. -->
The unit tests for this module still pass.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse closed pull request #1082: refactor(lib/run): simplify code to determine install/run target
Posted by GitBox <gi...@apache.org>.
raphinesse closed pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-io edited a comment on pull request #1082: refactor(lib/run): simplify code to determine install/run target
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082#issuecomment-711086385
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=h1) Report
> Merging [#1082](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/5d3591b8537647c0cc145308028e537c83e7ffd6?el=desc) will **decrease** coverage by `0.14%`.
> The diff coverage is `93.10%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1082 +/- ##
==========================================
- Coverage 69.83% 69.69% -0.15%
==========================================
Files 20 20
Lines 1810 1808 -2
==========================================
- Hits 1264 1260 -4
- Misses 546 548 +2
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/run.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9ydW4uanM=) | `96.82% <92.00%> (-3.18%)` | :arrow_down: |
| [bin/templates/cordova/lib/device.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9kZXZpY2UuanM=) | `100.00% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/retry.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9yZXRyeS5qcw==) | `100.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=footer). Last update [5d3591b...dd6ffd9](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek edited a comment on pull request #1082: refactor(lib/run): simplify code to determine install/run target
Posted by GitBox <gi...@apache.org>.
breautek edited a comment on pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082#issuecomment-703967585
When on this PR, it doesn't appear that the CLI properly detects of the emulator is already running and fails to redeploy the app on the `cordova run` command on the running emulator.
```
BUILD SUCCESSFUL in 8s
41 actionable tasks: 5 executed, 36 up-to-date
Built the following apk(s):
/development/totalpave/tp-issue-tracker/platforms/android/app/build/outputs/apk/debug/app-debug.apk
Checking Java JDK and Android SDK versions
ANDROID_SDK_ROOT=/development/Android/Sdk (recommended setting)
ANDROID_HOME=/development/Android/Sdk (DEPRECATED)
Using Android SDK: /development/Android/Sdk
No emulator specified, defaulting to Pixel_API_29
Waiting for emulator to start...
emulator: ERROR: Running multiple emulators with the same AVD is an experimental feature.
Please use -read-only flag to enable this feature.
```
Prompt waits for the emulator to start when it's already running.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on pull request #1082: refactor(lib/run): simplify code to determine install/run target
Posted by GitBox <gi...@apache.org>.
raphinesse commented on pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082#issuecomment-704078874
Thanks for the thorough check @breautek. I'll look into this when I find the time. I converted this to a draft until then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek commented on pull request #1082: refactor(lib/run): simplify code to determine install/run target
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082#issuecomment-703967585
When on this PR, it doesn't appear that the CLI properly detects of the emulator is already running and fails to redeploy the app on the `cordova run` command on the running emulator.
```
BUILD SUCCESSFUL in 8s
41 actionable tasks: 5 executed, 36 up-to-date
Built the following apk(s):
/development/totalpave/tp-issue-tracker/platforms/android/app/build/outputs/apk/debug/app-debug.apk
Checking Java JDK and Android SDK versions
ANDROID_SDK_ROOT=/development/Android/Sdk (recommended setting)
ANDROID_HOME=/development/Android/Sdk (DEPRECATED)
Using Android SDK: /development/Android/Sdk
No emulator specified, defaulting to Pixel_API_29
Waiting for emulator to start...
emulator: ERROR: Running multiple emulators with the same AVD is an experimental feature.
Please use -read-only flag to enable this feature.
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on pull request #1082: refactor(lib/run): simplify code to determine install/run target
Posted by GitBox <gi...@apache.org>.
raphinesse commented on pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082#issuecomment-712368121
Closing in favor of #1101
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-io edited a comment on pull request #1082: refactor(lib/run): simplify code to determine install/run target
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082#issuecomment-711086385
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=h1) Report
> Merging [#1082](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/5d3591b8537647c0cc145308028e537c83e7ffd6?el=desc) will **decrease** coverage by `0.25%`.
> The diff coverage is `93.10%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1082 +/- ##
==========================================
- Coverage 69.83% 69.57% -0.26%
==========================================
Files 20 20
Lines 1810 1808 -2
==========================================
- Hits 1264 1258 -6
- Misses 546 550 +4
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/run.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9ydW4uanM=) | `96.82% <92.00%> (-3.18%)` | :arrow_down: |
| [bin/templates/cordova/lib/device.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9kZXZpY2UuanM=) | `100.00% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `58.49% <0.00%> (-0.95%)` | :arrow_down: |
| [bin/templates/cordova/lib/retry.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9yZXRyeS5qcw==) | `100.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=footer). Last update [5d3591b...dd6ffd9](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-io edited a comment on pull request #1082: refactor(lib/run): simplify code to determine install/run target
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082#issuecomment-711086385
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=h1) Report
> Merging [#1082](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/5d3591b8537647c0cc145308028e537c83e7ffd6?el=desc) will **decrease** coverage by `0.19%`.
> The diff coverage is `93.10%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1082 +/- ##
==========================================
- Coverage 69.83% 69.63% -0.20%
==========================================
Files 20 20
Lines 1810 1808 -2
==========================================
- Hits 1264 1259 -5
- Misses 546 549 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/run.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9ydW4uanM=) | `96.82% <92.00%> (-3.18%)` | :arrow_down: |
| [bin/templates/cordova/lib/device.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9kZXZpY2UuanM=) | `100.00% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `58.96% <0.00%> (-0.48%)` | :arrow_down: |
| [bin/templates/cordova/lib/retry.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9yZXRyeS5qcw==) | `100.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=footer). Last update [5d3591b...dd6ffd9](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-io commented on pull request #1082: refactor(lib/run): simplify code to determine install/run target
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1082:
URL: https://github.com/apache/cordova-android/pull/1082#issuecomment-711086385
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=h1) Report
> Merging [#1082](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/5d3591b8537647c0cc145308028e537c83e7ffd6?el=desc) will **decrease** coverage by `0.25%`.
> The diff coverage is `93.10%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1082 +/- ##
==========================================
- Coverage 69.83% 69.57% -0.26%
==========================================
Files 20 20
Lines 1810 1808 -2
==========================================
- Hits 1264 1258 -6
- Misses 546 550 +4
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/cordova/lib/run.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9ydW4uanM=) | `96.82% <92.00%> (-3.18%)` | :arrow_down: |
| [bin/templates/cordova/lib/device.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9kZXZpY2UuanM=) | `100.00% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `58.49% <0.00%> (-0.95%)` | :arrow_down: |
| [bin/templates/cordova/lib/retry.js](https://codecov.io/gh/apache/cordova-android/pull/1082/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9yZXRyeS5qcw==) | `100.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=footer). Last update [5d3591b...dd6ffd9](https://codecov.io/gh/apache/cordova-android/pull/1082?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org