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

Status: RESOLVED FIXED

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

Description

[Richard Newman [:rnewman]]

Particularly modules/commonjs/sdk/test, particularly on Fennec. Your call if you want to exclude it from desktop.

Comment 1

[Tomislav Jovanovic :zombie]

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..

Comment 2

[Dave Townsend [:mossop]]

But we use these modules in our tests. How do you plan to remove dependencies on them?

Comment 3

[Richard Newman [:rnewman]]

(In reply to Tomislav Jovanovic [:zombie] from comment #1)

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

:thumbsup:

Thanks!

Comment 4

[Richard Newman [:rnewman]]

(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…

Comment 5

[Dave Townsend [:mossop]]

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.

Comment 6

[Richard Newman [:rnewman]]

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?

Comment 7

[Tomislav Jovanovic :zombie]

(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..

Comment 8

[Tomislav Jovanovic :zombie]

hey Jeff, was this discussed in the meeting? is this a go? or is there a better solution?

Comment 9

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(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.

Comment 10

[Dave Townsend [:mossop]]

(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 :(

Comment 11

[Richard Newman [:rnewman]]

> 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?

Comment 12

[Dave Townsend [:mossop]]

(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

Comment 13

[Tomislav Jovanovic :zombie]

Attachment: Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1650

Comment 14

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/6a1e86ad9983a8c5206e300188ce51125c980405
bug 1044197 - don't ship httpd.js

https://github.com/mozilla/addon-sdk/commit/d0430978259a7b9bbe664ac4ef552d439b6736a4
Merge pull request #1650 from zombie/1044197-dont-ship-httpd

bug 1044197 - don't ship httpd.js, r=@ZER0