colszowka / phantomjs-gem

Phantomjs via Rubygems: Auto-install phantomjs on demand for current platform. Comes with poltergeist integration.
Other
222 stars 104 forks source link

Fix for windows x64 machines #69

Open pf-bajorek opened 8 years ago

pf-bajorek commented 8 years ago
  1. Phatnomjs works for x64 machines so it doesn't have to be validate by architecture.
  2. To find phanotm path we use whereinstead of which
colszowka commented 8 years ago

@pf-bajorek Thanks - the build is failing though because of the changed where/which line for the windows tests. Could you fix that test please?

pf-bajorek commented 8 years ago

@colszowka Hi! I checked travis and failed examples is:

rspec ./spec/platform_spec.rb:176 # Phantomjs::Platform on Windows with system install returns the correct phantom js executable path for the platform

The error encourage because it is executed on linux machine:

Called from /home/travis/build/colszowka/phantomjs-gem/spec/platform_spec.rb

I don't know whether trevis can run tests on windows?

What do you do to fix this test? Skip it on unix machines?

colszowka commented 8 years ago

The test suite is built to simulate the corresponding platforms by stubbing out the respective root calls, see the setup block of the failing test in https://github.com/colszowka/phantomjs-gem/blob/master/spec/platform_spec.rb#L177 :)

pf-bajorek commented 8 years ago

@colszowka done :)

pf-bajorek commented 8 years ago

So, will it be merged?

pf-bajorek commented 8 years ago

ping

pf-bajorek commented 8 years ago

@colszowka Are you planing to merge this issue to master and push new gem version?

karlhe commented 8 years ago

I would suggest 2 additional changes.

To account for more windows setups (mine identifies as mswin32):

        def useable?
          host_os =~ /mingw|mswin|cygwin/
        end

To account for multiple phantomjs.exe on path:

        def system_phantomjs_path
          `where phantomjs`.split("\n")[0]
        rescue
        end
``