Closed Bug 1437855 Opened 6 years ago Closed 6 years ago

Enable browser_console_webconsole_iframe_messages.js in new frontend

Categories

(DevTools :: Console, enhancement, P1)

enhancement

Tracking

(firefox60 fixed)

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: nchevobbe, Assigned: miker)

References

Details

(Whiteboard: [newconsole-mvp])

Attachments

(1 file)

      No description provided.
Has Regression Range: --- → irrelevant
Has STR: --- → irrelevant
Priority: -- → P2
Whiteboard: [newconsole-mvp]
Severity: normal → enhancement
Assignee: nobody → mratcliffe
Status: NEW → ASSIGNED
OS: Unspecified → All
Priority: P2 → P1
Hardware: Unspecified → All
Attachment #8950907 - Flags: review?(jdescottes) → review?(nchevobbe)
Comment on attachment 8950907 [details]
Bug 1437855 - Enable browser_console_webconsole_iframe_messages.js in new frontend

https://reviewboard.mozilla.org/r/220162/#review226044

This is looking good to me. I have some doubt about the "iframe 1" message case, but let's land this if it has a green try and does not fail with `--verify`

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_console_webconsole_iframe_messages.js:23
(Diff revision 1)
> -// "iframe 1" console messages can be coalesced into one if they follow each
> +const expectedDupedMessage = "iframe 1";
> -// other in the sequence of messages (depending on timing). If they do not, then
> -// they will be displayed in the console output independently, as separate
> -// messages. This is why we need to match any of the following two rules.
> -const expectedMessagesAny = [
> -  {
> -    name: "iframe 1 (count: 2)",
> -    text: "iframe 1",
> -    category: CATEGORY_WEBDEV,
> -    severity: SEVERITY_LOG,
> -    count: 2
> -  },
> -  {
> -    name: "iframe 1 (repeats: 2)",
> -    text: "iframe 1",
> -    category: CATEGORY_WEBDEV,
> -    severity: SEVERITY_LOG,
> -    repeats: 2
> -  },
> -];

it was a weird thing. Did we got rid of this ?
Attachment #8950907 - Flags: review?(nchevobbe) → review+
Comment on attachment 8950907 [details]
Bug 1437855 - Enable browser_console_webconsole_iframe_messages.js in new frontend

https://reviewboard.mozilla.org/r/220162/#review226048

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_console_webconsole_iframe_messages.js:23
(Diff revision 1)
> -// "iframe 1" console messages can be coalesced into one if they follow each
> +const expectedDupedMessage = "iframe 1";
> -// other in the sequence of messages (depending on timing). If they do not, then
> -// they will be displayed in the console output independently, as separate
> -// messages. This is why we need to match any of the following two rules.
> -const expectedMessagesAny = [
> -  {
> -    name: "iframe 1 (count: 2)",
> -    text: "iframe 1",
> -    category: CATEGORY_WEBDEV,
> -    severity: SEVERITY_LOG,
> -    count: 2
> -  },
> -  {
> -    name: "iframe 1 (repeats: 2)",
> -    text: "iframe 1",
> -    category: CATEGORY_WEBDEV,
> -    severity: SEVERITY_LOG,
> -    repeats: 2
> -  },
> -];

Yes, `findMessages()` will return a count of them whether they are grouped or not so we no longer need the special treatment.
(In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from comment #3)
> Yes, `findMessages()` will return a count of them whether they are grouped
> or not so we no longer need the special treatment.

Looking at https://searchfox.org/mozilla-central/rev/d03ad8843e3bf2e856126bc53b0475c595e5183b/devtools/client/webconsole/new-console-output/test/mochitest/head.js#196-203 , I think it only returns the number of nodes with expected text.
So if a message is grouped (repeated), there would only be one node, and the test would fail.

Now, this is true only if the message is repeated, which may not be the case anymore ?
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 450420aeded4c6846f9c7480ce8b6c08733ef2ad -d 4eab912e9f42: rebasing 447338:450420aeded4 "Bug 1437855 - Enable browser_console_webconsole_iframe_messages.js in new frontend r=nchevobbe" (tip)
merging devtools/client/webconsole/new-console-output/test/mochitest/browser.ini
warning: conflicts while merging devtools/client/webconsole/new-console-output/test/mochitest/browser.ini! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by mratcliffe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/03fb2429290e
Enable browser_console_webconsole_iframe_messages.js in new frontend r=nchevobbe
Attachment #8950907 - Flags: review?(jdescottes)
https://hg.mozilla.org/mozilla-central/rev/03fb2429290e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: