Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Support for launching retina and tall devices #16

Open
wants to merge 10 commits into from

8 participants

@rhgills

I made a few changes which impact the public API used to launch the simulator:

  • Direct launching goes through launch_ios_app as before, but with changed parameters: an app_path and an options hash.

    This also cleans up the code to parse a http request into a call to launch_ios_app.

  • HTTP launching passes the device as a parameter to a new launch_ios_app endpoint, replacing the seperate ipad and iphone endpoints.
  • I removed the launch_ipad_app and launch_iphone_app methods which don't pull their weight anymore and aren't used by Frank.

I'll be submitting a pull request to Frank shortly which leverages this support. EDIT: moredip/Frank#218

I'd love to hear your thoughts on this.

@rhgills rhgills referenced this pull request in moredip/Frank
Open

Support for launching retina and tall devices #218

@rynbyjn

Would love this functionality within Frank and the SimLauncher!
+1

@moredip
Owner

Apologies it's taken a while for me to get back to you on this PR.

Thanks for the contribution, it looks good. One thing I noticed is that it replaces /launch_iphone_app and /launch_ipad_app with /launch_ios_app. For backwards compatibility I'd rather leave these endpoints around, even if they just delegate through to /launch_ios_app. I realize that you're updating the client too so there's less of a need for backwards compatibility, but it still makes me nervous to remove the endpoints.

rhgills added some commits
@rhgills rhgills Add TestApp. Extract logic out of bin/sim_launcher into SinatraLaunch…
…er. Fix bug where PhoneDevices included all devices, including PadDevices. Reimplement launch_ipad_app and launch_iphone_app for backwards compatibility. Add specs for direct and indirect launching.
c244882
@rhgills rhgills Remove extra Gemfiles. 2740630
@rhgills rhgills fix broken manual_direct_acceptance_test 3afde57
@rhgills rhgills Move the manual tests out of spec. ca22dba
@rhgills

Hi Pete,

No worries. Please accept my apologies for taking so long to get back to you as well.

As you suggested, I reimplemented the /launch_iphone_app and /launch_ipad_app endpoints, as well as the launch_iphone_app and launch_ipad_app methods on the Simulator. This does clutter the code, so if the concern is only regarding the sinatra endpoints then these should still be removed. I also took the opportunity to add some tests (and found a bug which broke launching on ipad).

The simulator_spec is ugly, but it tests that the correct launch command is sent to ios-sim. Currently it does so by verifying the exact string sent to Kernel#` to be executed. Going forward, it looks like the project would benefit from extracting the dependencies on ios-sim from the Simulator, which would clean up the tests of Simulator by at least moving the use of Kernel#` off into some IosSimWrapper component. The spec currently returns hardcoded values for the xcode version and the installed SDKs, which breaks the dependency on the installed version of Xcode. But again, this could be made more robust.

@rhgills

Looks like SImulator#start_simulator and Simulator#reset might also benefit from some changes allowing specifying a retina or tall device.

@RayAnd

guys, any chance this will go to next release?

@mgrebenets

I'll join to the last question asked.

Really looking forward for this type of change to be part of the latest gem version.
In fact, I have an additional feature request related to this PR.

The latest ios-sim 1.8.2 supports lots of other arguments, besides --tall and --retina.
The one I am most interested in is --env which allows to pass a plist file with key-value pairs for environment variables to be set.
The app can then parse these environment variables from [[NSProcessInfo processInfo] environment] dictionary.
Right now I am writing Calabash tests for such an app.
Of course, that' not really a feature of the final product, but it comes really handy when we need to point the app to another server endpoint for testing purposes, to enable development mode and so on, all that using environment vars.

It can also be done using --setenv option, but using plist is more compact.

I'd really appreciate any update on this one.

@matthewsinclair

I'd really like to see this one merged as well. Thanks!

@jdhovland

Another vote for merge.

@sagpatil

+1 for having this feature

@moredip
Owner

I'd love to pull this functionality in, but the PR is big and has been sitting for so long that it'd be a bit of a beast to merge with current master. I realize that this is my fault for not pulling it in once it was ready :(

Anyone want to help out with getting it into a mergeable state? @rhgills?

@rhgills

Haven't been using Frank lately and have been pretty busy as of late... Sadly I doubt I'll be able to help out much for the foreseeable future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 19, 2013
  1. @rhgills
Commits on May 20, 2013
  1. @rhgills

    Fix broken for_ipad and for_iphone methods on Client and DirectClient…

    rhgills authored
    …. Replace device type and families with constants. Add documentation.
  2. @rhgills

    Fix crash in sinatra server.

    rhgills authored
  3. @rhgills

    Make constants less horrible.

    rhgills authored
  4. @rhgills
Commits on May 22, 2013
  1. @rhgills

    Bump ios-sim to v1.7.

    rhgills authored
Commits on Jul 12, 2013
  1. @rhgills

    Add TestApp. Extract logic out of bin/sim_launcher into SinatraLaunch…

    rhgills authored
    …er. Fix bug where PhoneDevices included all devices, including PadDevices. Reimplement launch_ipad_app and launch_iphone_app for backwards compatibility. Add specs for direct and indirect launching.
  2. @rhgills

    Remove extra Gemfiles.

    rhgills authored
  3. @rhgills
  4. @rhgills
Something went wrong with that request. Please try again.