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

XUnit Reporter Writes to stdout, falls back to console.log #2005

Merged
merged 2 commits into from
Dec 25, 2015

Conversation

jonnyreeves
Copy link
Contributor

Fixes #1674; provides support much needed for mocha-phantomjs (see #133, #220, etc). Maintains support for running the XUnit reporter in a browser (unlike the previously reverted PR #1068).

ping @alemangui, @nathanboktae

Thanks all.

Review on Reviewable

Fixes mochajs#1674; provides support *much* needed for [mocha-phantomjs](https://github.com/nathanboktae/mocha-phantomjs) (see [mochajs#133](nathanboktae/mocha-phantomjs#133), [mochajs#220](nathanboktae/mocha-phantomjs#220), etc).  Maintains support for running the XUnit reporter in a browser (unlike the previously reverted PR mochajs#1068).

ping @alemangui, @nathanboktae

Thanks all.
@@ -109,6 +109,8 @@ XUnit.prototype.done = function(failures, fn) {
XUnit.prototype.write = function(line) {
if (this.fileStream) {
this.fileStream.write(line + '\n');
} else if (typeof process === 'object' && typeof process.stdout === 'object') {
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be better written as:

} else if (typeof process === 'object' && process.stdout) {

@danielstjules
Copy link
Contributor

Comment aside, LGTM. Ideally, reporters wouldn't invoke console.log directly anyway, but instead be passed a writable stream to use.

@jonnyreeves
Copy link
Contributor Author

Updated based on feedback; merging this would be joy to the worldmany! 🎄

danielstjules added a commit that referenced this pull request Dec 25, 2015
XUnit Reporter Writes to stdout, falls back to console.log
@danielstjules danielstjules merged commit c972f0b into mochajs:master Dec 25, 2015
@danielstjules
Copy link
Contributor

👍 🎅 Thanks!

@nathanboktae
Copy link
Contributor

Ideally, reporters wouldn't invoke console.log directly anyway, but instead be passed a writable stream to use.

Yes this would be great and solve alot of magic done by plugins like mocha-multi, mocha-phantomjs,mocha-casperjs`, etc. Thanks all! 👏 🎅

nathanboktae added a commit to nathanboktae/mocha-phantomjs that referenced this pull request Dec 27, 2015
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.

3 participants