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

junit reporter #349

Closed
wants to merge 16 commits into from
Closed

junit reporter #349

wants to merge 16 commits into from

Conversation

jstockdi
Copy link

I added a junit reporter for integration with Bamboo, Hudson and other CI servers. The code is closely modeled after the xunit reporter.

Please review. If you like what you see, please request changes or merge as is.

Thanks for contributing the library. We hope to get a lot of mileage out of it.

--jon

@@ -1,7 +1,4 @@
coverage.html
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove these other ones?

Copy link
Author

Choose a reason for hiding this comment

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

I didn't intentionally remove the other ones.

@tj
Copy link
Contributor

tj commented Mar 27, 2012

doesn't xunit work with hudson?

@jstockdi
Copy link
Author

jstockdi commented Apr 9, 2012

It looks like it requires a plugin:
http://wiki.hudson-ci.org/display/HUDSON/xUnit+Plugin

I mostly use Bamboo, which is easy to setup with the junit format.

--jon

On Tuesday, March 27, 2012 at 11:03 AM, TJ Holowaychuk wrote:

doesn't xunit work with hudson?


Reply to this email directly or view it on GitHub:
#349 (comment)

}else{
console.log('test.state not known, test not recorded: ' + test);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

style-wise this is all way off the rest of the project, try and copy what the rest is like. For example there's no need to have the clean function in scope since it's not enclosing anything, and things like if (test.failed == true can just be if (test.failed, } else { vs }else{ etc

@wolfeidau
Copy link

Do these patches still need some tidy before the push request will be accepted?

I am chasing something similar, if required I am happy to tidy them up if that is all that is required.

@tj
Copy link
Contributor

tj commented Jun 14, 2012

yeah it's just quite different from the rest of the project's style, i'll comment some more

testing
_mocha.js
.project
.settings
Copy link
Contributor

Choose a reason for hiding this comment

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

these changes need reverting

@wolfeidau
Copy link

Thanks for that!

@tj
Copy link
Contributor

tj commented Jun 15, 2012

good enough :D i'll merge and refactor

@tj
Copy link
Contributor

tj commented Jun 15, 2012

hmm might have to rebase it's not applying to master

@wolfeidau
Copy link

Thanks for trying to get his into mocha will make it much easier to integrate into existing CI servers.

@tj
Copy link
Contributor

tj commented Jun 16, 2012

cleaning things up a bit, can junit output have multiple testsuite tags or what's the best way to describe the hierarchy? or does it just expect a flat list of test cases?

@wolfeidau
Copy link

The xsd for the xml is located at

https://svn.jenkins-ci.org/trunk/hudson/dtkit/dtkit-format/dtkit-junit-model/src/main/resources/com/thalesgroup/dtkit/junit/model/xsd/junit-4.xsd

This illustrates that you can have many testsuite tags wrapped in a testsuites tag.

This thread on stack overflow has some more examples.

http://stackoverflow.com/questions/4922867/junit-xml-format-specification-that-hudson-supports

@tj
Copy link
Contributor

tj commented Jun 16, 2012

hmm not a very stream-friendly format, i'll see what i can do

@wolfeidau
Copy link

Yeah I am just having a look over the ruby gem.

https://github.com/nicksieger/ci_reporter

Not sure if it helps with ideas on how to architect the code.

@tj
Copy link
Contributor

tj commented Jun 16, 2012

something like 703cef4 ?

@tj
Copy link
Contributor

tj commented Jun 16, 2012

if we want to add durations to the testsuite tags then we'll have to do our own looping and stacks which is kinda lame, I like to avoid that when possible since the events take care of that for us

@wolfeidau
Copy link

Yeah it doesn't seem to fit the stream model with the summary being written before the testsuite / testsuites items have been written.

Would it be wiser to write a intermediate flattened file then transform it, only other alternative is to store this in a data structure which is then has the report built from it.

@tj
Copy link
Contributor

tj commented Jun 16, 2012

yeah I guess that explains why Jon was building it at the end. That's too bad

@jstockdi
Copy link
Author

I did start with something like:
703cef4

but ended up moving everything to the end in commit:
https://github.com/kenzanmedia/mocha/commit/36c1379f3f896631ed806847ff52a60204af08cd

@knewter
Copy link

knewter commented Aug 15, 2012

This never got merged? It's super useful to me fwiw, big +1 and thanks @jon077 :)

@rauchg
Copy link
Contributor

rauchg commented Aug 15, 2012

Aside from dot, spec, json, json-stream, nyan, browser and TAP, I think most reporters belong in userland. It should also be the case for this one.

@knewter
Copy link

knewter commented Aug 15, 2012

Alright. I just saw above where @visionmedia said a merge was forthcoming, so I assumed that was still the case. For a userland reporter like this, should it just be released as a standalone reporter npm module? Also, how does it plug in to mocha as an available reporter? Is this a built-in API? I'm new to mocha...but I love me some jenkins graphs :)

@rauchg
Copy link
Contributor

rauchg commented Aug 15, 2012

That's just my opinon, @visionmedia might still merge this

@tj
Copy link
Contributor

tj commented Aug 15, 2012

yeah this should be userland and added to https://github.com/visionmedia/mocha/wiki

@tj tj closed this Aug 15, 2012
@tj
Copy link
Contributor

tj commented Aug 15, 2012

via something like:

npm install junit-reporter
mocha --reporter junit-reporter

@knewter
Copy link

knewter commented Aug 15, 2012

surely that should be called junit-reporter-mocha or something? But yeah, I get it. I'll try to extract the reporter from this pull request for my personal use, but won't publish or anything because it's @jon077's to publish :)

@tj
Copy link
Contributor

tj commented Aug 15, 2012

doesn't have to have -mocha in the name, eventually (we have an issue open for this) we'd like to have all these reporters just consume the JSON stream, so then they're completely decoupled from mocha

@sjonnet19
Copy link

I decoupled mine recently....

https://github.com/sjonnet19/mocha-cobertura-reporter

@knewter
Copy link

knewter commented Aug 15, 2012

I honestly don't even know what I was thinking...xunit reporter works just fine for what I'm doing here, don't even see how xunit / junit output differs :-\ Jenkins sees no problem with it apparently.

@jstockdi
Copy link
Author

Userland seems best. I will publish after vacation.

--jon

On Wednesday, August 15, 2012 at 1:36 PM, Josh Adams wrote:

I honestly don't even know what I was thinking...xunit reporter works just fine for what I'm doing here, don't even see how xunit / junit output differs :-\ Jenkins sees no problem with it apparently.


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

@drefined
Copy link

drefined commented Nov 2, 2012

@jon077 back from vacation yet? :)

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.

7 participants