mantoni / mochify.js

☕️ TDD with Browserify, Mocha, Headless Chrome and WebDriver
MIT License
346 stars 57 forks source link

Update to Mocha 4 #160

Closed fatso83 closed 7 years ago

fatso83 commented 7 years ago

Updates the Growl dependency that had a vulnerability See

fatso83 commented 7 years ago

Not sure if it would have been preferable to move over Mocha to be a peer dependency, as it would probably be installed alongside Mocha in most cases, and that would avoid cases of bithound failing deep hierarchies. On the other hand: more cumbersome, probably, as "If Mocha should be a peer dependency, then why not x,y,z?".

mantoni commented 7 years ago

Thank you! Upgrading mocha requires the corresponding change in mocaccino first:

https://github.com/mantoni/mocaccino.js/blob/master/package.json

Once that is release as a major it can be changes here for consistency.

It shouldn’t really be a dev dependency because you can use mochify as a replacement for mocha in a browser-only project. Also, that wouldn‘t help with upgrading major releases. We still need some level of control to make sure the API is compatible.

I’m not able to do this until later tonight. I‘ll give you push access to both repositories and npm.

fatso83 commented 7 years ago

@mantoni, I didn't mean they should be dev dependencies, but peer dependencies, so that they could all use the same version. You could still specify a version range in each project for the API compatibility, like we do in sinon-test, a la "mocha": "3.4 - 4.0"

But hey, not important, I'll send a PR for moccacino as well :)

mantoni commented 7 years ago

Like I explained, I'm using this project as the only test dependency without explicitly specifying mocha. Would a peer-dependency install the newest specified mocha version then?

Previous major releases came with some API changes anyway, so I guess this is the first time it would actually work to depend on a range. But let's not make this more complicated than it needs to be :smile:

Care to also update the compatibility list in the readme?

fatso83 commented 7 years ago

README and Mocaccino updated. Pushing new version to NPM once this is merged.

mantoni commented 7 years ago

Awesome! 🎉 Thanks a lot.