Support for multiple reporters #930

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
@misterdjules

Hi!

This pull request adds support for multiple reporters.

One of my use cases for multiple reporters is described in my recent post on mocha's google group. To sum it up, I'm using mocha to run tests on a Bamboo instance, and I use the xunit-file reporter to create a file that can be parsed by Bamboo to display tests results. However, I also need to send tests reports to a CouchDB instance that doesn't accept xml files, but JSON documents. Right now, I'm running the tests twice: first with the xunit-file reporter, and then with the mocha-json-file-reporter to generate the xunit file and the json document. This has a number of cons like slowing down the whole process, and even sometimes not generating the exact same results, which can lead to a lot of confusion.

These changes allow me to run both reporters at the same time, and I think being able to run more than one reporter at a time can be useful to others. All the tests seem to run fine.

Julien Gilli added some commits Jul 19, 2013

Julien Gilli
- Mocha.prototype.reporter is now Mocha.prototype.reporters and can t…
…ake the same types of arguments

as before, plus a list of reporter names as a comma separated string, or a list of constructors.
This allows using more than one reporter.
- Replaced command line switch --reporter <name> by --reporters <name>,... so that users
can pass a comma separated list of reporters on the command line.
- The old --reporters command line switch used to list available reporters is now --list-reporters.
Julien Gilli
- Revert the change of command line interface, --reporters is back to…
… --reporter and --list-reporters

is back to --reporters.
- Use the "list" arguments parser for --reporter now.
- Mocha.reporters does not accept a comma-separated string of reporter names anymore, this is the command line parser's job
to parse the --reporter command line switch and provide an array to this function.
@park9140

This comment has been minimized.

Show comment
Hide comment
@park9140

park9140 Jul 31, 2013

Contributor

Bump,

This would be especially useful for the karma js mocha adapter. Currently it reports just results back to karma. I would like the debug view to show tests in progress using the test reporter, and this would allow that fairly simply.

Contributor

park9140 commented Jul 31, 2013

Bump,

This would be especially useful for the karma js mocha adapter. Currently it reports just results back to karma. I would like the debug view to show tests in progress using the test reporter, and this would allow that fairly simply.

- if (options.growl) this._growl(runner, reporter);
+ // Use only the first reporter for growl, since we don't want to
+ // send several notifications for the same tests suite
+ if (options.growl) this._growl(runner, this._reportersInstances[0]);

This comment has been minimized.

@park9140

park9140 Aug 1, 2013

Contributor

Would probably make sense to have growl as a reporter instead if we have multiple reporter support here.

@park9140

park9140 Aug 1, 2013

Contributor

Would probably make sense to have growl as a reporter instead if we have multiple reporter support here.

This comment has been minimized.

@misterdjules

misterdjules Aug 3, 2013

Excellent idea, thank you! I'll investigate that next week.

@misterdjules

misterdjules Aug 3, 2013

Excellent idea, thank you! I'll investigate that next week.

This comment has been minimized.

@misterdjules

misterdjules Sep 17, 2013

@park9140 Sorry I haven't been able to find the time to work on this. Any update on your side about this?

@misterdjules

misterdjules Sep 17, 2013

@park9140 Sorry I haven't been able to find the time to work on this. Any update on your side about this?

This comment has been minimized.

@park9140

park9140 Oct 15, 2013

Contributor

Haven't had a chance to look at it yet. If we could pull the multiple reporter thing in it would be a simple update.

@park9140

park9140 Oct 15, 2013

Contributor

Haven't had a chance to look at it yet. If we could pull the multiple reporter thing in it would be a simple update.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 9, 2013

Looks like these changes broke old CLI usage: --reporter option won't work anymore

ghost commented Aug 9, 2013

Looks like these changes broke old CLI usage: --reporter option won't work anymore

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Aug 9, 2013

@deadproger Did you see this commit: 5f3cbe9 ? If yes, what makes you think it would break --reporter?

@deadproger Did you see this commit: 5f3cbe9 ? If yes, what makes you think it would break --reporter?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 12, 2013

Sorry, I haven't seen :(

ghost commented Aug 12, 2013

Sorry, I haven't seen :(

@renehamburger

This comment has been minimized.

Show comment
Hide comment

+1 for this

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Sep 17, 2013

Contributor

+1 from me but this should be done after the other issue to place all streams ontop of a streaming json thing, so they all just become consumers, then you could tee to multiple no problem

Contributor

tj commented Sep 17, 2013

+1 from me but this should be done after the other issue to place all streams ontop of a streaming json thing, so they all just become consumers, then you could tee to multiple no problem

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Sep 17, 2013

@visionmedia Thank you very much for your feedback! Any link to the issue you mentioned about "placing all streams on top of a streaming json thing" would be much appreciated. I searched for it on GitHub but couldn't find it.

@visionmedia Thank you very much for your feedback! Any link to the issue you mentioned about "placing all streams on top of a streaming json thing" would be much appreciated. I searched for it on GitHub but couldn't find it.

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Oct 6, 2013

@visionmedia Did you see my previous question? I'd really like to get this feature in.

@visionmedia Did you see my previous question? I'd really like to get this feature in.

@travisjeffery

This comment has been minimized.

Show comment
Hide comment
@travisjeffery

travisjeffery Oct 16, 2013

Contributor

hmm likely either #492 or #897

Contributor

travisjeffery commented Oct 16, 2013

hmm likely either #492 or #897

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Oct 16, 2013

@travisjeffery Thank you for the pointer, I had somehow missed it.

@travisjeffery Thank you for the pointer, I had somehow missed it.

@Quazie

This comment has been minimized.

Show comment
Hide comment
@Quazie

Quazie Nov 6, 2013

+1 This would be a big bonus for me as well.

Quazie commented Nov 6, 2013

+1 This would be a big bonus for me as well.

@Quazie

This comment has been minimized.

Show comment
Hide comment
@Quazie

Quazie Nov 25, 2013

I am using a custom version of Mocha locally that includes this change. Thanks a ton @misterdjules - I hope this gets merged in soon.

Quazie commented Nov 25, 2013

I am using a custom version of Mocha locally that includes this change. Thanks a ton @misterdjules - I hope this gets merged in soon.

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Nov 25, 2013

@Quazie Thank you, I'm glad it helps! I'd love to get this feature merged too. As @travisjeffery pointed out, it seems that we need to resolve at least #492 or #897 before being able to work on merging this one. @visionmedia Could you please confirm which issue(s) need(s) to be fixed before being able to merge this one? Also, it would be nice to get an idea of what needs to be done on these dependent issues so that we can make some progress towards merging this pull request.

@Quazie Thank you, I'm glad it helps! I'd love to get this feature merged too. As @travisjeffery pointed out, it seems that we need to resolve at least #492 or #897 before being able to work on merging this one. @visionmedia Could you please confirm which issue(s) need(s) to be fixed before being able to merge this one? Also, it would be nice to get an idea of what needs to be done on these dependent issues so that we can make some progress towards merging this pull request.

@jamescarr

This comment has been minimized.

Show comment
Hide comment
@jamescarr

jamescarr Nov 27, 2013

Contributor

Could definitely use this as well. In my case I want to capture code coverage and xunit.

Contributor

jamescarr commented Nov 27, 2013

Could definitely use this as well. In my case I want to capture code coverage and xunit.

@glenjamin

This comment has been minimized.

Show comment
Hide comment
@glenjamin

glenjamin Dec 6, 2013

Contributor

I've put together a third-party reporter to try and achieve this. I'd appreciate any feedback from people on if this actually works reasonably well.

It's a massive hack.

https://github.com/glenjamin/mocha-multi

Contributor

glenjamin commented Dec 6, 2013

I've put together a third-party reporter to try and achieve this. I'd appreciate any feedback from people on if this actually works reasonably well.

It's a massive hack.

https://github.com/glenjamin/mocha-multi

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Dec 6, 2013

@glenjamin I'd like to find some time to try it in the near future, I'll let you know If I do. Thanks!

@glenjamin I'd like to find some time to try it in the near future, I'll let you know If I do. Thanks!

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Dec 8, 2013

@glenjamin It works well, except that the process does not exit, even if I use only the "dot" reporter. The fact that it's an external solution (using a reporter) is definitely very interesting, but the code sure looks like it's doing a lot of hacky stuff :)

Let me know if I can help you diagnose why it's not exiting properly.

@glenjamin It works well, except that the process does not exit, even if I use only the "dot" reporter. The fact that it's an external solution (using a reporter) is definitely very interesting, but the code sure looks like it's doing a lot of hacky stuff :)

Let me know if I can help you diagnose why it's not exiting properly.

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Dec 8, 2013

I created a Google document to try to sum up what's going on with this multiple reporters feature. I think it's a good way to keep track of the various efforts going, their dependencies and what needs to be done to ship this. Any help would be greatly appreciated.

I created a Google document to try to sum up what's going on with this multiple reporters feature. I think it's a good way to keep track of the various efforts going, their dependencies and what needs to be done to ship this. Any help would be greatly appreciated.

@glenjamin

This comment has been minimized.

Show comment
Hide comment
@glenjamin

glenjamin Dec 8, 2013

Contributor

@misterdjules if you could raise an issue on mocha-multi itself with platform versions and steps to reproduce that'd be great.

I'm using a hack similar to --no-exit to ensure stream writing is complete, so if you've got anything keeping the event loop going then that would keep your process running.

Contributor

glenjamin commented Dec 8, 2013

@misterdjules if you could raise an issue on mocha-multi itself with platform versions and steps to reproduce that'd be great.

I'm using a hack similar to --no-exit to ensure stream writing is complete, so if you've got anything keeping the event loop going then that would keep your process running.

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Dec 9, 2013

@glenjamin Turns out that fs.watch causes mocha to hangs when using mocha-multi. More info in this issue

@glenjamin Turns out that fs.watch causes mocha to hangs when using mocha-multi. More info in this issue

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Dec 11, 2013

Just to let you know that @glenjamin's mocha-multi works very well, and it works right now. You might have to wait until he publishes the latest version to npm though, otherwise take it directly from master.

Just to let you know that @glenjamin's mocha-multi works very well, and it works right now. You might have to wait until he publishes the latest version to npm though, otherwise take it directly from master.

@enricostano

This comment has been minimized.

Show comment
Hide comment

👍 :shipit:

@Quazie Quazie referenced this pull request in StevenLooman/mocha-lcov-reporter Jan 21, 2014

Open

Mocha-lcov now has file pushing capabilites #3

@jamesplease

This comment has been minimized.

Show comment
Hide comment

👍

@janpaul123

This comment has been minimized.

Show comment
Hide comment

👍

@giggio

This comment has been minimized.

Show comment
Hide comment
@giggio

giggio Apr 10, 2014

Contributor

👍

Contributor

giggio commented Apr 10, 2014

👍

@jamlen

This comment has been minimized.

Show comment
Hide comment
@jamlen

jamlen Apr 11, 2014

👍 from me, this would be really useful. I need/want to use the markdown reporter for documentation but this makes it unreadable on the console when running tests locally. Being able to specify a reporter and the output file would be great. I guess then if you specified a reporter with no output file it would go to stdout.

jamlen commented Apr 11, 2014

👍 from me, this would be really useful. I need/want to use the markdown reporter for documentation but this makes it unreadable on the console when running tests locally. Being able to specify a reporter and the output file would be great. I guess then if you specified a reporter with no output file it would go to stdout.

@rubiii

This comment has been minimized.

Show comment
Hide comment

rubiii commented Apr 18, 2014

:shipit: please

@Aaronius

This comment has been minimized.

Show comment
Hide comment

Aaronius commented May 1, 2014

+1

@lsnyder

This comment has been minimized.

Show comment
Hide comment

lsnyder commented May 9, 2014

+1

@ninelb

This comment has been minimized.

Show comment
Hide comment

ninelb commented May 28, 2014

+1

@seanmonstar

This comment has been minimized.

Show comment
Hide comment
@seanmonstar

seanmonstar Jun 3, 2014

So whats the hold up with this one?

So whats the hold up with this one?

@glenjamin

This comment has been minimized.

Show comment
Hide comment
@glenjamin

glenjamin Jun 3, 2014

Contributor

It would be a major change to internal workings, and it's also unclear exactly how multiple reporters would be specified on the command line

Contributor

glenjamin commented Jun 3, 2014

It would be a major change to internal workings, and it's also unclear exactly how multiple reporters would be specified on the command line

@pmoriarty

This comment has been minimized.

Show comment
Hide comment

+1

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Sep 10, 2014

Sorry for not posting any update to this issue earlier. I still haven't had the time to work on this. I'd very much like to though, and I'll try to find some time in the near future. In the meantime, if anyone wants to pick this up, don't let me hold you back.

Sorry for not posting any update to this issue earlier. I still haven't had the time to work on this. I'd very much like to though, and I'll try to find some time in the near future. In the meantime, if anyone wants to pick this up, don't let me hold you back.

@benvinegar

This comment has been minimized.

Show comment
Hide comment
@benvinegar

benvinegar Sep 12, 2014

Contributor

@misterdjules – this is a huge pain point for us right now, so I'd be glad to pick this up.

Contributor

benvinegar commented Sep 12, 2014

@misterdjules – this is a huge pain point for us right now, so I'd be glad to pick this up.

@boneskull

This comment has been minimized.

Show comment
Hide comment
@boneskull

boneskull Sep 13, 2014

Member

👍 This would be huge

Member

boneskull commented Sep 13, 2014

👍 This would be huge

@jbnicolai

This comment has been minimized.

Show comment
Hide comment
@jbnicolai

jbnicolai Sep 13, 2014

Contributor

+1 from me as well

Contributor

jbnicolai commented Sep 13, 2014

+1 from me as well

@misterdjules

This comment has been minimized.

Show comment
Hide comment
@misterdjules

misterdjules Sep 15, 2014

@benvinegar Please go ahead and let us know how things go. I agree with your comment about not being able to demultiplex reporters' output. It's definitely not clean, and I think we should come up with a solution to this problem before merging it.

FWIW, I have used https://github.com/glenjamin/mocha-multi for a while, and the syntax used to specify reporters' output destination works well for me.

@benvinegar Please go ahead and let us know how things go. I agree with your comment about not being able to demultiplex reporters' output. It's definitely not clean, and I think we should come up with a solution to this problem before merging it.

FWIW, I have used https://github.com/glenjamin/mocha-multi for a while, and the syntax used to specify reporters' output destination works well for me.

@callicles

This comment has been minimized.

Show comment
Hide comment

+1

@dasilvacontin

This comment has been minimized.

Show comment
Hide comment
Member

dasilvacontin commented Oct 27, 2014

+1

@theogravity

This comment has been minimized.

Show comment
Hide comment
@theogravity

theogravity Dec 12, 2014

+1, it's the only thing that prevents me from advocating usage of mocha over jasmine.

+1, it's the only thing that prevents me from advocating usage of mocha over jasmine.

@efolio

This comment has been minimized.

Show comment
Hide comment

efolio commented Dec 31, 2014

+1

@cloderic

This comment has been minimized.

Show comment
Hide comment

+1

@park9140

This comment has been minimized.

Show comment
Hide comment
@park9140

park9140 Jan 30, 2015

Contributor

@travisjeffery, @misterdjules, whats the holdup here?

Contributor

park9140 commented Jan 30, 2015

@travisjeffery, @misterdjules, whats the holdup here?

@dasilvacontin

This comment has been minimized.

Show comment
Hide comment
@dasilvacontin

dasilvacontin Feb 1, 2015

Member

@park9140, @benvinegar picked up the PR, but it's still work in progress.

Member

dasilvacontin commented Feb 1, 2015

@park9140, @benvinegar picked up the PR, but it's still work in progress.

@benvinegar

This comment has been minimized.

Show comment
Hide comment
@benvinegar

benvinegar Feb 1, 2015

Contributor

I'm basically looking for guidance on what the command line argument format
is for specifying which reporters output where.
On Jan 31, 2015 5:44 PM, "David da Silva Contín" notifications@github.com
wrote:

@park9140 https://github.com/park9140, @benvinegar
https://github.com/benvinegar picked up the PR, but it's still work in
progress.


Reply to this email directly or view it on GitHub
#930 (comment).

Contributor

benvinegar commented Feb 1, 2015

I'm basically looking for guidance on what the command line argument format
is for specifying which reporters output where.
On Jan 31, 2015 5:44 PM, "David da Silva Contín" notifications@github.com
wrote:

@park9140 https://github.com/park9140, @benvinegar
https://github.com/benvinegar picked up the PR, but it's still work in
progress.


Reply to this email directly or view it on GitHub
#930 (comment).

@dasilvacontin

This comment has been minimized.

Show comment
Hide comment
@dasilvacontin

dasilvacontin Feb 1, 2015

Member

@benvinegar is that the only thing left?

Member

dasilvacontin commented Feb 1, 2015

@benvinegar is that the only thing left?

@benvinegar

This comment has been minimized.

Show comment
Hide comment
@benvinegar

benvinegar Feb 1, 2015

Contributor

@dasilvacontin Pretty much.

Contributor

benvinegar commented Feb 1, 2015

@dasilvacontin Pretty much.

@dasilvacontin

This comment has been minimized.

Show comment
Hide comment
Member

dasilvacontin commented Feb 1, 2015

@benvinegar Cool!

@fearphage

This comment has been minimized.

Show comment
Hide comment
@fearphage

fearphage Feb 9, 2015

--reporter spec:output.log,dot:test.txt

What about that?

--reporter spec:output.log,dot:test.txt

What about that?

@glenjamin

This comment has been minimized.

Show comment
Hide comment
@glenjamin

glenjamin Feb 9, 2015

Contributor

I had also considered
`--reporter spec --add-reporter xunit:result.xml'

add-reporter would append, and reporter would reset. This would allow a default in mocha.opts, and the ability to add additional reporters on demand.

Contributor

glenjamin commented Feb 9, 2015

I had also considered
`--reporter spec --add-reporter xunit:result.xml'

add-reporter would append, and reporter would reset. This would allow a default in mocha.opts, and the ability to add additional reporters on demand.

@feugy feugy referenced this pull request in hapijs/lab Feb 24, 2015

Closed

Support multiple reporters #312

@TakenPilot

This comment has been minimized.

Show comment
Hide comment
@TakenPilot

TakenPilot Mar 27, 2015

Is this still moving forward? It's been a month since last comment.

Is this still moving forward? It's been a month since last comment.

@dasilvacontin

This comment has been minimized.

Show comment
Hide comment
@dasilvacontin

dasilvacontin Mar 27, 2015

Member

@TakenPilot, I believe @benvinegar's PR is waiting for him to work on it. At least we managed to decide a format for the arguments.

Member

dasilvacontin commented Mar 27, 2015

@TakenPilot, I believe @benvinegar's PR is waiting for him to work on it. At least we managed to decide a format for the arguments.

@benvinegar

This comment has been minimized.

Show comment
Hide comment
@benvinegar

benvinegar Mar 27, 2015

Contributor

Sorry everyone, I found it hard to pick up this PR again after rebasing, trying to finagle conflicts, etc. I'm willing to put in one last hurrah this weekend, but if I don't get anywhere, I should probably pass the torch.

Contributor

benvinegar commented Mar 27, 2015

Sorry everyone, I found it hard to pick up this PR again after rebasing, trying to finagle conflicts, etc. I'm willing to put in one last hurrah this weekend, but if I don't get anywhere, I should probably pass the torch.

@dasilvacontin

This comment has been minimized.

Show comment
Hide comment
@dasilvacontin

dasilvacontin Mar 27, 2015

Member

We'll be here to help @benvinegar, you can contact me on Gitter if needed. :)

Member

dasilvacontin commented Mar 27, 2015

We'll be here to help @benvinegar, you can contact me on Gitter if needed. :)

@danielstjules

This comment has been minimized.

Show comment
Hide comment
@danielstjules

danielstjules Mar 31, 2015

Contributor

I'm thinking this can be closed in the meantime, in favor of #1360? @travisjeffery @boneskull

Contributor

danielstjules commented Mar 31, 2015

I'm thinking this can be closed in the meantime, in favor of #1360? @travisjeffery @boneskull

@boneskull

This comment has been minimized.

Show comment
Hide comment
@boneskull

boneskull Mar 31, 2015

Member

indeedy

Member

boneskull commented Mar 31, 2015

indeedy

@boneskull boneskull closed this Mar 31, 2015

@benvinegar benvinegar referenced this pull request May 4, 2015

Closed

Implement support for multiple reporters #1681

1 of 2 tasks complete
@ivankravchenko

This comment has been minimized.

Show comment
Hide comment
@ivankravchenko

ivankravchenko Feb 8, 2016

👍 please! :)

👍 please! :)

@nicolehollyNYT

This comment has been minimized.

Show comment
Hide comment

👍

@rcbop

This comment has been minimized.

Show comment
Hide comment

rcbop commented May 10, 2016

👍

@jan-molak

This comment has been minimized.

Show comment
Hide comment

👍

@rrajkowski

This comment has been minimized.

Show comment
Hide comment
@rrajkowski

rrajkowski Jun 10, 2016

👍 is the merged yet? I checked out mocha-multi but would be nice as part of core Mocha

rrajkowski commented Jun 10, 2016

👍 is the merged yet? I checked out mocha-multi but would be nice as part of core Mocha

@Kadrian

This comment has been minimized.

Show comment
Hide comment
@Kadrian

Kadrian Jul 14, 2016

Does anyone know the status of this PR?

Kadrian commented Jul 14, 2016

Does anyone know the status of this PR?

@corvinrok

This comment has been minimized.

Show comment
Hide comment
@corvinrok

corvinrok Feb 8, 2017

why is this closed? Was this finally merged?

why is this closed? Was this finally merged?

@pmarreck

This comment has been minimized.

Show comment
Hide comment
@pmarreck

pmarreck Mar 1, 2018

@corvinrok If you literally read just 10 comments up or so, you will see that it got moved/merged with #1360

pmarreck commented Mar 1, 2018

@corvinrok If you literally read just 10 comments up or so, you will see that it got moved/merged with #1360

@pmarreck

This comment has been minimized.

Show comment
Hide comment
@pmarreck

pmarreck Mar 1, 2018

... Aaaand then I noticed that that got closed too, without any visible traction, and despite multiple people requesting it and proposing various PR's. sigh.

pmarreck commented Mar 1, 2018

... Aaaand then I noticed that that got closed too, without any visible traction, and despite multiple people requesting it and proposing various PR's. sigh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment