811078 - Capture JavaScript strict warnings during xpcshell harness execution

Status: NEW

(Testing :: XPCShell Harness, defect)

Description

[Gregory Szorc [:gps]]

I would like the xpcshell test runner to capture JavaScript strict warnings (javascript.options.strict) during execution and have them automatically rendered to the process's output in such a way that they are easily seen and recorded. Currently these warnings don't appear to be captured.

I feel that having the warnings enabled by default will raise visibility of these warnings and will result in more higher quality, warning free code being written.

Can someone from the JS team weigh in on the preferred method for the xpcshell test driver to set up warnings capture?

Comment 1

[:Gavin Sharp [email: [e-mail]]]

You can capture them by setting the pref and using an nsIConsoleListener that matches on the string, or something. But like I said in the other bug, I'm not sure the resulting signal/noise ratio would be high enough for this to be useful.

Comment 2

[Gregory Szorc [:gps]]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #1)
> You can capture them by setting the pref and using an nsIConsoleListener
> that matches on the string, or something. But like I said in the other bug,
> I'm not sure the resulting signal/noise ratio would be high enough for this
> to be useful.

Are you implying:

a) our existing code is so bad that we'd see too many warnings for it to be useful
b) the warnings this mode emits are not useful for enforcing good code writing practices
c) something else

If a), I'd argue part of that is because we don't have good visibility into these warnings today and this bug would try to address that. If b), then perhaps we need more granularity on which warnings are emitted/recorded so we can improve the ability to write good code.

Either way, I think you are making a conclusion before seeing any data. I'd at least like to see a try push showing what we're dealing with. Then, we can decide if it's useful and worth turning on.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to Gregory Szorc [:gps] from comment #2)
> a) our existing code is so bad that we'd see too many warnings for it to be
> useful
> b) the warnings this mode emits are not useful for enforcing good code
> writing practices

Some combination of both.

You're absolutely right that I have no data, and this is just gut feel. Using gut feel can be a useful tool to avoid wasting resources collecting data on every whim, or it can be a dangerous path to suboptimal outcomes :) I'm not trying to discourage you from collecting the data in this particular case, but it's worth considering whether it's the best use of your limited time.

Comment 4

[Mark Banner (:standard8)]

Just came across this bug. MailNews already does this in a lot of its tests:

http://hg.mozilla.org/comm-central/annotate/3efb0311a21c/mailnews/test/resources/logHelper.js#l63

Now bug 889911 is landed, it is definitely picking up a lot of strict errors (yes, there's some actual bugs) and warnings, so IMO doing this would at least help code correctness and likely weed out some smaller bugs.