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

Modify runner and reporters to use JSON serializable results #685

Closed
wants to merge 1 commit into from
Closed

Modify runner and reporters to use JSON serializable results #685

wants to merge 1 commit into from

Conversation

ForbesLindesay
Copy link
Contributor

The first step down the road to turn reporters into separate modules

Extremely useful for writing CI systems as you can serialize the complete state of the world and let people view it in a local command line, the browser etc. using whichever reporter they want. This would make tj/mocha-cloud#5 much simpler as it wouldn't need to be able to serialize circular data structures with prototype chains.

I've specified the resulting API here so you can read through the basics of what's changed in plain English.

@tj
Copy link
Contributor

tj commented Dec 9, 2012

I dont see a ton of immediate value in having CI related stuff utilizing regular reporters, if you're running tests in 8+ browsers whatever the least noisy solution is is the best IMO

@ForbesLindesay
Copy link
Contributor Author

I think they'd be useless as the only reporter. But really usefull as a drildown. For example I don't have easy access to an ie 7.0 browser so if it fails for that browser I want all the info that the browser reporter provides.

@ForbesLindesay
Copy link
Contributor Author

The other thing I think would be really useful is getting output from the more special reporters such as html-coverage and markdown documentation. Markdown documentation as a pre-made website that's always up to date would be a nice thing to be able to link to (and more people might find out that the feature exists to generate it themselves). Code coverage would be nice mostly because it's a pain to set up the build yourself (especially if your on windows), so having it set up for you online would be great. I do realise that getting code-coverage to work the same would require some extra work, but I think this represents the first step.

@ForbesLindesay
Copy link
Contributor Author

It would also helpfully let someone re-use the same reporters with a completely different test framework.

@tj
Copy link
Contributor

tj commented Dec 10, 2012

the only thing I would want to see from a CI is which browsers failed and what the error(s) are, the rest doesn't matter at all really

@ForbesLindesay
Copy link
Contributor Author

I find it helpful to be able to view which tests failed and passed as well as often that narrows down what could've caused the error a lot. I think if people in general were happy with that we'd have far fewer and far simpler reporters.

@ForbesLindesay
Copy link
Contributor Author

Perhaps the ci thing is leaving us with a little bit of a red herring here (I think we use them in quite different ways).

I think it presents a cleaner simpler abstraction to reporters and is advantageous because of that, and I think it would be nice to have the ability to run the reporters as separate command line apps and pipe data from the test runner to the reporters (e.g. For aggregating multiple tests or using non-mocha test frameworks with mocha reporters) and I can't see any way of doing that without simplifying the format to be JSON serializable.

@tj
Copy link
Contributor

tj commented Dec 15, 2012

yeah I dont disagree there, I'd like to do that eventually anyway, but we already have json-stream working just fine, they would just be built on that or really similar

@ForbesLindesay
Copy link
Contributor Author

OK, but JSON stream doesn't output anywhere near enough information to build the other reporters on top of it.

@tj
Copy link
Contributor

tj commented Dec 15, 2012

yeah fair enough, at a glance I dont totally get what this pull-request adds, it seems to be a lot of noise from adding (time whatever that's for and a few additional props in the clean thing lower. I'll try and pull it down soon and have a better look

@ForbesLindesay
Copy link
Contributor Author

The time thing is so that all the timings are available as part of the test results (from the runner) and therefore the timings computed bor the test summary are correct even if the results were stored or processed in one go rather than streamed.

All the other properties are used by at least one reporter at some point.

@jbnicolai
Copy link

@ForbesLindesay cleaning this PR up, as I believe it's no longer of interest. Sorry for the long silence, and feel free to add an explanation if you feel that this is still relevant.

Thanks for the effort 👍 and sorry for the long silence.

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

Successfully merging this pull request may close these issues.

None yet

3 participants