Closed Bug 1406028 Opened 7 years ago Closed 6 years ago

Migrate browser_webconsole_console_trace_duplicates.js to the 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

(Blocks 1 open bug)

Details

(Whiteboard: [newconsole-mvp])

Attachments

(1 file)

      No description provided.
Priority: -- → P3
Priority: P3 → P2
Whiteboard: [newconsole-mvp]
Assignee: nobody → mratcliffe
Status: NEW → ASSIGNED
Has Regression Range: --- → irrelevant
Has STR: --- → irrelevant
OS: Unspecified → All
Hardware: Unspecified → All
Priority: P2 → P1
Comment on attachment 8950003 [details]
Bug 1406028 - Migrate browser_webconsole_console_trace_duplicates.js to the new frontend

https://reviewboard.mozilla.org/r/219282/#review225128

This bug was initially intended to migrated the old mochitest to mocha. As discussed on #console with Nicolas, let's land this but rename the bug and have a follow up to actually migrate this to mocha.

Otherwise some nits, but looks good.

::: commit-message-a8e15:1
(Diff revision 1)
> +Bug 1406028 - Migrate browser_webconsole_console_trace_duplicates.js to mocha r?jdescottes

The test is not migrated to mocha here.

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_console_trace_duplicates.js:12
(Diff revision 1)
> +
> +"use strict";
> +
> +const TEST_URI = "http://example.com/browser/devtools/client/webconsole/" +
> +                  "new-console-output/test/mochitest/" +
> +                  "test-bug_939783_console_trace_duplicates.html";

As we go we are trying to rename the support files in order to remove the "bug_xxxxxx" part from their filenames and to mirror the test name. 

So here that would be test-console-trace-duplicates.html

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:579
(Diff revision 1)
> + *                ...
> + *              ]
> + *            }
> + */
> +function getStackInfo(message) {
> +  const getLineAndColumn = node => {

nit: Can we move this method out of getStackInfo, it doesn't use anything from this context.

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:584
(Diff revision 1)
> +    if (lineAndColumn.startsWith(":")) {
> +      lineAndColumn = lineAndColumn.substr(1);
> +
> +      let lc = lineAndColumn.split(":");
> +      line = lc[0];
> +      column = lc[1];
> +    }

nit: A comment with an example of line being split would help here. "http://url.of/file.js:10:15", and another for the case with startsWith(":") ?

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:587
(Diff revision 1)
> +      let lc = lineAndColumn.split(":");
> +      line = lc[0];
> +      column = lc[1];

nit: can we do 

[line, column] = lineAndColumn.split(":");

instead (shorter, and the variable name "lc" is used for a different kind of object a few lines below)

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:593
(Diff revision 1)
> +      line: line,
> +      column: column

nit: Convert to integers here rather than doing it in all consumers of the API?

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:620
(Diff revision 1)
> +      if (!result.stack) {
> +        result.stack = [];
> +      }

nit: could initialize stack to [] when creating result = {} and get rid of this if.
Attachment #8950003 - Flags: review?(jdescottes) → review+
Comment on attachment 8950003 [details]
Bug 1406028 - Migrate browser_webconsole_console_trace_duplicates.js to the new frontend

https://reviewboard.mozilla.org/r/219282/#review225138

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:553
(Diff revision 1)
>      return selected && selected.url === url;
>    });
>  
>    ok(true, "The attached url is correct.");
>  }
> +

The new test helpers are specific to the new test, let's keep them in the test file for now. They will move to head js if they are needed by more than one test.
Summary: Migrate browser_webconsole_console_trace_duplicates.js to mocha → Migrate browser_webconsole_console_trace_duplicates.js to the new frontend
Comment on attachment 8950003 [details]
Bug 1406028 - Migrate browser_webconsole_console_trace_duplicates.js to the new frontend

https://reviewboard.mozilla.org/r/219282/#review225128

> The test is not migrated to mocha here.

We will migrate to mocha in bug 1437469
Pushed by mratcliffe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/955d0aaa5846
Migrate browser_webconsole_console_trace_duplicates.js to the new frontend r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/955d0aaa5846
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: