Convert matcher tests to mocha

[mantoni]

I just tried converting the matcher tests to Mocha. It turns out to be straight forward. In cases where the requiresSupportFor is used, it would need if statements around the describe blocks.

Before I walk down any rabbit holes and convert the world, I wanted to get feedback on this. We touched on the idea in previous discussions but never concluded to do this. The motivation is to have the same simple npm scripts as in lolex. We can then also run the tests in real browsers easily.

When you check out this branch, you can run the matcher tests on Node and in PhantomJS like this:

$ node_modules/.bin/mocha test/match-test.js
$ node_modules/.bin/mochify test/match-test.js

This needs some thoughts for tests that do real XHR calls. I also don't know yet how this is going to work with the webworker tests.

What are your thoughts? What is problematic or what do we loose in exchange?
Especially @cjohansen, since Buster is your thing and you know the trade offs best.

[cjohansen]

I'm pretty much all for it. This is how I test stuff at work as well. Really I'm less concerned with what tools are being used and more about it being nice and easy to run and write tests. This approach makes it nice and easy to run and write tests.

[fatso83]

👍

[mantoni]

@jonnyreeves We might have some overlap here. I'm currently going through /test from top to bottom. Your last PRs where around the util stuff. Please let me know if you're currently refactoring any tests so that we don't produce conflicts.

[jonnyreeves]

Happy to pause whilst we move to mocha!

I have a sneaky feeling this will avoid any potential issues with buster placing its own copy of sinon on the window.

[mantoni]

Great. If nothing surprising happens, this is done relatively quick.
That's it for today – I'm going to dream of vim macros now 😴

[mantoni]

Status update:

• All tests converted to mocha
• With the exception of the webworker test, all tests pass on node and in phantomjs
• npm test now runs tests in both environments
• For some reason a few async tests fail in phantomjs when run via an npm script while node_modules/.bin/mochify --recursive test/ is all green
• Tests that where skipped before are still skipped

Outstanding:

• The webworker test needs investigation. I couldn't get it to run in phantomjs 1.9. I didn't try 2.0 yet.
• Code coverage (either mochify-istanbul or on node or both)
• The CI script currently also runs against the packaged version of Sinon.
• Configuring SauceLabs to run tests in real browsers 🎉

[mroderick]

Most excellent

[mantoni]

THE BUILD IS GREEN!

I managed to put a webworker test together and configured the SauceLabs build.

The tests are passing in Node 0.12, 4.3 and 5, PhantomJS 2, IE 10 (with a fix), IE 11 and Firefox. Chrome seems to suffer from the same problem as the Lolex build. I can reproduce this with a local Selenium / chromedriver setup and will look into this another time.

I’ve put everything into npm scripts. The scripts/ci-test.sh is now not used anymore.

Still missing:

• Coverage report, should be piece of 🍰
• Running ./build and testing the bundled js file. The problem here is that the tests are referring Sinon source files directly which causes them to be included in the browserified test files (again). So, I think we actually never really tested the bundle. It was just included in the sources. Any ideas?

From my point of view this is good to merge. The open points can be addressed later.

Thoughts?

[jonnyreeves]

Nice work @mantoni! :)

testing the bundled js file

We would need a new set of tests which only try to pick up from the sinon global (as opposed to importing the sources) - I voiced before that such tests may be better split out from the existing tests by having a clear divide between unit tests (which stub out dependencies) and integration tests (which use the bundled version and only read from the exported sinon global).

[jonnyreeves]

Maybe just add this rule to the test/.eslintrc file?

[mroderick]

We should probably add https://github.com/lo1tuma/eslint-plugin-mocha to the configuration, so that we won't accidentally merge a commit that disables most tests

[mantoni]

@jonnyreeves Yes, I also think that we should have a separate set of integration tests for the bundled file. At least some sort of sanity check that verifies that all the API function are there and don't throw.
@mroderick Good idea! Added eslint-plugin-mocha and configured the test specific stuff in /test/.eslintrc.

[mantoni]

Also, npm run test-coverage now produces the instanbul coverage report for PhantomJS.

[mantoni]

So, on the chrome front, I think we're not the only ones with the issue:
https://bugs.chromium.org/p/chromedriver/issues/detail?id=402

There is also a corresponding issue in Mochify to track the issue on the test runner side.

The good news is that the test suite passes fine in Chrome if you run

$ node_modules/.bin/mochify --consolify test.html
$ open test.html

[fearphage]

This is amazing. @mantoni You're a saint.

Since I see a lot of you referencing the node_modules folder, here's something I use to prevent this.

export PATH="./node_modules/.bin:$PATH"

If you add that to your .bashrc/.zshrc file, you'll never have to type node_modules or npm install -g again.