Breaking change from version 2.4.2 to 2.4.3

[justinmchase]

We are suddenly experiencing hangs in our test run when updating from mocha 2.4.2 to 2.4.3.

We are using mochify to run our tests and I believe what it does is run your test file through browserify then runs the test with mocha in phantom.

One of your recent changes:
a4345ef

Appears to be setting the variable require onto the global object, thus breaking mochify and causing it to hang.
https://github.com/mochajs/mocha/blob/master/mocha.js#L1

I'm not sure what the solution is. But manually reverting our local version back to 2.4.2 works around the issue.

[boneskull]

No idea. Browserify thing. If you can dig up more info it'd be appreciated

[KidkArolis]

It also breaks any sort of requirejs usage such as karma-requirejs.

[boneskull]

Ok. I think I know what is wrong. Need to use browser field in package.json instead I believe. Can someone test this?

[boneskull]

I don't think I want do to another release after this fix without proper sanity checks for the browser in place. This is too frustrating.

[justinmchase]

Do you have a branch? I can pull the branch and link it locally to verify.

[tschaub]

We've also got hanging tests (see https://travis-ci.org/openlayers/ol3/builds/105218073) using mocha-phantomjs-core@1.3.0 and mocha@2.4.3.

[benbayard]

I am also experiencing problems using Webpack w/ Karma and Babel. I have rolled back and am no longer experiencing this issue.

[boneskull]

v2.4.4 released. sorry about this. please let me know if it worked

[emord]

@boneskull I'm experiencing the same issue with 2.4.4 https://travis-ci.org/dimagi/Vellum/builds/105259400

[peol]

@emord: I just tried 2.4.4 in our require+mocha setup and it seems to fix the issue for us.

[tschaub]

Perhaps a different issue, but tests still hang with 2.4.4 here: https://travis-ci.org/openlayers/ol3/builds/105278898

[boneskull]

@tschaub @emord I don't know what mocha-phantomjs or mocha-phantomjs-core does. Without more information I can't be of much help. I encourage you to open ticket(s) in those projects to do a bit of digging.

[mantoni]

Note for Mochify users: I've temporarily nailed the Mocha version to ~2.3.4 in mantoni/mocaccino.js@a9dae9c which fixes the issue after re-installing. Will revert once the underlying issue is solved or we know how to change Mochify to make it work again with 2.4.x.

[boneskull]

Ok. I think chalk needs to be included in the bundle. To support ie8 we will have to shim some things in chalk. Since we don't have any tears running on phantom right now I'll probably want @mantoni to confirm the fix is good for mochify. I can test in ie8. I'll see about forking @tschaub's repo and running a test against the fix on Travis. Tomorrow.

[boneskull]

s/tears/tests but may as well be tears at this point 😢

[tschaub]

@boneskull I'm happy to test with an update. I narrowed it down to a mocha.useColors() call. Without this, tests run normally.

[boneskull]

update: having a tough time shimming for IE8. if anyone knows a tried-and-true way to do it with browserify, please let me know.

[boneskull]

OK. we can't shim chalk because it uses Object.defineProperties() which is unshimmable.

I don't want to drop IE8 support (in a minor version), so I'm reverting 1192914. We can probably leverage chalk's dependencies for #1200, but we cannot use chalk itself.

Sorry about this everyone. I didn't expect something like changing the colors would prove to be such a headache.

[justinmchase]

It's ok, we had a workaround, 👍 to everyone for helping.

[boneskull]

Part of the problem is we just don't have the coverage for all expected situations in which Mocha runs. It's something we need to address (see #2079).

Anyone who wants to test a fix, please do this:

$ cd your-project
$ npm install boneskull/mocha
$ npm test # or whatever

In the meantime, I'll fork @tschaub's project and try it myself.

[boneskull]

@tschaub this build is looking ok to me. please confirm

[rodoabad]

Thanks, @boneskull.

[boneskull]

@mantoni It's not too late for you to be awake right now, but anyway this build looks good; you can ignore the Node.js v0.10 error which doesn't support the github url.

[boneskull]

I'm ready to merge and release this, but I will need some confirmation from someone whose project is affected before I do that.

[boneskull]

Summary of changes

[danielstjules]

#1200 (comment) If chalk's use of defineProperties is the issue, we should be able to work around it?

[justinmchase]

I cloned this repo and from the master branch I did

$ npm install
$ npm test

After that passed I removed the work around from my project and did:

15:09:26:justin:~/code/snap (master)$ npm test

> @trackif/snap@3.5.0 test /Users/justin/code/snap
> jshint . && jscs . && mochify --recursive

# phantomjs:


^C

The hang was reproduced.

Then:

15:09:43:justin:~/code/snap (master)$ npm link mocha
/Users/justin/code/snap/node_modules/mocha -> /Users/justin/.nvm/versions/node/v5.2.0/lib/node_modules/mocha -> /Users/justin/code/mocha
15:09:48:justin:~/code/snap (master)$ npm test

> @trackif/snap@3.5.0 test /Users/justin/code/snap
> jshint . && jscs . && mochify --recursive

# phantomjs:


  .....................................

  104 passing (99ms)

Fix verified here.

[tschaub]

I confirmed that boneskull@51c8ae4 fixes the issues we were seeing as well. Thanks for your work tracking this down @boneskull.

[mantoni]

I used the same technique as @justinmchase and I can confirm that it's fixing the issue. I get colors again and no hanging anymore 🎉 I've tested Phantom, and WebDriver on SauceLabs with various browsers.

Thanks a ton for the fast response time @boneskull.

[boneskull]

np guys. It was my fault. waiting on CI

[boneskull]

v2.4.5 released

[marcjansen]

Awesome collaboration, everyone 👏👏