1408946 - Enable browser_webconsole_output_order.js in the new console frontend

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1408946 - enable browser_webconsole_output_order.js;

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

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8931422 [details]
Bug 1408946 - enable browser_webconsole_output_order.js;

https://reviewboard.mozilla.org/r/202564/#review208108

Thanks Julian, I'm concerned about a race condition here, see my comments

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_output_order.js:18
(Diff revision 1)
> -  let jsterm = hud.jsterm;
> -
> -  jsterm.clearOutput();
> -  jsterm.execute("console.log('foo', 'bar');");
> -
> -  let [functionCall, consoleMessage, result] = yield waitForMessages({
> +  hud.jsterm.execute("console.log('foo', 'bar');");
> +
> +  let messages = ["console.log('foo', 'bar');", "foo bar", "undefined"];
> +  const [functionCall, consoleMessage, result] = await waitForMessages({
> +    hud,
> +    messages: messages.map(text => ({text}))
> -    webconsole: hud,
> -    messages: [{
> -      text: "console.log('foo', 'bar');",
> -      category: CATEGORY_INPUT,
> -    },
> -      {
> -        text: "foo bar",
> -        category: CATEGORY_WEBDEV,
> -        severity: SEVERITY_LOG,
> -      },
> -      {
> -        text: "undefined",
> -        category: CATEGORY_OUTPUT,
> -      }]
>    });

I think we open the door to intermittence here : waitForMessages is now listening for an event to be fired, and I guess if the jsterm execution is fast enough, we could miss it ? Unlinkely, but I'd still do : 

let messages = ["console.log('foo', 'bar');", "foo bar", "undefined"];
const onMessages = await waitForMessages({
    hud,
    messages: messages.map(text => ({text}))
});

hud.jsterm.execute("console.log('foo', 'bar');");
const [functionCall, consoleMessage, result] = await  onMessages;

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_output_order.js:26
(Diff revision 1)
> -  let fncallNode = [...functionCall.matched][0];
> -  let consoleMessageNode = [...consoleMessage.matched][0];
> -  let resultNode = [...result.matched][0];
> +  let fncallNode = functionCall.node.closest(".message");
> +  let consoleMessageNode = consoleMessage.node.closest(".message");
> +  let resultNode = result.node.closest(".message");

doesn't {waitForMessageResult}.node give us the .message element ?

Comment 3

[Julian Descottes [:jdescottes]]

Comment on attachment 8931422 [details]
Bug 1408946 - enable browser_webconsole_output_order.js;

https://reviewboard.mozilla.org/r/202564/#review208108

> I think we open the door to intermittence here : waitForMessages is now listening for an event to be fired, and I guess if the jsterm execution is fast enough, we could miss it ? Unlinkely, but I'd still do : 
> 
> let messages = ["console.log('foo', 'bar');", "foo bar", "undefined"];
> const onMessages = await waitForMessages({
>     hud,
>     messages: messages.map(text => ({text}))
> });
> 
> hud.jsterm.execute("console.log('foo', 'bar');");
> const [functionCall, consoleMessage, result] = await  onMessages;

Yep good point.

> doesn't {waitForMessageResult}.node give us the .message element ?

it does, not sure why I thought otherwise.

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8931422 [details]
Bug 1408946 - enable browser_webconsole_output_order.js;

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

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8931422 [details]
Bug 1408946 - enable browser_webconsole_output_order.js;

https://reviewboard.mozilla.org/r/202564/#review208394

Thanks for the changes Julian, let's land this

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_output_order.js:26
(Diff revision 2)
> -  let resultNode = [...result.matched][0];
> +  const [functionCall, consoleMessage, result] = await onMessages;
> +
> +  let fncallNode = functionCall.node;
> +  let consoleMessageNode = consoleMessage.node;
> +  let resultNode = result.node;

nit: we could clean this into 

const [fncallNode, consoleMessageNode, resultNode] = (await onMessages).map(msg => msg.node);

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8931422 [details]
Bug 1408946 - enable browser_webconsole_output_order.js;

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd5a8a6bb54e
enable browser_webconsole_output_order.js;r=nchevobbe

Comment 8

[Tiberius Oros[:tiberius_oros]]

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

Comment 9

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd5a8a6bb54e