Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for multiple reporters #930

Closed

Conversation

misterdjules
Copy link

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 2 commits July 19, 2013 10:36
…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.
… --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
Copy link
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.

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]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link

ghost commented Aug 9, 2013

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

@misterdjules
Copy link
Author

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

@ghost
Copy link

ghost commented Aug 12, 2013

Sorry, I haven't seen :(

@renehamburger
Copy link

+1 for this

@tj
Copy link
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
Copy link
Author

@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
Copy link
Author

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

@travisjeffery
Copy link
Contributor

hmm likely either #492 or #897

@misterdjules
Copy link
Author

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

@Quazie
Copy link

Quazie commented Nov 6, 2013

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

@Quazie
Copy link

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
Copy link
Author

@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
Copy link
Contributor

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

@glenjamin
Copy link
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

@misterdjules
Copy link
Author

@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
Copy link
Author

@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
Copy link
Author

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
Copy link
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.

@misterdjules
Copy link
Author

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

@misterdjules
Copy link
Author

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
Copy link

👍 :shipit:

@jamesplease
Copy link

👍

@janpaul123
Copy link

👍

@efolio
Copy link

efolio commented Dec 31, 2014

+1

1 similar comment
@cloderic
Copy link

+1

@park9140
Copy link
Contributor

@travisjeffery, @misterdjules, whats the holdup here?

@dasilvacontin
Copy link
Contributor

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

@benvinegar
Copy link
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).

@dasilvacontin
Copy link
Contributor

@benvinegar is that the only thing left?

@benvinegar
Copy link
Contributor

@dasilvacontin Pretty much.

@dasilvacontin
Copy link
Contributor

@benvinegar Cool!

@fearphage
Copy link

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

What about that?

@glenjamin
Copy link
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.

@TakenPilot
Copy link

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

@dasilvacontin
Copy link
Contributor

@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
Copy link
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.

@dasilvacontin
Copy link
Contributor

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

@danielstjules
Copy link
Contributor

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

@boneskull
Copy link
Contributor

indeedy

@ivankravchenko
Copy link

👍 please! :)

@nicolehollyNYT
Copy link

👍

2 similar comments
@rcbop
Copy link

rcbop commented May 10, 2016

👍

@jan-molak
Copy link

👍

@rrajkowski
Copy link

rrajkowski commented Jun 10, 2016

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

@Kadrian
Copy link

Kadrian commented Jul 14, 2016

Does anyone know the status of this PR?

@corvinrok
Copy link

why is this closed? Was this finally merged?

@pmarreck
Copy link

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
Copy link

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
Labels
status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.