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

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

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.

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1459398 - log a clear test failure if mandatory telemetry-test-helpers.js cannot be loaded;

Review commit: https://reviewboard.mozilla.org/r/241842/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/241842/

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

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

Comment 3

[Julian Descottes [:jdescottes]]

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.

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8973436 [details]
Bug 1459398 - log a clear test failure if mandatory telemetry-test-helpers.js cannot be loaded;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241842/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15f630edfb39
log a clear test failure if mandatory telemetry-test-helpers.js cannot be loaded;r=miker

Comment 6

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/15f630edfb39