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

Turn reporters into seperate module #492

Closed
Raynos opened this issue Jul 3, 2012 · 28 comments
Closed

Turn reporters into seperate module #492

Raynos opened this issue Jul 3, 2012 · 28 comments

Comments

@Raynos
Copy link
Contributor

Raynos commented Jul 3, 2012

The mocha reporters are sweet. However they are hard coupled to the mocha interface.

It would be elegant if they were a seperate module.

Consider mocha just outputting TAP format for the tests to stdout

And then doing

mocha test | mocha-reporters --dot

The mocha-reporters module could just be a thing that takes TAP input and outputs to the reporter to stdout.

You could also consider using it programatically as

mocha.output.pipe(mochaReporter.spec).pipe(process.stdout)

This allows people to re-use the reporters in any test framework that outputs TAP.

If you think this is a sensible idea we can draft out how the refactoring would go and I can try migrating the reporters into seperate modules. I've pretty much ported the HTML reporter to something that consumes TAP already so that feels like duplicated effort.

@tj
Copy link
Contributor

tj commented Jul 3, 2012

+1 from me but I'd like to retain the convenience that mocha already provides for backwards compat

@Raynos
Copy link
Contributor Author

Raynos commented Jul 3, 2012

Yes mocha --spec SPEC would simply pipe the TAP output into the correct reporter.

@ForbesLindesay
Copy link
Contributor

Meaning mocha-reporters would be a dependency of mocha and used whenever spec is used.

@tj
Copy link
Contributor

tj commented Aug 9, 2012

I would vote against TAP personally, and just use JSON, but either way this would be a nice feature because not only can we decouple but it makes outputting multiple reports (cov, markdown etc) in parallel really simple since you can just tee to a few pipes

@logicalparadox
Copy link

+1 for JSON format as it is easier to consume.

@ForbesLindesay
Copy link
Contributor

+1 for JSON, if you want tap you can just do mocha --spec TAP which would load the mocha-tap reporter. This could work a bit like consolidate where you have to install the relevant reporter in order to use it, but can then use it simply as mocha --spec REPORTER

@tj
Copy link
Contributor

tj commented Aug 10, 2012

another nice thing with this is that we could easily combine output. Right now for example in Express I have two make targets, test-unit and test-acceptance, and they output separate reports. That's not the end of the world but it would be nice to pipe these together and funnel into a single report

@hunterloftis
Copy link

👍 for me, running dozens of tests in separate processes, would love consolidated reporting for them.

@ForbesLindesay
Copy link
Contributor

I just wanted to come back to this. At the moment, our events aren't JSON serializable, I'd like to cut down what we report to be JSON serializable, that way I wont need to use the insane ForbesLindesay/jssn I've created to serialize them. At the moment you need a serializer that can handle circular references and maintain the prototype chain. Getting that to work in IE6 was a P.I.T.A.

@Raynos
Copy link
Contributor Author

Raynos commented Sep 8, 2013

@visionmedia you said your interested in this change.

If I were to do I would be tempted to have the intermediate format be TAP. so you could do mocha --tap | reporter-dot with TAP on stdout & TAP on stdin

Obviously there would be a JS api and mocha --dot needs to work.

The only breaking change would be "how to write custom reporters" and maybe default mocha to TAP.

Alternatively we can have a json like TAP interface on stdout & stdin.

If I work on this i'll probably not do all reporters at the same time but do them a bunch at the time and get the ball rolling earlier.

@ForbesLindesay
Copy link
Contributor

The problem with TAP is that it encodes a fraction of the information that mocha provides and that mocha-reporters consume. It wouldn't be that much of a stretch to envision a really nice format for structured test logs as newline delimited JSON. That would be much easier to extend with all the additional information.

@tj
Copy link
Contributor

tj commented Sep 11, 2013

yeah that's the thing you'd have to augment TAP, making it super ugly and not very useful to TAP consumers that aren't familiar with the format. I'd love the change but I don't think we should use TAP. It would just end up being json-infused TAP haha

@ForbesLindesay
Copy link
Contributor

You could support TAP by essentially doing TAP + JSON comments. I don't think there would be any real point though as the reporters wouldn't be TAP compatible

@tj
Copy link
Contributor

tj commented Sep 11, 2013

plus TAP isn't really all that popular, node is about it, most CIs don't consume TAP, so we're not losing much, they all have their own crazy-ass weird format. It's too bad we don't have Test-Anything-JSON because then you could augment it at will without making the test labels super ugly

@Raynos
Copy link
Contributor Author

Raynos commented Sep 11, 2013

If anyone has a suggestion for the new line delimited JSON format I"ll use that.

Otherwise I'll use whatever mocha does. I'll probably get the reporters to understand TAP as well since that will make them work with tape for my use case

@ForbesLindesay
Copy link
Contributor

Only a fraction of the reporters can be made to understand TAP in any meaningful way.

As for a format, I started formalizing some thoughts in https://github.com/test-everything/spec/blob/master/stream.md but they're pretty rubbish at the moment. I think we should specify something based loosely on mocha, but mocha's internal model (i.e. the event emitter) not the current JSON reporter (which doesn't report nearly enough information).

Things I'd like to see from a format:

  • It should be possible to rebuild the test report from a stored copy of the JSON log, complete with timing info, so every line of the JSON log should include the full timestamp of when that line was written (accurate to the ms)
  • The reporters should treat pretty much all the fields of each JSON line as optional, and ignore any they don't recognize. That way a simplistic test harness could just emit a tiny subset of the full information.
  • All the reporters should be able to handle lines that are not valid JSON documents gracefully.
  • TAP support should be created using two transformers (to-tap and from-tap) we shouldn't make it a first class citizen built into our tools. As @visionmedia says, it's really not that popular or useful.

I'd love to collaborate with people on designing such a standard.

@logicalparadox
Copy link

One of the things that I would like to see is the inclusion of meta data for any failed test, not just the failure message/stack. Chai's assertion-error constructor already includes a .toJSON() method that allows for any user/plugin provided data (such as actual and expected) to be neatly packed for wire transfer.

Side Note: I have also started a discussion about what might be included in future assertion-error enhancements.

@Raynos
Copy link
Contributor Author

Raynos commented Jan 30, 2014

So i've come around to the idea that TAP is a terrible format and you need some kind of JSON object format.

👍

@ForbesLindesay
Copy link
Contributor

The other thing I would add to my list of requirements for reporting is that success or failure should be detected/indicated purely through the use of the exit code. i.e. it should be possible to have a failing test, but overall state that tests pass, and visa versa. All CIs should depend purely on the exit code.

@rlidwka
Copy link

rlidwka commented Feb 1, 2014

So i've come around to the idea that TAP is a terrible format and you need some kind of JSON object format.

TAP is a human-readable format, JSON isn't. It's a good reason to use TAP.

@tj
Copy link
Contributor

tj commented Feb 3, 2014

it doesn't need to be readable, it just needs to be an intermediate data format, which JSON is very good at, no reason to use TAP for this

@rlidwka
Copy link

rlidwka commented Feb 3, 2014

Well okay, I'm mostly concerned about interoperability with node-tap, since I'm trying to maintain npm fork and stuck with its tests for the time being (at least output can be changed...).

Anyway, what information does mocha provide that isn't supported by tap? Pending tests? Duration? Hmm... maybe you're right. starting to read TAP spec

@rlidwka
Copy link

rlidwka commented Feb 6, 2014

I just noticed that jenkins uses tap too, see node.js tests.

I'd say it's pretty much a standard thing already.

@hallas
Copy link

hallas commented Feb 6, 2014

When in Node space, using JSON is the sane thing to do.

@ForbesLindesay
Copy link
Contributor

Yes, tap is a standard. Yes, it's widely used. Yes, it has a fair amount of usage in node.

No, it doesn't contain enough information for any of our reporters. No, not many systems are actually consuming TAP and doing anything useful with the fact that they can see how many tests passed/failed.

@jb55
Copy link

jb55 commented Jun 12, 2014

I needed something like this for mocha-broken so I put together a quick prototype

I'm using json-stream as a quick intermediate format until I put together a more comprehensive one that supports suites and fail errors. Actually if we update json-stream with suites and errors that would be ideal.

@jb55
Copy link

jb55 commented Jun 12, 2014

Hmm it may have been easier to adapt mocha-broken to use tap and then use a tap reporter.

Thanks!

I assume we can close this now that there are sufficient alternatives?

On Wed, Jun 11, 2014 at 7:05 PM, Raynos (Jake Verbaten)
notifications@github.com wrote:

btw there is:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants