Closed Bug 1044197 Opened 8 years ago Closed 8 years ago

Don't ship sdk/test/httpd.js in omni.ja

Categories

(Add-on SDK Graveyard :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla35

People

(Reporter: rnewman, Assigned: zombie)

References

Details

Attachments

(1 file)

Particularly modules/commonjs/sdk/test, particularly on Fennec. Your call if you want to exclude it from desktop.
it looks like removing just the httpd.js will get us the biggest bang for the buck. it's 147k (out of 188k), and is probably the most specific/least used one.

i understand (from Jordan) that when we run tests for addons (like with jpm), we use the built-in modules from this dir (like harness), so it makes some sense to leave them, even for android.

but i think we can outright remove the httpd.js, even from desktop, as it's used only in a few tests, and they can just bring copies with them.

(i verified that none of the addons on AMO actually use this module, however unlikely that may be)

if this is the agreed upon solution, i can take this..
Assignee: nobody → tomica+amo
Status: NEW → ASSIGNED
But we use these modules in our tests. How do you plan to remove dependencies on them?
(In reply to Tomislav Jovanovic [:zombie] from comment #1)

> if this is the agreed upon solution, i can take this..

:thumbsup:

Thanks!
(In reply to Dave Townsend [:mossop] from comment #2)
> But we use these modules in our tests. How do you plan to remove
> dependencies on them?

jchen determined experimentally that this httpd.js wasn't used during ordinary test runs (Bug 1044079 Comment 3); IRC chatter suggested that something else was being used instead. So you might not have a problem to solve…
Oh during Fennec test runs currently not but that's because we don't currently run our tests on Fennec, something that I'd like to fix.
We'll need to fix that some way that doesn't involve shipping 150KB of test code to 70M devices. Maybe a "test services" add-on?
(In reply to Dave Townsend [:mossop] from comment #2)
> But we use these modules in our tests. How do you plan to remove
> dependencies on them?

i'm suggesting removing only httpd.js (biggest), and that one module is used only in a few tests, and we can move it to the addon-sdk/test/ dir to be used from there.

i should have been clearer, it's not exactly "removing", it's "moving" to the tests folder..
hey Jeff, was this discussed in the meeting? is this a go? or is there a better solution?
Flags: needinfo?(jgriffiths)
(In reply to Tomislav Jovanovic [:zombie] from comment #8)
> hey Jeff, was this discussed in the meeting? is this a go? or is there a
> better solution?

It was, and there is no reason we can think of to ship test code. People that needs the tests for a specific version can check out that tag on github, right?

Generally, we should try to help Fennec as much as possible here - rnewman thinks once they get into minifying code that there may be diminishing returns in removing more things but the tests are an obvious win.
Flags: needinfo?(jgriffiths)
(In reply to Tomislav Jovanovic [:zombie] from comment #7)
> (In reply to Dave Townsend [:mossop] from comment #2)
> > But we use these modules in our tests. How do you plan to remove
> > dependencies on them?
> 
> i'm suggesting removing only httpd.js (biggest), and that one module is used
> only in a few tests, and we can move it to the addon-sdk/test/ dir to be
> used from there.
> 
> i should have been clearer, it's not exactly "removing", it's "moving" to
> the tests folder..

You'll also need to copy it into the content-permissions and places test add-ons so we'll have duplicate versions which is unfortunate :(
> You'll also need to copy it into the content-permissions and places test
> add-ons so we'll have duplicate versions which is unfortunate :(

Or move all test support code to a test support add-on, no?
(In reply to Richard Newman [:rnewman] from comment #11)
> > You'll also need to copy it into the content-permissions and places test
> > add-ons so we'll have duplicate versions which is unfortunate :(
> 
> Or move all test support code to a test support add-on, no?

Probably more work
Priority: -- → P1
Summary: Don't ship SDK test code in omni.ja → Don't ship sdk/test/httpd.js in omni.ja
Attachment #8495161 - Flags: review?(zer0) → review+
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Depends on: 1079444
You need to log in before you can comment on or make changes to this bug.