nathanboktae / mocha-phantomjs

:coffee: :ghost: Run client-side mocha tests in the command line through phantomjs
MIT License
954 stars 112 forks source link

`fail` should output to stderr #162

Closed shellscape closed 9 years ago

shellscape commented 9 years ago

Currently the fail method uses console.log. This is technically "incorrect." Since its explicitly in a failure state at that point, it should probably output to stderr or console.error.

metaskills commented 9 years ago

I agree, as illustrated by $ curl 2> ~/error.log dumping stderr to that file. Would you like to whip up a PR?

shellscape commented 9 years ago

@metaskills let me see if I can do that, and not manage to screw it up. I'm coffeescript illiterate :)

metaskills commented 9 years ago

Fair, but it is not that hard. Anyone that knows JS should be able to read this file and see where your points are. Can you please make an effort?

https://github.com/metaskills/mocha-phantomjs/blob/master/lib/mocha-phantomjs.coffee#L31

metaskills commented 9 years ago

FYI, we have tests too. So you are not without a safety net.

nathanboktae commented 9 years ago

This should just work but doesn't due to ariya/phantomjs#10150

There is a monkey patch solution suggested:

console.error = function () {
    require("system").stderr.write(Array.prototype.join.call(arguments, ' ') + '\n');
};

which in coffee-script that is

console.error = ->
  require("system").stderr.write(Array.prototype.join.call(arguments, ' ') + '\n')

or

console.error = ->
  require('system').stderr.write "#{Array.prototype.join.call(arguments, ' ')}\n"
shellscape commented 9 years ago

@nathanboktae wouldn't that work because core_extensions.js sets that up?

nathanboktae commented 9 years ago

no this monkey patching needs to be done the PhantomJS side, not the browser side.

shellscape commented 9 years ago

@nathanboktae @metaskills ping. made a commit last week on this one.

nathanboktae commented 9 years ago

let's continue this discussion in the PR (in the future, you can attach pull requests to existing issues with the GitHub API)