1181838 - Migrate devtools/client/debugger tests to shared-head.js

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Brian Grinstead [:bgrins]]

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

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: MozReview Request: Bug 1181838 - Convert debugger tests to use shared-head.js;r=jlongster

Bug 1181838 - Convert debugger tests to use shared-head.js;r=jlongster

Comment 2

[Brian Grinstead [:bgrins]]

There were a couple of special things with the debugger tests, but not as many as I thought.  Specifically, they use a different promise implementation and have some custom addTab / removeTab behavior.

Comment 3

[James Long (:jlongster)]

Comment on attachment 8682619 [details]
MozReview Request: Bug 1181838 - Convert debugger tests to use shared-head.js;r=jlongster

https://reviewboard.mozilla.org/r/24115/#review22381

Looks great, sorely needed.

::: devtools/client/framework/test/shared-head.js:23
(Diff revision 1)
> -const promise = require("promise");
> +let promise = require("promise");

Why the change to `let`? I've seen const->var so that it exists in the global scope. Can this be set later on in the file?

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to James Long (:jlongster) from comment #3)
> Comment on attachment 8682619 [details]
> MozReview Request: Bug 1181838 - Convert debugger tests to use
> shared-head.js;r=jlongster
> 
> https://reviewboard.mozilla.org/r/24115/#review22381
> 
> Looks great, sorely needed.
> 
> ::: devtools/client/framework/test/shared-head.js:23
> (Diff revision 1)
> > -const promise = require("promise");
> > +let promise = require("promise");
> 
> Why the change to `let`? I've seen const->var so that it exists in the
> global scope. Can this be set later on in the file?

No reason in particular.  It seems to work fine (probably something with how the script loader works): https://treeherder.mozilla.org/#/jobs?repo=try&revision=20bffb29aeaa

Comment 5

[Brian Grinstead [:bgrins]]

To be clear, it had to change from const because the debugger tests would throw an error when trying to override it with its own promise implementation if it was a const.  When it's let (or var, I'm sure) the error doesn't happen and all is well.

Comment 6

[Pulsebot]

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

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/9f30981c1fae