1181852 - Migrate devtools/client/shared tests to shared-head.js

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[Brian Grinstead [:bgrins]]

We should import shared-head.js in shared/test/head.js and remove any newly unnecessary code after that.

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: MozReview Request: Bug 1181852 - Use shared-head.js for devtools/client/shared;r=jryans

Bug 1181852 - Use shared-head.js for devtools/client/shared;r=jryans

Comment 2

[Brian Grinstead [:bgrins]]

Comment on attachment 8685239 [details]
MozReview Request: Bug 1181852 - Use shared-head.js for devtools/client/shared;r=jryans

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24765/diff/1-2/

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #1)
> Created attachment 8685239 [details]
> MozReview Request: Bug 1181852 - Use shared-head.js for
> devtools/client/shared;r=jryans
> 
> Bug 1181852 - Use shared-head.js for devtools/client/shared;r=jryans

Most of the diff is replacing promiseTab with addTab.. The interesting parts are in head.js and then a few tests that were using the old addTab implementation that took in a callback.  For a couple I've converted to add_task, and one was too scary to touch so I just changed it to be addTab(..).then(callback).

Sorry for the size of the diff, I realize now that I should have done the promiseTab part in another step.

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8685239 [details]
MozReview Request: Bug 1181852 - Use shared-head.js for devtools/client/shared;r=jryans

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24765/diff/2-3/

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: MozReview Request: Bug 1181852 - Replace all instances of promiseTab with addTab (just to make review easier, this will be squashed);r=jryans

Bug 1181852 - Replace all instances of promiseTab with addTab (just to make review easier, this will be squashed);r=jryans

Comment 6

[Brian Grinstead [:bgrins]]

There we go, I've split the big find/replace out into the second patch so hopefully the first is easier to review

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8685239 [details]
MozReview Request: Bug 1181852 - Use shared-head.js for devtools/client/shared;r=jryans

https://reviewboard.mozilla.org/r/24765/#review22331

::: devtools/client/shared/test/head.js:95
(Diff revision 3)
>  function oneTimeObserve(name, callback) {

Should we extend `once` in shared head to support observers so this can be removed?

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8685242 [details]
MozReview Request: Bug 1181852 - Replace all instances of promiseTab with addTab (just to make review easier, this will be squashed);r=jryans

https://reviewboard.mozilla.org/r/24771/#review22333

Comment 9

[Brian Grinstead [:bgrins]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #7)
> Comment on attachment 8685239 [details]
> MozReview Request: Bug 1181852 - Use shared-head.js for
> devtools/client/shared;r=jryans
> 
> https://reviewboard.mozilla.org/r/24765/#review22331
> 
> ::: devtools/client/shared/test/head.js:95
> (Diff revision 3)
> >  function oneTimeObserve(name, callback) {
> 
> Should we extend `once` in shared head to support observers so this can be
> removed?

We discussed on #devtools and will wait to see if there is another directory that uses something similar as part of Bug 1181833.. If so we'll pull that into the shared file (either by passing Services.obs as a param to once or as a new function)

Comment 10

[Brian Grinstead [:bgrins]]

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ded3a51b9cac

Comment 11

[Pulsebot]

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

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f65cfe891fe8