-
Notifications
You must be signed in to change notification settings - Fork 112
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
fail
should output to stderr
#162
Comments
I agree, as illustrated by |
@metaskills let me see if I can do that, and not manage to screw it up. I'm coffeescript illiterate :) |
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 |
FYI, we have tests too. So you are not without a safety net. |
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" |
@nathanboktae wouldn't that work because core_extensions.js sets that up? |
no this monkey patching needs to be done the PhantomJS side, not the browser side. |
@nathanboktae @metaskills ping. made a commit last week on this one. |
let's continue this discussion in the PR (in the future, you can attach pull requests to existing issues with the GitHub API) |
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.The text was updated successfully, but these errors were encountered: