Closed Bug 1408947 Opened 2 years ago Closed 2 years ago

Enable browser_webconsole_reopen_closed_tab.js in the new console frontend

Categories

(DevTools :: Console, enhancement, P1)

enhancement

Tracking

(firefox59 fixed)

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: nchevobbe, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

(Whiteboard: [newconsole-mvp])

Attachments

(1 file)

No description provided.
Blocks: 1400847
Priority: P3 → P2
Whiteboard: [newconsole-mvp]
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: P2 → P1
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 ?
Attachment #8931397 - Flags: review?(nchevobbe) → review+
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bac113bafbdf
enable browser_webconsole_reopen_closed_tab.js in the new console frontend;r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/bac113bafbdf
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.