search

dflynn15 / gulp-jasmine-phantom

Gulp plugin to run Jasmine tests with Phantom or mininodejasmine2.
27 stars 30 forks source link

Fix path to phantomjs executable and file system paths on Windows #66

Closed gwynjudd closed 8 years ago

gwynjudd commented 8 years ago

Resolves #53

This combines a fix from @Nysosis (https://github.com/dflynn15/gulp-jasmine-phantom/issues/53#issuecomment-200033552) along with fixing the name to run the phantomjs executable

dflynn15 commented 8 years ago

Have you tested this with Phantom versions under 2.x? I believe the Phantom executable was different for older versions which is why it had previously been asked to execute with the .cmd

gwynjudd commented 8 years ago

@dflynn15 I don't see why that would matter - if phantom was installed as "phantom.cmd", running "phantom" would still pick it up?

dflynn15 commented 8 years ago

That's what I had assumed before, but was asked to include phantom.cmd

gwynjudd commented 8 years ago

@dflynn15 I'm looking through the older releases of phantomjs now

gwynjudd commented 8 years ago

I found the oldest phantomjs releases here:

https://code.google.com/archive/p/phantomjs/downloads?page=6

None of the releases seem to include phantom.cmd. It seems like even phantomjs 1.0.0 did not include a phantom.cmd in the zip file. I'm a bit baffled why it was using the .cmd in that case.

dflynn15 commented 8 years ago

This seems to work fine with my setup. I'm going to have a few co-workers test it out with different versions that they have and it should be merged through 👍

dflynn15 commented 8 years ago

This got the 👍 . I'm going to get some of the other PR's in before a version bump! Thanks for your help 😄

gwynjudd commented 8 years ago

@dflynn15 are you going to merge the "phantomjs.cmd" bit as well?

dflynn15 commented 8 years ago

You'll notice that I merged the PR as it stood. I couldn't replicate the issue that previously required it to be there. If it comes up again it'll be an easy rollback.

mnmercer commented 8 years ago

Confirmed that phantomjs.cmd is required to run on a Windows 10(x64) with Phantom v2.1.1, Gulp v3.9.1, Node v4.4.4, npm v3.9.0, and the latest version of this plugin from GitHub.

gwynjudd commented 8 years ago

@mnr1 I have almost that exact same setup here, and I do not have phantomjs.cmd anywhere. What is the content of phantomjs.cmd? Did it come when you installed phantom?

mnmercer commented 8 years ago

I installed Phantom using the latest version of the phantomjs-prebuilt npm module. When installed, it seems to add both the phantomjs and phantomjs.cmd executables to my node_modules/.bin/.

gwynjudd commented 8 years ago

Oh I installed it by downloading the binaries from http://phantomjs.org/ - it didn't come with phantomjs.cmd.

gwynjudd commented 8 years ago

Maybe we could fix gulp-phantom-js by having it use the phantomjs-prebuilt modules instead

mnmercer commented 8 years ago

Were you able to run your tests from your Windows machine without the .cmd file? What do you mean by gulp-phantom-js?

gwynjudd commented 8 years ago

Sorry I meant gulp-jasmine-phantom. Yes, the tests run without the .cmd. I'm going to try adding phantomjs-prebuilt and see if it works