1408947 - Enable browser_webconsole_reopen_closed_tab.js in the new console frontend

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1408947 - enable browser_webconsole_reopen_closed_tab.js in the new console frontend;

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

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8931397 [details]
Bug 1408947 - enable browser_webconsole_reopen_closed_tab.js in the new console frontend;

https://reviewboard.mozilla.org/r/202518/#review207880

I only have nits, but this is already good to land

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_reopen_closed_tab.js:14
(Diff revision 1)
> -var HUD;
> +function expectUncaughtExceptionNoE10s() {
> -
> -add_task(function* () {
>    // On e10s, the exception is triggered in child process
>    // and is ignored by test harness
>    if (!Services.appinfo.browserTabsRemoteAutostart) {
>      expectUncaughtException();
>    }
> +}

nit: move this after add_task, so the test is (almost) the first thing in the file.

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_reopen_closed_tab.js:23
(Diff revision 1)
>      expectUncaughtException();
>    }
> +}
>  
> -  let { browser } = yield loadTab(TEST_URI);
> -  HUD = yield openConsole();
> +add_task(async function () {
> +  info("Open console and refresh tab.");

wasn't in the original test, but can we ensure that "Persist logs" is not on by using `pushPref("devtools.webconsole.persistlog", false)`

Because if we do persist log, the test might be successful even if only the first error log is shown

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_reopen_closed_tab.js:33
(Diff revision 1)
>  
> -  yield reload(browser);
> +  expectUncaughtExceptionNoE10s();
> +  await refreshTab();
>  
> -  yield testMessages();
> +  info("Wait for error message");
> +  await waitFor(() => findMessage(hud, "fooBug597756_error", ".message.error"));

nit: extract `waitFor(() => findMessage(hud, "fooBug597756_error", ".message.error"))` in its own function (waitForErrorMessage ?)

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:405
(Diff revision 1)
> + *        Optional tab element for which you want close the Web Console. The
> + *        default tab is taken from the global variable |tab|.
> + * @return object
> + *         A promise that is resolved once the web console is closed.
> + */
> +async function closeConsole(tab) {

could we use a default parameter ?

Comment 3

[Julian Descottes [:jdescottes]]

Comment on attachment 8931397 [details]
Bug 1408947 - enable browser_webconsole_reopen_closed_tab.js in the new console frontend;

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

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8931397 [details]
Bug 1408947 - enable browser_webconsole_reopen_closed_tab.js in the new console frontend;

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bac113bafbdf
enable browser_webconsole_reopen_closed_tab.js in the new console frontend;r=nchevobbe

Comment 6

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/bac113bafbdf