1264804 - Close tabs and toolboxes between tasks in browser_webconsole_netlogging.js

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Description

[Brian Grinstead [:bgrins]]

For more context:

https://bugzilla.mozilla.org/show_bug.cgi?id=1242234#c16
https://bugzilla.mozilla.org/show_bug.cgi?id=1242234#c23

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: MozReview Request: Bug 1264804 - Clean up tabs between tests in browser_webconsole_netlogging.js;r=pbro

Also adds a new closeTabAndToolbox helper function to shared-head.js

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

Comment 2

[Brian Grinstead [:bgrins]]

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.

Comment 3

[Brian Grinstead [:bgrins]]

Try push:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=59fe70a90079

Comment 4

[Patrick Brosset <:pbro>]

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.

Comment 5

[Brian Grinstead [:bgrins]]

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/

Comment 6

[Brian Grinstead [:bgrins]]

Started using removeTab in both closeToolboxAndTab and closeTabAndToolbox

Comment 7

[Brian Grinstead [:bgrins]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b04f7a7444

Comment 8

[Brian Grinstead [:bgrins]]

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/

Comment 9

[Brian Grinstead [:bgrins]]

devtools tests look fine on the try push

Comment 10

[Carsten Book [:Tomcat]]

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 !

Comment 11

[Brian Grinstead [:bgrins]]

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/

Comment 12

[Brian Grinstead [:bgrins]]

Should be rebased now, thanks

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6582d0af6d89

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6582d0af6d89