657507 - Log or Error when something reports an error to the error console

Status: RESOLVED WONTFIX

(Testing :: XPCShell Harness, defect)

Description

[Shawn Wilsher :sdwilsh]

Last week I spent over an hour trying to debug a hang in a test that would not have taken very long at all had xpcshell either been throwing an error or at the very least logging when some code does Components.utils.reportError.  I'm leaning toward error being the right call, but at this point, we might have tests that depend on that not failing, so logging may be the only option.  See https://hg.mozilla.org/mozilla-central/annotate/6105fb36f613/toolkit/components/places/tests/unit/test_preventive_maintenance_console.js#l17 for an example on how to do this (hell, I'll even write this patch if someone makes the decision on which way to go).

Comment 1

[(not currently active) Ted Mielczarek]

If it doesn't fail any tests, then make it error. If it does, then just make it log for now.

Comment 2

[Gregory Szorc [:gps]]

Attachment: Write all console service messages to output, v1

In the course of filing bug 677779, I created a simple patch to write all console service messages to the output of the xpcshell test harness.

The patch doesn't conditionally log (as desired from comment #1), so I'm not submitting it for review. But, if somebody wants to use it as the base for the final patch, be my guest! Or, if we just want to ignore comment #1, I suppose this is ready for review.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Write console service messages to output, v2

This patch will have xpcshell write *all* console service messages to the output. I have a Try running now. It will report in when complete.

It's possible this may add significant output to xpcshell tests and may even cause them to slow down or break some parsers of the output. Should I consider limiting printing to cases that begin with "[JavaScript Error:" ? Is this satisfactory to catch all swallowed messages?

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 565412 [details] [diff] [review]
Write console service messages to output, v2

Cancelling review due to Try failures.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

(In reply to Gregory Szorc [:gps] from comment #4)
> Comment on attachment 565412 [details] [diff] [review]
> Cancelling review due to Try failures.

Can you remember what kinds of errors you saw on try?  This cost me some time today too - maybe a lighter approach with less impact on a "normal" run is to have the harness buffer output from a test and only dump it on test failure?

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:jmaher, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

this has been around a long time and development appears to be going on ok.