Last Comment Bug 749258 - toolkit/devtools/debugger xpcshell tests that throw errors do not fail
: toolkit/devtools/debugger xpcshell tests that throw errors do not fail
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Debugger (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 15
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
: 749309 749311 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-26 10:33 PDT by Jim Blandy :jimb
Modified: 2012-05-02 13:15 PDT (History)
2 users (show)
jimb: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Have toolkit/devtools/debugger xpcshell tests register a listener for Components.utils.reportError, so tests fail when they throw an exception. (4.63 KB, patch)
2012-04-26 10:33 PDT, Jim Blandy :jimb
past: review+
Details | Diff | Splinter Review
Fixes for tests that were exposed as broken with the above patch (2.43 KB, patch)
2012-04-26 11:57 PDT, Panos Astithas [:past]
dcamp: review+
Details | Diff | Splinter Review

Description Jim Blandy :jimb 2012-04-26 10:33:26 PDT
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.
Comment 1 Jim Blandy :jimb 2012-04-26 10:38:15 PDT
Try: https://tbpl.mozilla.org/?tree=Try&rev=73d997fe8c3d
Comment 2 Jim Blandy :jimb 2012-04-26 10:45:55 PDT
(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 3 Panos Astithas [:past] 2012-04-26 10:59:03 PDT
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
Comment 4 Panos Astithas [:past] 2012-04-26 11:57:59 PDT
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.
Comment 5 Panos Astithas [:past] 2012-04-26 14:26:49 PDT
*** Bug 749309 has been marked as a duplicate of this bug. ***
Comment 6 Panos Astithas [:past] 2012-04-26 14:27:12 PDT
*** Bug 749311 has been marked as a duplicate of this bug. ***
Comment 8 Tim Taubert [:ttaubert] 2012-05-02 06:48:39 PDT
https://hg.mozilla.org/mozilla-central/rev/ff39d7da1dd5
Comment 9 :Ehsan Akhgari 2012-05-02 12:44:03 PDT
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!
Comment 10 :Ehsan Akhgari 2012-05-02 13:14:52 PDT
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

Note You need to log in before you can comment on or make changes to this bug.