toolkit/devtools/debugger xpcshell tests that throw errors do not fail

RESOLVED FIXED in Firefox 15

Status

()

Firefox
Developer Tools: Debugger
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jimb, Unassigned)

Tracking

Trunk
Firefox 15
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 618709 [details] [diff] [review]
Have toolkit/devtools/debugger xpcshell tests register a listener for Components.utils.reportError, so tests fail when they throw an exception.

If a toolkit/devtools/debugger/tests xpcshell test causes dbg-server or dbg-client code to throw an error, the test still passes. This is because:

1) both client and server contain 'catch' clauses which report the error via Cu.reportError; and

2) we never register a console listener.

We should register a console listener, and if it gets an nsIScriptError, we should throw.
Attachment #618709 - Flags: review?(dcamp)
(Reporter)

Comment 1

5 years ago
Try: https://tbpl.mozilla.org/?tree=Try&rev=73d997fe8c3d
(Reporter)

Comment 2

5 years ago
(The patch also includes a bunch of minor changes like changing throw 'foo' to throw Error('foo') so we'll get backtraces, and printing stacks so Emacs can pick up the filenames and line numbers.)
Comment on attachment 618709 [details] [diff] [review]
Have toolkit/devtools/debugger xpcshell tests register a listener for Components.utils.reportError, so tests fail when they throw an exception.

Review of attachment 618709 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/debugger/server/dbg-server.js
@@ +63,5 @@
>    }
>  }
>  
> +/* Turn the error e into a string, without fail. */
> +function safeErrorString(e) {

We use aError or somesuch in browser code.

::: toolkit/devtools/debugger/tests/unit/head_dbg.js
@@ +16,5 @@
>  Cu.import("resource:///modules/devtools/dbg-server.jsm");
>  Cu.import("resource:///modules/devtools/dbg-client.jsm");
>  
> +// Convert an nsIScriptError 'flags' value into an appropriate string.
> +function scriptErrorFlagsToKind(flags) {

aFlags
Attachment #618709 - Flags: review?(dcamp) → review+
Created attachment 618744 [details] [diff] [review]
Fixes for tests that were exposed as broken with the above patch

We should land both of these at the same time.
Attachment #618744 - Flags: review?(dcamp)

Updated

5 years ago
Attachment #618744 - Flags: review?(dcamp) → review+
Duplicate of this bug: 749309
Duplicate of this bug: 749311
(Reporter)

Comment 7

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/6a369d883d74
Status: NEW → ASSIGNED
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/ff39d7da1dd5
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
Backed out the latest fx-team merge whole-sale because of Ts regressions:

https://hg.mozilla.org/mozilla-central/rev/b0200dab0ccc

Please reland only after investigating and fixing the regression.  Thanks!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I messed up and backed out the wrong range, sorry about that.  I backed out my backout, so this is still on mozilla-central:

https://hg.mozilla.org/mozilla-central/rev/dec5b367c421
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.