Merge work done in grunt-simple-mocha to this plugin

[lauripiispanen]

I'm opening this pull request to propose merging work done in grunt-simple-mocha into this plugin. The patches I've created incorporate the functionality to this plugin in the following manner:

1. If the list of files passed contains no .html files, the tests will be run in node.js
2. If the list does contain .html files, PhantomJS will be used instead
3. If the multi-task target is configured with type: unit, the tests will be run in node.js

This pull request was inspired by having to deal with the two plugins sharing task names (see some of the forks of the grunt-simple-mocha plugin). I'm also attempting to contact the author of grunt-simple-mocha to have him collaborate on this plugin instead on the node.js testing part.

[kmiyashiro]

Can you provide an example of the problems you had? Is the purpose to have both server and client side tests in a single task? It seems like you could do that by creating an alias/registertask.

[lauripiispanen]

Can you provide an example of the problems you had?

Both of these plugins register the task "mocha", and hence cannot be used simultaneously:

https://github.com/kmiyashiro/grunt-mocha/blob/master/tasks/mocha.js#L142
https://github.com/yaymukund/grunt-simple-mocha/blob/master/tasks/simple-mocha.js#L14

Conceptually, grunt-simple-mocha seems cleaner, since it's 'just mocha', versus grunt-mocha fires up a browser and a bunch of .html pages. Since grunt is used to develop not only browser-based solutions, it would be logical that choosing mocha as the test runner would not automatically mean switching to browser-based testing.

[yaymukund]

Hello, I'm the author of simple-mocha. One of my main reasons for publishing simple-mocha is that PhantomJS shouldn't be a dependency for people who want to run server-side tests.

I feel like the right solution is for one of us to rename the task. I'd be happy to change it to simplemocha (as at least one fork has already done), but I'm still figuring out the best way to make the change. Maybe we just want to bump the version to 0.2 and add a big note in the README...

[kmiyashiro]

@yaymukund Yeah, I think these tasks are for fundamentally different goals (client vs server tests), it just so happens mocha can be used for both without modification. I'd hate for either of us to change the task name since people will probably wonder why their task broke, but I'm more hesitant to change grunt-mocha's task name ever since yeoman added it as a dependency and there are now thousands of installations.

[lauripiispanen]

Yes, I see this issue having more to do with grunt. One example grunt could do is have some sort of namespacing for npm tasks. Other option could maybe is to have grunt-mocha act as some sort of "base" mocha integration, which could be extended by further plugins - but this would break builds relying on the current functionality, as they would require one more plugin ("grunt-mocha-phantom?").

[kmiyashiro]

The more I think about it, the more I like the idea of just making grunt-simple-mocha a dependency and then including the runner via the module. The only problem is, given the setup of grunt's task modules, we can't just include grunt-simple-mocha as-is. In order to make the task function exportable, it will have to expose a function that returns the task function so we can pass grunt in.

[zappan]

Hi - Just to confirm that the forked version of simple-mocha with its grunt task renamed to simplemocha works fine, we're using both plugins and with that change there are no conflicts.

So, a readme of this project can be updated regarding the naming conflict, as Mukund has renamed its task, so things now can work together.

Cheers

[kmiyashiro]

Cool, updated. I still like the idea of detecting whether there are html files in the files list and the trying to run simplemocha, maybe attempting a child process grunt call to it and echoing the stdout. Not sure what the best method is for calling another grunt task from within a grunt task.

[yaymukund]

If I put the mocha_instance.run() code in a helper, I think you can reuse the helper. But I haven't had a chance to test this yet.

[kmiyashiro]

@yaymukund helpers are going to be removed in grunt 0.4, instead you can just use normal node modules to import functions. Maybe you can wrap your main code in a normal node module and then it can easily be imported by any node module.

[sindresorhus]

👍

@kmiyashiro Can we get this landed?

[kmiyashiro]

I'd have to review it since this was for grunt 0.3 and the config has changed, but there were also questions about whether it makes sense to combine frontend and backend tests in the same task.

[syamanaka]

It's not so much about frontend vs backend tests; rather it's plain Mocha vs Mocha+PhantomJS. We write most client-side tests using Zombie.js and the simplemocha task. When that's not sufficient, we use this task. Looking for html files is okay, but I prefer to have a PhantomJS option for each target.

[sindresorhus]

I agree. Would be nice not to have to task for this. I have multiple modules that work both in the browser and in Node.js and currently I'm required to use two tasks to test it.

[Bartvds]

Note: it could also be worth to look at grunt-mocha-test (compared to grunt-simple-mocha) for node.js testing as it is fully featured with many tests (less 'simple' :)

[kmiyashiro]

I can't help but wonder what demons lie down that path.

[jamesplease]

👍