Closed Bug 1459398 Opened 4 years ago Closed 4 years ago

Log explicit failure when telemetry-test-helpers.js is missing from browser.ini

Categories

(DevTools :: General, enhancement, P3)

61 Branch
enhancement

Tracking

(firefox61 wontfix, firefox62 wontfix, firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(1 file)

Bug 1098374 made telemetry-test-helpers.js a mandatory support file, to be defined in the browser.ini of any test suite that uses shared-head.js. 

Can we clearly indicate to developers what is wrong if they forgot to add the dependency? I faced this issue during a debugger release which removed the fix to the debugger's browser.ini, and the test failure was confusing, just complaining about an undefined method.

I think the same situation may arise whenever we create a new test suite, so I would like to propose to improve the logs here. Logging a test failure could be an improvement, at least it should be visible at the end of the test and should lead the developer in the right direction.
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Comment on attachment 8973436 [details]
Bug 1459398 - log a clear test failure if mandatory telemetry-test-helpers.js cannot be loaded;

https://reviewboard.mozilla.org/r/241842/#review248526

I like this... r+ if you fix the other place we load the file.

::: devtools/client/shared/test/shared-head.js:40
(Diff revision 1)
>  const URL_ROOT = CHROME_URL_ROOT.replace("chrome://mochitests/content/",
>                                           "http://example.com/");
>  const URL_ROOT_SSL = CHROME_URL_ROOT.replace("chrome://mochitests/content/",
>                                               "https://example.com/");
>  
> +try {

This looks great... can you also add this to the other place we load it?
devtools/client/performance/test/head.js
Attachment #8973436 - Flags: review?(mratcliffe) → review+
Product: Firefox → DevTools
Comment on attachment 8973436 [details]
Bug 1459398 - log a clear test failure if mandatory telemetry-test-helpers.js cannot be loaded;

https://reviewboard.mozilla.org/r/241842/#review248526

Thanks for the review! Completely forgot about this bug. Will land now.

> This looks great... can you also add this to the other place we load it?
> devtools/client/performance/test/head.js

Added it there. We also call it from commandline head.js but since we should delete this soon I haven't updated it.
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/15f630edfb39
log a clear test failure if mandatory telemetry-test-helpers.js cannot be loaded;r=miker
https://hg.mozilla.org/mozilla-central/rev/15f630edfb39
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
See Also: → 1503169
You need to log in before you can comment on or make changes to this bug.