1404853 - Migrate browser_webconsole_bug_653531_highlighter_console_helper.js to the new frontend

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

the test is always skipped on ci at the moment (because it requires direct access to content).
We should take this migration as an opportunity to rewrite the test in a way that does not require direct access

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1404853 - Enable browser_webconsole_highlighter_console_helper.js in new console frontend; .

This changes the code of the test since it was relying on direct access to the content page.
In order to be able to interact with the element picker, the test-actor and test-actor-registery
files are loaded by head.js.

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

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8939576 [details]
Bug 1404853 - Enable browser_webconsole_highlighter_console_helper.js in new console frontend; .

https://reviewboard.mozilla.org/r/209888/#review215800

Looks good, just couple of nits.

Honza

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_highlighter_console_helper.js:56
(Diff revision 1)
> -  }
>  
> -  function onNodeUpdate(node) {
> -    isnot(node.textContent.indexOf("bug653531"), -1,
> -          "correct output for $0.textContent");
> -    is(inspector.selection.node.textContent, "bug653531",
> +  let onEvaluationResult = waitForMessage(hud, "<h1>");
> +  jsterm.execute("$0");
> +  await onEvaluationResult;
> +  ok(true, "correct output for $0");

Why not using just `info` API?

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_highlighter_console_helper.js:65
(Diff revision 1)
> -  gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
> -  gBrowser.selectedBrowser.addEventListener("load", function () {
> -    waitForFocus(createDocument, content);
> -  }, {capture: true, once: true});
> +  const newH1Content = "newH1Content";
> +  let onAssignmentResult = waitForMessage(hud, "<h1>");
> +  jsterm.execute(`$0.textContent = "${newH1Content}";$0`);
> +  await onAssignmentResult;
> +
> +  ok(true, "correct output for $0 after setting $0.textContent");

The same as above.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8939576 [details]
Bug 1404853 - Enable browser_webconsole_highlighter_console_helper.js in new console frontend; .

https://reviewboard.mozilla.org/r/209888/#review215800

> Why not using just `info` API?

because if we make it to this point this means the test case is correct (PASS).
I think I'm used to this because you can't have a test without assertions, and here, even if we don't assert a specific value, we assert that we made it to this point.
Does that make sense ? I don't feel too strong about it, so I'm also okay with changing it.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #3)
> because if we make it to this point this means the test case is correct
> (PASS).
> I think I'm used to this because you can't have a test without assertions,
> and here, even if we don't assert a specific value, we assert that we made
> it to this point.
I see, sounds reasonable. Let's keep it as is.
Thanks,
Honza

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e14fbde53491
Enable browser_webconsole_highlighter_console_helper.js in new console frontend; r=Honza.

Comment 6

[Dorel Luca [:dluca]]

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

Comment 7

[Gerry Chang [:gchang]]

It's too late for 58. Mark 58 won't fix.