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

cordova run --list --device fails for Virtualhere devices (fixes #458) #461

Merged
merged 2 commits into from
Jan 9, 2019

Conversation

regiMario
Copy link
Contributor

@regiMario regiMario commented Nov 12, 2018

Platforms affected

iOS

What does this PR do?

I'm using Virtualhere (https://www.virtualhere.com/home) for developing remotely.
While Safari and Xcode do see the device after connecting via Virtualhere, cordova does not.
The attached patch remedies the issue in [email protected]

What testing has been done on this change?

Manual testing with virtualhere devices as well as regular USB devices

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

closes #458

@brody4hire brody4hire added the bug label Nov 12, 2018
@dpogue
Copy link
Member

dpogue commented Nov 12, 2018

Just a note that the test failures are coming from these tests that are checking to see if a command matching /system_profiler/ was invoked. You can update those to check for /ioreg/ and they should pass.

@regiMario
Copy link
Contributor Author

I tried running the tests before i uploaded and got this far.
`[mario@mac13dev ~/github/cordova-ios]$ npm test

[email protected] test /Users/mario/github/cordova-ios
npm run unit-tests && npm run test:component && npm run objc-tests && npm run e2e-tests

[email protected] unit-tests /Users/mario/github/cordova-ios
jasmine --config=tests/spec/unit.json

DEPRECATION: Setting throwOnExpectationFailure directly on Env is deprecated, please use the oneFailurePerSpec option in configure
DEPRECATION: Setting randomizeTests directly is deprecated, please use the random option in configure
internal/modules/cjs/loader.js:582
throw err;
^

Error: Cannot find module 'cordova-common'
at Function.Module._resolveFilename (internal/modules/cjs/loader.js:580:15)
at Function.Module._load (internal/modules/cjs/loader.js:506:25)
at Module.require (internal/modules/cjs/loader.js:636:17)
at require (internal/modules/cjs/helpers.js:20:18)
at Object. (/Users/mario/github/cordova-ios/tests/spec/unit/Api.spec.js:22:21)
at Module._compile (internal/modules/cjs/loader.js:688:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
at Module.load (internal/modules/cjs/loader.js:598:32)
at tryModuleLoad (internal/modules/cjs/loader.js:537:12)
at Function.Module._load (internal/modules/cjs/loader.js:529:3)
npm ERR! code ELIFECYCLE
npm ERR! errno 4
npm ERR! [email protected] unit-tests: jasmine --config=tests/spec/unit.json
npm ERR! Exit status 4
npm ERR!
npm ERR! Failed at the [email protected] unit-tests script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?
npm ERR! Test failed. See above for more details.
[mario@mac13dev ~/github/cordova-ios]$ npm -g list --depth=0
/usr/local/lib
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]`

@codecov-io
Copy link

Codecov Report

Merging #461 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #461   +/-   ##
=======================================
  Coverage   74.29%   74.29%           
=======================================
  Files          12       12           
  Lines        1564     1564           
=======================================
  Hits         1162     1162           
  Misses        402      402

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62ebfbd...8ad9dc5. Read the comment docs.

@janpio
Copy link
Member

janpio commented Nov 13, 2018

@regiMario Thanks for fixing the tests. Could you please report your problems running the tests in a new issue, with the output you already posted? This should be investigated separately. Thanks.

Copy link
Member

@dpogue dpogue left a comment

Choose a reason for hiding this comment

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

This looks good to me.

For those wondering, ioreg is a lower-level call than system_profiler, but they serve similar purposes as far as I can tell and there are several StackOverflow posts about macOS scripts where people are encouraged to switch to ioreg to get the info they're looking for.

@shazron shazron changed the title fix issue #458 cordova run --list --device fails for Virtualhere devices (fixes #458) Jan 9, 2019
@shazron
Copy link
Member

shazron commented Jan 9, 2019

Tested with the nightly. Detects my device ok

@shazron shazron merged commit dc03987 into apache:master Jan 9, 2019
erisu pushed a commit to erisu/cordova-ios that referenced this pull request Jan 16, 2019
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.

cordova run --list --device fails for Virtualhere devices
6 participants