jberger / Mojo-Phantom

PhantomJS client-side testing for Mojolicious apps
Other
13 stars 6 forks source link

redirect console output to note #5

Closed plicease closed 8 years ago

plicease commented 8 years ago

A log of JavaScript libraries use console.log(). I was missing important diagnostics that would have saved me a lot of frustration had I seen them earlier. I think others might find this also to be a useful default.

I'd also like to see note included by default in addition to diag since I personally use it much more frequently.

plicease commented 8 years ago

If this is a reasonable approach it is probably worth redirecting console.warn and console.error in a similar way.

jberger commented 8 years ago

I generally like this idea, though I think perhaps this http://phantomjs.org/api/webpage/handler/on-console-message.html is a better way to attach than monkey patching. Would you mind trying it that way? That should give other log levels as well I think.

plicease commented 8 years ago

Using the callback will catch calls to console.{log,warn,error} in the faux browser, but not the phantom side, which is from where I was missing the diagnostic. I would actually like to see both, but if nothing else catching the browser console's messages would at least be a start. I will open another PR when I get a chance and amend this one, hopefully it will be clear.

plicease commented 8 years ago

I've updated this PR to intercept the phantom script's console.log rather than replacing it. I've also opened up #7 in response to your comment above, to redirect just the browser's console.log, and rebased this PR on top of it.

I'm still hoping you will consider also redirecting the console.log in the phantom script as it can also be helpful for diagnostic purposes. I've attempted to make it at least a little more palatable by intercepting the original console.log rather than completely replacing it. I found the two environments aspect of PhantomJS very confusing until I read this: http://www.crmarsh.com/phantomjs/

jberger commented 8 years ago

This looks fine to me. I'd like to add just a small conditional at least to the page context to quiet a noisy page.

plicease commented 8 years ago

done!