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

Bump phantomjs package to latest patch version #231

Closed
wants to merge 1 commit into from

Conversation

astorije
Copy link
Contributor

@astorije astorije commented Apr 5, 2016

The current version of PhantomJS is having issues with Bitbucket, which serves the archive for the main executable.

Without attempting a major version bump (already discussed and apparently stagnating in #175), this bumps from 1.9.7-15 to 1.9.20 as the goal is to prevent all CI tools to fail on projects using mocha-phantomjs.

The current version of PhantomJS is [having issues](Medium/phantomjs#522) with Bitbucket, which serves the archive for the main executable.

Without attempting a major version bump (already discussed and apparently stagnating in nathanboktae#175), this bumps from 1.9.7-15 to 1.9.20 as the goal is to prevent all CI tools to fail on projects using `mocha-phantomjs`.
@astorije
Copy link
Contributor Author

astorije commented Apr 5, 2016

This is pretty critical on AppVeyor for example as most configurations using a build matrix will fail: https://ci.appveyor.com/project/astorije/chai-immutable/build/65.

@astorije
Copy link
Contributor Author

astorije commented Apr 5, 2016

Same on Travis CI actually: https://travis-ci.org/astorije/chai-immutable/builds/120780880

@nathanboktae
Copy link
Owner

Then you bring back #167 . gonna have to fork 1.9.7 and pull from that....

Or switch to mocha-phantomjs-core ugh distributing the binary is a world of hurt.

@nathanboktae
Copy link
Owner

Looks like bitbucket is back.

@astorije astorije deleted the patch-1 branch April 6, 2016 02:13
@astorije
Copy link
Contributor Author

astorije commented Apr 6, 2016

Meh, too bad we didn't bump though :(
I hope #167 or #175 will find a resolution though. Good luck in this project and thanks for providing it! :-)

@rlamorea
Copy link

Nope, bitbucket isn't back. It is still failing. Rolling back to 1.9.7-15 has caused our builds to start breaking again due to the bitbucket url.

@astorije
Copy link
Contributor Author

I second @rlamorea's comment. @nathanboktae, what can we do about this?

@nathanboktae
Copy link
Owner

nathanboktae commented Apr 19, 2016

There are a few options:

  1. Tell the phantomjs npm package where to get the binary for example at https://github.com/Medium/phantomjs/releases/tag/v1.9.7-14 since 1.9.7-15 isn't on github. It also says it that it detects the global install of phantomjs so if it's there globally with version 1.9.7 then that should work.
  2. If you are ok with Unsafe JavaScript attempt #167 then you can use npm shrinkwrap to forcibly override the dependency to phantomjs@1.9.20
  3. Install phantomjs however you want and use mocha-phantomjs-core directly. In the next major version of that I will move the CLI parsing into it, but for now it has a more difficult CLI (most options go in as a JSON blob) but it's well supported (Gulp and Grunt plugins for it use this).

@nathanboktae nathanboktae mentioned this pull request Apr 25, 2016
@nathanboktae
Copy link
Owner

oh and now 4. Medium/phantomjs#538 gets accepted and we move to that new package. Hopefully :)

@astorije
Copy link
Contributor Author

Oh what a frustrating night...

  1. Setting a custom URL:
    1. We cannot use the GitHub releases URL because Medium/phantomjs has only the 1.9.8 and 2.1.1 versions in their releases (see https://github.com/Medium/phantomjs/releases/tag/v1.9.19). When using PHANTOMJS_CDNURL=https://github.com/Medium/phantomjs/releases/download/v1.9.19, it is tried to be downloaded from https://github.com/Medium/phantomjs/releases/download/v1.9.19/phantomjs-1.9.7-macosx.zip, which doesn't exist...

    2. Alright alright, let's use your link documentation and use the mirror URL (paaaaainfully slow): PHANTOMJS_CDNURL=http://cnpmjs.org/downloads. Cool, it works when running PHANTOMJS_CDNURL=http://cnpmjs.org/downloads npm install. Let's try now to have this set when simply running npm install so that CI and users running npm install. Well, as opposed to their docs, using .npmrc is not an option:

      If the content of my .npmrc is PHANTOMJS_CDNURL=http://cnpmjs.org/downloads and running a REPL loaded with the full env (via a "node": "node" script and running npm run node), I get:

      > process.env.PHANTOMJS_CDNURL
      undefined
      > process.env.npm_config_PHANTOMJS_CDNURL
      'http://cnpmjs.org/downloads'

      Yup, env vars set in .npmrc do not come verbatim, they are prefixed...
      It is confirmed by looking at the env (using a "env": "env" script, run with npm run env):

      [...]
      npm_config_PHANTOMJS_CDNURL=http://cnpmjs.org/downloads
      [...]
      

      (Maybe I entirely misunderstood something with env vars and .npmrc...)

    3. From there, I tried everything I could think of, even a script saying "preinstall": "export PHANTOMJS_CDNURL=http://cnpmjs.org/downloads"... no luck of course...

    4. Using installed version of phantomjs is not an option either: Travis CI comes with 1.9.8 and therefore the package tries to download 1.9.7, sighs...

  2. Using npm shrinkwrap is not an option, at least not right now and not to solve this, as it has other consequences on the project.
  3. seems to be overkill for both the need and the problem. What would be the 1:1 vanilla (i.e. Gulp/Grunt-free) equivalent between having mocha-phantomjs in the dependencies and "test": "mocha-phantomjs test.js", and using mocha-phantomjs-core?
  4. Well, maybe the only short-term solution, but kind of a status quo until they merge it.

So yes, frustrating indeed 😄
I honestly don't know how I can easily and short-term fix this. It's an insignificant issue that blocks things as CI doesn't get run, potentially users cannot npm install, etc.
At this point, I am considering either disabling entirely mocha-phantomjs tests right now (and relying on mocha only) or temporarily forking this project to use phantomjs 1.9.8...
Both solutions are good enough to my needs, at least temporarily, all things considered.

Anyway, not trying to rant, but to give a summary if someone else has the issue and looks for solutions :-)

@nathanboktae
Copy link
Owner

Ugh sorry to hear that @astorije. If they don't accept my PR soon, I'll take action to resolve this.

I'd recommend using your fork and not disabling your tests. #167 is real but mostly annoying, IIRC.

@astorije
Copy link
Contributor Author

FYI if someone had the same issue, I had disabled them in astorije/chai-immutable#56 and just re-enabled in astorije/chai-immutable#77, everything works now. I simply forgot to do it before :)

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.

None yet

3 participants