Skip to content

Fix to enable appium ruby console (arc) on windows;#119

Merged
bootstraponline merged 1 commit into
appium:masterfrom
misttar:feature/appium_ruby_console_on_windows
Feb 20, 2014
Merged

Fix to enable appium ruby console (arc) on windows;#119
bootstraponline merged 1 commit into
appium:masterfrom
misttar:feature/appium_ruby_console_on_windows

Conversation

@misttar
Copy link
Copy Markdown
Contributor

@misttar misttar commented Feb 20, 2014

Removed unnecessary ruby based absolute path conversion in load_appium_txt, as it is done later in Driver::initialize;
Simplified absolute_app_path method for resolving windows paths;

Removed unnecessary ruby based absolute path conversion in load_appium_txt, as it is done later in Driver::initialize;
Simplified absolute_app_path method for resolving windows paths;
@bootstraponline
Copy link
Copy Markdown
Member

load_appium_txt is called without a driver, for example in the arc reload method. In this case, we want to explicitly ensure the app path is absolute because the .txt may define relative paths.

@misttar
Copy link
Copy Markdown
Contributor Author

misttar commented Feb 20, 2014

It isn't till we actually initialize a driver, that we need the app_path to be correct, and at that point it uses the absolute_app_path method to validate the path is correct. The Ruby expand path method doesn't take into account urls, app ID's, or saucelab connection parameters. Plus, it rewrites the path on windows in a style that can't be passed directly in a capabilities object on windows (replaces backslashes with forward slashes).

Even in the case of the reload method, it isn't till the Driver is initialize that we need or even check that the APP_PATH is correct.

bootstraponline added a commit that referenced this pull request Feb 20, 2014
…indows

Fix to enable appium ruby console (arc) on windows;
@bootstraponline bootstraponline merged commit 69f2058 into appium:master Feb 20, 2014
@bootstraponline
Copy link
Copy Markdown
Member

Makes sense. Merged.

@misttar misttar deleted the feature/appium_ruby_console_on_windows branch February 21, 2014 01:34
@bootstraponline
Copy link
Copy Markdown
Member

It turns out I was correct and this ended up breaking the app I'm testing.

`absolute_app_path': App doesn't exist /Users/user/.rvm/gems/ruby-2.0.0-p247/gems/appium_lib-0.19.0/lib/appium_lib/../../binary/app.apk (RuntimeError)

The issue is with relative paths and not calling File.expand_path. I think this needs some automated tests.

@misttar
Copy link
Copy Markdown
Contributor Author

misttar commented Feb 24, 2014

Damn, sorry. I will get some tests written and see if I can reproduce your issue and get another fix in quickly.

@bootstraponline
Copy link
Copy Markdown
Member

I pushed this fix to master.

@misttar
Copy link
Copy Markdown
Contributor Author

misttar commented Feb 24, 2014

Looks like a good fix, and I think an improvement in general on the absolute path call to make it easier to use later.

I do think there is a place for adding some Cucumber tests for the ruby_lib in general, to test these scenarios. I will take a stab at adding some basic test cases.

@bootstraponline
Copy link
Copy Markdown
Member

I have some tests here, working on adding the tests for this issue now.

@misttar
Copy link
Copy Markdown
Contributor Author

misttar commented Feb 24, 2014

Wow, didn't know those existed. Excellent. I will fork those and make sure they all pass/get extended for my future pull requests.

@bootstraponline
Copy link
Copy Markdown
Member

I added a few tests and it turns out the file system needs to be mocked.

driver::Appium::Driver#test_0003_absolute_app_path:
RuntimeError: App doesn't exist. C:\Program Files\myapp.apk

On OS X, that Windows path will never be valid.

@misttar
Copy link
Copy Markdown
Contributor Author

misttar commented Feb 24, 2014

If you aren't aware of it, https://github.com/defunkt/fakefs is around to help with this. Never tried it to mock a windows bath on OSX though.

@bootstraponline
Copy link
Copy Markdown
Member

That looks like it'll work. Opened #120 to track progress.

@bootstraponline
Copy link
Copy Markdown
Member

fakefs fails on windows paths for me (running on OS X). I pushed the tests to github.

I'm sure there's some way to mock the windows FS on non-Windows platforms. I'm out of time to spend on this particular issue though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants