nathanboktae / mocha-phantomjs

:coffee: :ghost: Run client-side mocha tests in the command line through phantomjs
MIT License
954 stars 112 forks source link

Bump phantomjs package to latest patch version #231

Closed astorije closed 8 years ago

astorije commented 8 years ago

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 https://github.com/nathanboktae/mocha-phantomjs/issues/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 commented 8 years ago

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 commented 8 years ago

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

nathanboktae commented 8 years ago

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 commented 8 years ago

Looks like bitbucket is back.

astorije commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

nathanboktae commented 8 years ago

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 #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 commented 8 years ago

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

astorije commented 8 years ago

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 commented 8 years ago

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 commented 7 years ago

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