Add a test for console grouping by clicking the group icon and check if messages are getting expanded/collapsed

RESOLVED FIXED in Firefox 59

Status

enhancement
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: abhinav.koppula, Assigned: abhinav.koppula)

Tracking

unspecified
Firefox 59
Points:
---

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Follow-up of https://bugzilla.mozilla.org/show_bug.cgi?id=1424690
Add a test to verify if message expanding is working properly.
Assignee: nobody → abhinav.koppula
Status: NEW → ASSIGNED
The TRY run on the patch looks good to me. So if you want to abhinav, you can copy what's in there that was missing in your patch, and push to review :)
Attachment #8939775 - Attachment is obsolete: true
Comment on attachment 8939925 [details]
Bug 1427006 - Enhanced the browser_webconsole_console_group test;

https://reviewboard.mozilla.org/r/210218/#review216120
Attachment #8939925 - Flags: review?(nchevobbe) → review+
TRY is fine, let's push
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/af69a7e1b59f
Enhanced the browser_webconsole_console_group test; r=nchevobbe
Clearing needinfo since Abhinav is working on a fix for this failure.
Flags: needinfo?(nchevobbe)
Hi Nicolas,
I have updated the patch a bit and TRY looks good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=45023344b8072b428c38cbf991612a07b6a3c0c1

Can you also check once from your side? If possible, can we push to autoland and see if there are any failures before actually pushing to mc?
Flags: needinfo?(nchevobbe)
Attachment #8939925 - Flags: review+ → review?(nchevobbe)
Comment on attachment 8939925 [details]
Bug 1427006 - Enhanced the browser_webconsole_console_group test;

https://reviewboard.mozilla.org/r/210218/#review218762

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_console_group.js:114
(Diff revision 2)
> +async function isOpen(node) {
> +  return node.classList.contains("open");
> +}

Is there a reason to make this function async ? 
It is only checking a class so it can be non-async I think. But I may miss something ?

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_console_group.js:118
(Diff revision 2)
> +async function testGroupToggle({ node, store, shouldBeOpen,
> +  visibleMessageIdsAfterExpand, visibleMessageIdsAfterCollapse }) {

nit: turn this into 
```js
async function testGroupToggle({ 
  node, 
  store, 
  shouldBeOpen,
  visibleMessageIdsAfterExpand,     
  visibleMessageIdsAfterCollapse 
}) {
```

so its more readable

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_console_group.js:133
(Diff revision 2)
> +  await isOpen(node);
> +  assertVisibleMessageIds(shouldBeOpen);
> +
> +  toggleArrow.click();
> +  shouldBeOpen = !shouldBeOpen;
> +  await isOpen(node);

I don't think this is what we want.
Here, unless I miss something, `isOpen` do not need to be async, since we only check a class on a node.

What we want insted, is to wait until the node changed, i.e. : 

```js
await waitFor(() => isOpen(node) === shouldBeOpen)
```

I think what you did fixed some failure on TRY because turning isOpen in a async function wraps everything woth promise, and maybe gives a little more time for the node to be updated, but this is still subject to intermittent failure in my opinion.
Attachment #8939925 - Flags: review?(nchevobbe) → review-
Flags: needinfo?(nchevobbe)
Comment on attachment 8939925 [details]
Bug 1427006 - Enhanced the browser_webconsole_console_group test;

https://reviewboard.mozilla.org/r/210218/#review218900

Looks good with a green TRY push :)
Attachment #8939925 - Flags: review?(nchevobbe) → review+
Hi Nicolas,
Pushed to TRY and it looks good to me.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d14ec85dbb410a8daa65b97743fc1025f831e63
Can you also confirm once?
Flags: needinfo?(nchevobbe)
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5a130721f8ea
Enhanced the browser_webconsole_console_group test; r=nchevobbe
Thanks Abhinav !
Flags: needinfo?(nchevobbe)
https://hg.mozilla.org/mozilla-central/rev/5a130721f8ea
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.