Implement support for multiple reporters

[santiagoaguiar]

Continuation of #1772, rebased from latest.

Besides merging latest with the code @benvinegar & @misterdjules wrote, this PR also:

• Changes the reporters that only did process.stdout.write to use the Base write method, and to inherit from Base if they didn't. If we don't want to inherit from Base from those reporters, which was the approach from the previous PR, I guess we can pass the stream through the options argument. I didn't touch reporters that used colors/cursors.
• Fixes lint warnings.
• Fixed issues with parsing the reporterOptions when there were multiple reporters.
• Adds a test case to check that the json reporter writes to the file specified by the <reporter>:<outputPath> option.

They are two independent commits in case we don't want to do the inherit from Base change, otherwise I can squash them if necessary.

Comments welcome, more than eager to see this pull through. Thanks!

[santiagoaguiar]

I've been thinking now that I'm a bit more familiar with the mocha codebase.

The current option of passing reporters outputfiles like: --reporter json:output.json,json-cov:output-cov discussed on #1360 (comment) doesn't make a lot of sense. First, some reporters like xunit already support an explicit option to provide the output file (through --reporter-options), which would mean there are now two ways of saying the same thing, and second, for reporters that use colors/cursors or are obviously specific for console (progress) passing an output file does not make any sense.

I agree that if you execute two console specific reporters, it won't look nice ;), but I don't think there's really something we can do in that case, except throw.

Maybe the best approach is to always handle the output file through --reporter-options, and in reporters where it make sense to have an output file (from the built in ones: html-cov, json-cov, json, markdown) consider that case explicitly (add an output option to each one). This removes the need of inheriting from Base, removes the path argument from the Base constructor, and simplifies the pull a bit in general. If for those built ins we want to reuse some code through inheritance, that's a different problem, but we don't put that burden on third-party plugins at least. I say we should code that for each case, even if it will be almost identical, it's just a couple of lines.

Comments?

[davemo]

This would greatly improve our CI builds right now, where we have to do multiple runs of test and test:cover where the only difference between runs is the output format options and reporter. (We generate reports for Cobertura in jenkins, as well as lcov and xunit style).

[santiagoaguiar]

I didn't have much feedback about the PR, but I guess I'll do the things I mentioned on my previous message and provide a new PR in any case.

You might want to try out the branch that includes the change. You need to pass the file output on the ---reporter option for built-in reporters that write to stdout. If your reporters accept --reporter-options, you need to pass them using the syntax for multiple reporters.

Example:

mocha --reporter spec,json-cov:json-cov-output.json,xunit --reporter-options xunit:{output:xunit-output.xml}

[mekdev]

Hey guys, were they any updates to this it looks like there are conflicts but all tests have passed

[dasilvacontin]

Hi all – thanks for the work @santiagoaguiar! We'll try giving some feedback this week.

[santiagoaguiar]

Great! As I mentioned, I think we should remove the --reporter foo:<output> syntax, use --reporter-options output= instead, and add reporter-options support to the reporters where it make sense (ie. not for reporters that are strictly designed for console/interactive).

If you think it's ok, I'll be glad to finish up this PR later today and submit those changes.

Thanks!

[boneskull]

This might be a good place to use subargs

[santiagoaguiar]

I reworked this PR here: #2184.

I removed the --reporter json:<output> option, which simplifies things a little, and makes more sense, and also rebased to master.

I'll leave this one open in case we want to go for that route, but I think the one at #2184 is better.

[santiagoaguiar]

Closing this PR in favor of #2184.