Closed
Bug 660619
Opened 13 years ago
Closed 13 years ago
Silence a warning during tests
Categories
(DevTools :: General, defect)
DevTools
General
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 7
People
(Reporter: past, Assigned: past)
Details
(Whiteboard: [fixed-in-devtools][merged-to-mozilla-central])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•13 years ago
|
||
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.
Assignee | ||
Comment 2•13 years ago
|
||
Comment on attachment 536062 [details] [diff] [review]
Simpl
This is ddahl's code AFAICT.
Attachment #536062 -
Attachment is patch: true
Attachment #536062 -
Attachment mime type: text/x-patch → text/plain
Attachment #536062 -
Flags: review?(ddahl)
Updated•13 years ago
|
Attachment #536062 -
Flags: review?(ddahl) → review+
Assignee | ||
Updated•13 years ago
|
Attachment #536062 -
Flags: review?(gavin.sharp)
Comment 3•13 years ago
|
||
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.
Attachment #536062 -
Flags: review?(gavin.sharp) → review-
Assignee | ||
Comment 4•13 years ago
|
||
Assignee: nobody → past
Attachment #536062 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #536246 -
Flags: review?(gavin.sharp)
Comment 5•13 years ago
|
||
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.
Attachment #536246 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 6•13 years ago
|
||
(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.
Whiteboard: [checkin-needed]
Updated•13 years ago
|
Whiteboard: [checkin-needed] → [fixed-in-devtools]
Comment 7•13 years ago
|
||
Comment on attachment 536246 [details] [diff] [review]
[in-devtools][checked-in] Unconditional import
http://hg.mozilla.org/projects/devtools/rev/c4bd2219aafc
Attachment #536246 -
Attachment description: Unconditional import → [in-devtools] Unconditional import
Comment 8•13 years ago
|
||
whups, that's not the right link. I should refresh tbpl before grabbing the checkin url.
http://hg.mozilla.org/projects/devtools/rev/b9ceaebb878f
Updated•13 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-devtools] → [fixed-in-devtools][merged-to-mozilla-central]
Target Milestone: --- → Firefox 7
Comment 9•13 years ago
|
||
Comment on attachment 536246 [details] [diff] [review]
[in-devtools][checked-in] Unconditional import
http://hg.mozilla.org/mozilla-central/rev/b9ceaebb878f
Attachment #536246 -
Attachment description: [in-devtools] Unconditional import → [in-devtools][checked-in] Unconditional import
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•