WIP: Model tests

[tandrewnichols]

So . . . writing additional tests is AWFUL with the setup we have. I'm trying to write some specs for the player model to test the new methods added in the last refactor, and it makes sense to put those in a separate file. HOWEVER, if I do that and run the tests, I get an error from mongoose saying it can't reload the player schema (because index-spec.js already loads it). I can't put my tests in index because I'm mocking most of the mongoose functionality. I can't move the require-ing of mongoose to a spec-helper for the same reason (one file wants the real thing and one wants a fake one). I thought about destroying the mongoose object at the end of index so it can be reloaded in player-spec.js but we're using jasmine-node which has no afterAll test hook for final cleanup. There's also the fact that the different models have to have mongoose passed in to them. Dependency injection I guess, but it actually makes things more difficult to test (for the reasons above). I'd rather see each model just require mongoose and add it's schema. It's not extra overhead since node caches modules on require.

In sum, I'm proposing the following:

1. Abandon jasmine-node. It is silly. Let's use mocha, which has all the same features, plus beforeAll and afterAll, plus more asynchronous support, plus it's actually made for node.js (first), plus a lot more (see the docs). And if that's not enough, it's shorter to type mocha spec than jasmine-node spec.
2. Make the models require mongoose themselves, register their schemas, and export them. Then in models/index.js we can just accumulate all the models for app.js to require.
3. Separate tests more; add a spec-helper for global, one-time setup; mock more objects (why are we actually saving players and matches in our tests? I guess index-spec.js is more of an end-to-end test, but even so, it seems like we could abstract the actual db calls. The combination of sandboxed-module and jasmine spies (which you can use even if your testing framework is mocha) is pretty handy - see player-spec.js), etc.

I realize testing is not going to be a high priority, since this is sort of a pet project. But we've sort of worked ourselves into a corner where we're actually discouraging testing because it's become so difficult to add new tests.

[jshemas]

+1 for more tests.

When I originally wrote these test, they were for generally testing the routes and returns just to see if the application was "basically" working. These tests weren't model specific. But I like the new player test file. We should also think about writing tests for our controllers. (And move them out of routes) But that should be a different pull request.

As far as the whole jasmine-node vs mocha thing. I mostly use mocha myself. I build the current tests using mocha. In #64 I thought I updated the docs/package so everyone else should be using mocha. But I guess I forgot. Can you update the readme in the PR?

If we start making lots of test files, we should split them up by like end-2-end or integration or by what they are testing. (Like models or routes)

[tandrewnichols]

Do you know if mocha works recursively? If we have nested dirs (e.g. models, routes), will the tests in those get picked up?

At work, we've been organizing tests in the same structure as the actual code, so we've got lib and routes and spec/lib and spec/routes (and even more nesting inside that). But we also don't use mocha... I feel like I read a response from TJ Holowaychuk one time in which he indicated that mocha was NOT recursive.

[tandrewnichols]

So . . . the tests in player.spec.js will currently only run with jasmine-node. I think it's because I'm requiring jasmine-stealth at the beginning because when I log beforeEach when I require jasmine-stealth, it includes something about jasmine, but when I don't, it's more like return suites[0].fn.something which is more mocha-esque. Unfortunately, jasmine-stealth is REALLY useful. Probably right now, we could just not use it, since I've only got one .when() and it's not really necessary, but I can imagine it becoming more and more important as we add more tests.

[jshemas]

I thought mocha works recursively, if it doesn't i'm sure we can make it work that way...

I've never used jasmine-stealth before, but it looks like Sinon.JS. (Which can be ran by mocha) Maybe try switching? If you don't want to or can't switch maybe we can just make a script that runs mocha on the "mocha" tests and jasmine on the "jasmine" tests.