Output junit reports if CI env var is set #182
Conversation
ghost
assigned 
Codecov Report
@@ Coverage Diff @@
## master #182 +/- ##
==========================================
+ Coverage 77.37% 78.16% +0.79%
==========================================
Files 6 6
Lines 137 142 +5
==========================================
+ Hits 106 111 +5
Misses 31 31
Continue to review full report at Codecov.
|
| }) | ||
| after(() => { | ||
| console.log.restore() | ||
| }) |
There was a problem hiding this comment.
Log from IRC about this:
16:10:08 <@victorbjelkholm> dignifiedquire: the only reason is to temporary hide the output of the lint command right?
16:10:38 <@dignifiedquire> yes, otherwise it will spew the console potentially during a test run with lint output, which is not that helpful
16:10:51 <@victorbjelkholm> because having some troubles with another reporter, for jenkins, that adds some functionality (saves the console.log output together with the junit report) which breaks that. I commented out the hiding of that and things work fine, but shows the lint output
16:11:14 <@victorbjelkholm> specifically it's this part which doesn't work well with console.log being stubbed: https://github.com/juhovh/mocha-jenkins-reporter/blob/master/lib/jenkins.js#L321-L327
16:11:15 <@dignifiedquire> hmm that is really strange
16:12:06 <@dignifiedquire> oh :( yeah they are using console.log directly that is unfortunate
16:12:15 <@dignifiedquire> regular mocha reporters use process.stdout
16:13:30 <@victorbjelkholm> dignifiedquire: hm, what do you mean? It's saving the output from console.log's in tests
16:13:34 <@dignifiedquire> it's not the end of the world to remove this in the tests, but might be good to file a bug that they shouldn't rely on console.log working
16:13:51 <@dignifiedquire> oh now I understand sorry
16:13:53 <@victorbjelkholm> it's to capture debug output from the tests
16:13:58 <@dignifiedquire> yeah that's just bad practice :(
16:14:09 <@dignifiedquire> they should be capturing process.stdout and stderr
16:14:12 <@victorbjelkholm> yeah :/ But guess to work with existing tests rather than adding a new function
16:14:19 <@victorbjelkholm> console.logs ends up on process.stdout?
16:14:26 <@victorbjelkholm> well, it should
16:14:29 <@dignifiedquire> if all is set up well yes
16:14:36 <@dignifiedquire> and console.error ends up on stderr
16:16:31 <@victorbjelkholm> ok. I'll remove the mocking for now, and open a issue on the reporters repository to hopefully fix it
TLDR: plugin is bad and changes the way console.log logs
There was a problem hiding this comment.
All right, is the issue to track this open?
There was a problem hiding this comment.
I was sure I opened it yesterday but forgot to submit. Anyways, is here now: juhovh/mocha-jenkins-reporter#75
There was a problem hiding this comment.
Perhaps open one also in this repo so that one is easy to find
|
Oh, right! This PR is missing exporting a report for webworker tests. I was unable to get the reporter to work with the webworker code. @dryajov maybe you could help out here, think you implemented the webworker tester, correct? |
|
Ok, this is LGTM. However, aegir is kind of blocked on releasing until #181 is figured out. Can we have some extra hands there? Thank you! |
Adds junit output if env var CI is set to true.