Close tabs and toolboxes between tasks in browser_webconsole_netlogging.js

RESOLVED FIXED in Firefox 48

Status

defect
P3
normal
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

46 Branch
Firefox 48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

Patrick, I flagged you because of the new shared-head.js function.  I think it's pretty useful and was surprised that we didn't have it already but let me know what you think.
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Comment on attachment 8741549 [details]
MozReview Request: Bug 1264804 - Clean up tabs between tests in browser_webconsole_netlogging.js;r=pbro

https://reviewboard.mozilla.org/r/46563/#review43319

::: devtools/client/framework/test/shared-head.js:279
(Diff revision 1)
> +  let target = TargetFactory.forTab(gBrowser.selectedTab);
> +  if (target) {
> +    yield gDevTools.closeToolbox(target);
> +  }
> +
> +  gBrowser.removeCurrentTab();

Maybe call `removeTab` here instead, since we have this function available in the module. It seems to wait for some event, not sure if this is necessary.

::: devtools/client/framework/test/shared-head.js:286
(Diff revision 1)
> +
> +/**
>   * Close a toolbox and the current tab.
>   * @param {Toolbox} toolbox The toolbox to close.
>   * @return {Promise} Resolves when the toolbox and tab have been destroyed and
> - * closed.
> + *                   closed.

nit: you made a change here to vertically align "closed" below "Resolves", but you didn't do this in the jsdoc of the new `closeTabAndToolbox` function. Let's be consistent with the rest of the file.
Attachment #8741549 - Flags: review?(pbrosset) → review+
Comment on attachment 8741549 [details]
MozReview Request: Bug 1264804 - Clean up tabs between tests in browser_webconsole_netlogging.js;r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46563/diff/1-2/
Started using removeTab in both closeToolboxAndTab and closeTabAndToolbox
Comment on attachment 8741549 [details]
MozReview Request: Bug 1264804 - Clean up tabs between tests in browser_webconsole_netlogging.js;r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46563/diff/2-3/
devtools tests look fine on the try push
Keywords: checkin-needed
has conflicts summary:     Bug 1264804 - Clean up tabs between tests in browser_webconsole_netlogging.js;r=pbro

apply changeset? [ynmpcq?]: y
applying 23f9a79ec673
patching file devtools/client/framework/test/shared-head.js
Hunk #1 FAILED at 259
1 out of 1 hunks FAILED -- saving rejects to file devtools/client/framework/test/shared-head.js.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue

could you take a look, thanks !
Flags: needinfo?(bgrinstead)
Keywords: checkin-needed
Comment on attachment 8741549 [details]
MozReview Request: Bug 1264804 - Clean up tabs between tests in browser_webconsole_netlogging.js;r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46563/diff/3-4/
Should be rebased now, thanks
Flags: needinfo?(bgrinstead)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6582d0af6d89
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.