Closed Bug 776518 Opened 8 years ago Closed 8 years ago

Move Require.jsm somewhere into toolkit to fix comm-central's busted XPCShell tests.

Categories

(DevTools :: General, defect)

x86
All
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 17

People

(Reporter: mconley, Assigned: miker)

References

Details

(Whiteboard: [landed-in-fx-team])

Attachments

(2 files, 1 obsolete file)

Some of toolkit's XPCShell tests now require toolkit/devtools/sourcemap/tests/unit/Utils.jsm, and Utils.jsm in turn requires Require.jsm.

Unfortunately, Require.jsm is only available to browser, since it's under browser/devtools/shared.

This means that comm-central's XPCShell tests are busted (since we run toolkits XPCShell tests along with our own), and our tree is very orange.

Can we move Require.jsm and dependencies somewhere into toolkit?
Version: unspecified → Trunk
Assignee: nobody → mratcliffe
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
Moved to Require.jsm to toolkit as requested
Attachment #644947 - Flags: review?(rcampbell)
http://tbpl.mozilla.org/?tree=Try&rev=af8f536b5b46

This will work as long as we are not debugging dependencies. We could also move Console.jsm but that would be robcee's call.
(In reply to Mike Conley (:mconley) from comment #0)
> Some of toolkit's XPCShell tests now require
> toolkit/devtools/sourcemap/tests/unit/Utils.jsm, and Utils.jsm in turn
> requires Require.jsm.

Why?
Comment on attachment 644947 [details] [diff] [review]
Patch

you should change importers of Require.jsm to resource://gre/modules/Require.jsm and update the jar file accordingly.
Attachment #644947 - Flags: review?(rcampbell) → review-
(In reply to Rob Campbell [:rc] (:robcee) from comment #3)
> (In reply to Mike Conley (:mconley) from comment #0)
> > Some of toolkit's XPCShell tests now require
> > toolkit/devtools/sourcemap/tests/unit/Utils.jsm, and Utils.jsm in turn
> > requires Require.jsm.
> 
> Why?

disregard that. Dave helped me understand that we're blowing up your tests.
Can we just put the module directly under toolkit/devtools, rather than in its own directory? Flatter directory structures are better!
(In reply to Rob Campbell [:rc] (:robcee) from comment #4)
> Comment on attachment 644947 [details] [diff] [review]
> Patch
> 
> you should change importers of Require.jsm to
> resource://gre/modules/Require.jsm and update the jar file accordingly.

No jar involved but done.

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #6)
> Can we just put the module directly under toolkit/devtools, rather than in
> its own directory? Flatter directory structures are better!

Done
Attachment #644947 - Attachment is obsolete: true
Attachment #645249 - Flags: review?(rcampbell)
Comment on attachment 645249 [details] [diff] [review]
Patch II - the sequel

I'm gonna r+ to get Tbird back up and running, but I think we should probably have gre/modules/devtools as the path. I will update and land.
Attachment #645249 - Flags: review?(rcampbell) → review+
Attached patch require-fixSplinter Review
Attachment #645522 - Flags: review?(gavin.sharp)
Comment on attachment 645522 [details] [diff] [review]
require-fix

follow-up fix
Attachment #645522 - Flags: review?(gavin.sharp)
https://hg.mozilla.org/mozilla-central/rev/35bc0dd4080c
https://hg.mozilla.org/mozilla-central/rev/40bfa9aa4ab0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.