Last Comment Bug 660619 - Silence a warning during tests
: Silence a warning during tests
Status: RESOLVED FIXED
[fixed-in-devtools][merged-to-mozilla...
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Firefox 7
Assigned To: Panos Astithas [:past] (away until 7/21)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-30 05:38 PDT by Panos Astithas [:past] (away until 7/21)
Modified: 2011-06-10 18:02 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Simpl (705 bytes, patch)
2011-05-30 05:38 PDT, Panos Astithas [:past] (away until 7/21)
bugzilla: review+
gavin.sharp: review-
Details | Diff | Splinter Review
[in-devtools][checked-in] Unconditional import (1.09 KB, patch)
2011-05-31 00:45 PDT, Panos Astithas [:past] (away until 7/21)
gavin.sharp: review+
Details | Diff | Splinter Review

Description Panos Astithas [:past] (away until 7/21) 2011-05-30 05:38:16 PDT
Created attachment 536062 [details] [diff] [review]
Simpl
Comment 1 Panos Astithas [:past] (away until 7/21) 2011-05-30 05:43:10 PDT
There is an annoying warning that keeps recurring when running tests:

JS Component Loader: WARNING chrome://mochitests/content/browser/toolkit/components/console/hudservice/tests/browser/head.js:46
                     anonymous function does not always return a value

This is a simple fix for getting rid of the warning, although I would prefer avoiding the try/catch altogether. I suppose there must have been a reason for putting it there in the first place, though, so this patch leaves it in place.

And yes, pressing ENTER in the bug submission form, occasionally posts unfinished comments. Thanks for asking.
Comment 2 Panos Astithas [:past] (away until 7/21) 2011-05-30 05:45:26 PDT
Comment on attachment 536062 [details] [diff] [review]
Simpl

This is ddahl's code AFAICT.
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-05-30 08:21:55 PDT
Comment on attachment 536062 [details] [diff] [review]
Simpl

I don't think this try/catch is useful, "return HUDService" isn't actually going to throw if the import succeeded. I'd say just get rid of it. Actually, the entire lazy getter isn't actually useful given that pretty much all tests use it eagerly (and the perf difference is negligible anyways), so unless you somehow care about "ConsoleUtils" polluting the scope (seems unlikely), just remove it entirely and import() HUDService.jsm unconditionally.
Comment 4 Panos Astithas [:past] (away until 7/21) 2011-05-31 00:45:46 PDT
Created attachment 536246 [details] [diff] [review]
[in-devtools][checked-in] Unconditional import
Comment 5 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-05-31 02:27:52 PDT
Comment on attachment 536246 [details] [diff] [review]
[in-devtools][checked-in] Unconditional import

We really should get a bug on file about removing all these imports if there isn't one already. Test code shouldn't be re-importing all of these things - if they're really needed they're already imported by the chrome window the test is running in. I guess bug 610953 could cover that too.
Comment 6 Panos Astithas [:past] (away until 7/21) 2011-05-31 06:28:26 PDT
(In reply to comment #5)
> Comment on attachment 536246 [details] [diff] [review] [review]
> Unconditional import
> 
> We really should get a bug on file about removing all these imports if there
> isn't one already. Test code shouldn't be re-importing all of these things -
> if they're really needed they're already imported by the chrome window the
> test is running in. I guess bug 610953 could cover that too.

Thanks, added a note to that effect in bug 610953.
Comment 7 Rob Campbell [:rc] (:robcee) 2011-05-31 10:47:11 PDT
Comment on attachment 536246 [details] [diff] [review]
[in-devtools][checked-in] Unconditional import

http://hg.mozilla.org/projects/devtools/rev/c4bd2219aafc
Comment 8 Rob Campbell [:rc] (:robcee) 2011-05-31 10:48:27 PDT
whups, that's not the right link. I should refresh tbpl before grabbing the checkin url.

http://hg.mozilla.org/projects/devtools/rev/b9ceaebb878f
Comment 9 Dave Camp (:dcamp) 2011-06-10 18:02:33 PDT
Comment on attachment 536246 [details] [diff] [review]
[in-devtools][checked-in] Unconditional import

http://hg.mozilla.org/mozilla-central/rev/b9ceaebb878f

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