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 should use process.stdout if available... fallback to console.log #1674

Closed
alemangui opened this issue Apr 28, 2015 · 5 comments
Labels
status: waiting for author waiting on response from OP - more information needed

Comments

@alemangui
Copy link

Hello

I'm running Mocha on PhantomJS using https://github.com/metaskills/mocha-phantomjs This means Mocha is not run in Node and thus has no access to fs.

Mocha-PhantomJS provides the option of dumping the reporter output to a file by redirecting process.stdout.write. Unfortunately, I'm having issues with the xunit reporter since it is using console.log.

I understand this is to maintain compatibility with browsers, but would it be desirable to check the existence of process.stdout and call it if available?

It would be a trivial PR... I would gladly prepare it but I would like to ask for your insight first:

/**
 * Write out the given line
 */
XUnit.prototype.write = function(line) {
    if (this.fileStream) { 
        // Node JS environment
        this.fileStream.write(line + '\n');
    } else if (process && process.stdout) {
        // Browser with process.stdout (as in mocha-phantomjs)
        process.stdout.write(line + '\n');
    } else {
        // Browser
        console.log(line);
    }
};
@boneskull
Copy link
Member

console.log calls process.stdout.write last time I checked. I don't understand the problem. Is this a mocha-phantomjs issue?

@boneskull boneskull added Unconfirmed status: waiting for author waiting on response from OP - more information needed labels May 12, 2015
@alemangui
Copy link
Author

The problem is basically that Mocha-Phantom (and potentially other tools/scripts) can redirect the process.stdout to a different output (i.e., to have the reporter output saved into a file). Having some reporters using console.log removes this possibility.

@danielstjules
Copy link
Contributor

@alemangui console.log uses process.stdout.write internally. Here's an example:

var oldWrite = process.stdout.write;
var counter = 0;
var output = '';

process.stdout.write = function(str) {
  counter++;
  output += counter + '. ' + str;
};

// Does not output foo/bar
console.log('foo');
console.log('bar');

// Restore and print modified output
process.stdout.write = oldWrite;
console.log(output);
$ node example.js
1. foo
2. bar

So if you redirect process.stdout to a different output, console.log will use it as well.

@alemangui
Copy link
Author

Thanks for your clarification @danielstjules. console.log indeed uses process.stdout under a Node environment.

The problem I'm having with Mocha-Phantomjs - if I understand correctly - comes from the fact there is a differentiation between console.log calls and process.stdout calls in order to tell apart the reporter output from the other console.log calls (Mocha-Phantomjs is after all mocha running in a browser environment).

I'm guessing nathanboktae/mocha-phantomjs#161 and nathanboktae/mocha-phantomjs#114 (both on mocha-phantomjs side) are related to what I'm saying.

At this point I'm not sure where the fix should be though.

@jbnicolai jbnicolai added status: accepting prs Mocha can use your help with this one! and removed Unconfirmed labels Jul 6, 2015
@jbnicolai
Copy link

I'd be happy with a node/browser compatible solution for this, but otherwise we'll keep it as is.

jonnyreeves added a commit to jonnyreeves/mocha that referenced this issue Dec 14, 2015
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.
@boneskull boneskull removed the status: accepting prs Mocha can use your help with this one! label Oct 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting for author waiting on response from OP - more information needed
Projects
None yet
Development

No branches or pull requests

4 participants