644626 - move plugin tests from "/modules/plugin/test" to "/dom/plugins/test"

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Josh Aas]

We want to move plugin tests from "/modules/plugin/test" to "/dom/plugins/test". I believe we'll need to coordinate with releng on this.

Comment 1

[Benjamin Smedberg]

Specifically, the "mochitest-ipcplugins" tests currently hardcode modules/plugin/test and will need to be updated in sync with the move.

Comment 2

[Armen [:armenzg]]

Where modules/plugin/test is used:
http://mxr.mozilla.org/build/source/buildbotcustom/steps/unittest.py#262

Comment 3

[Armen [:armenzg]]

When do you guys want this change to be done?

Comment 4

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Armen: per convo with Josh on the walk into office this morning, this is housekeeping, not urgent, and is planned to happen after branch off from m-c to create FF5.0. I believe this change will *not* be backported to mozilla-2.0, mozilla-1.9.*, etc. This bug is to track any RelEng changes that we need to make at same time as Josh moves the code within the m-c repo.


(In reply to comment #1)
> Specifically, the "mochitest-ipcplugins" tests currently hardcode
> modules/plugin/test and will need to be updated in sync with the move.


bsmedberg/josh: can you file dep bugs with QA to remove any hardcoded strings like this?

Comment 5

[Nick Thomas [:nthomas] (UTC+12)]

(from a superficial look at the code) Can we shift ipcplugins into mochitests/runtests.py so we don't have to special case at
  http://mxr.mozilla.org/build/source/buildbotcustom/steps/unittest.py#259 ?

Seems odd that ipcplugins isn't a full target like chrome, browserchrome, a11y, etc.

Comment 6

[Josh Aas]

This doesn't have to be done for the current release cycle but it could be done in this release cycle. It's not really a matter of risk, just developer time.

What is the lowest effort solution to get the directory move done? Seems like just changing the hardcoded path in the buildbot code would be easiest, anything further can be in followup bugs. Who can make the buildbot change, and how do we make that take effect?

Comment 7

[Josh Aas]

Trunk is open for FF6 and we've started the plugin code re-org. Would be great to get an owner and get this done. Thanks.

Comment 8

[Armen [:armenzg]]

Attachment: change modules/plugin/test to dom/plugins/test

I am ready to take care of this.

Could we coordinate this for Monday morning?
There are releases going on this week.

Comment 9

[Armen [:armenzg]]

Josh, does this week for you?

Comment 10

[Josh Aas]

This week is not good for me if you need me to do something. I am traveling all week. Perhaps Benjamin Smedberg could commit any necessary changes to the tree?

Comment 11

[Benjamin Smedberg]

Attachment: Code changes, rev. 1

These are the code moves that should go with this. It's going to be a bit hard to test this before it goes live, but c'est la vie!

Comment 12

[Armen [:armenzg]]

We can test it on staging.

Comment 13

[Benjamin Smedberg]

revision 05396de20219 on try passed everything except for mochitest-ipcplugins and known random orange. I'm happy to either land this directly at some point or do a staging run with that revision.

Comment 14

[Armen [:armenzg]]

I would like to run it through staging to be sure.

I will get back to you with how it went.

Comment 15

[Armen [:armenzg]]

Is the patch still working?

I tried pushing it to the try server (as it wasn't compiling on staging) and I got all red.

Maybe I have done something wrong.

http://tbpl.mozilla.org/?tree=Try&rev=87ac1e3e5bc5
http://hg.mozilla.org/try/rev/87ac1e3e5bc5

Comment 16

[Benjamin Smedberg]

As noted on IRC, you have to use `hg import` instead of `patch` to apply this patch because it contains file moves/copies.

Comment 17

[Armen [:armenzg]]

bsmedberg IIUC other trees will go orange for mochitest-other because ipcplugin runs inside of it until they pull from mozilla-central; is this correct?

Comment 18

[Benjamin Smedberg]

Yes. Obviously the branch trees need to keep the old path.

Comment 19

[Armen [:armenzg]]

Attachment: [buildbotcustom] call mochitest-ipcplugins with a --ipcplugins flag rather than with --test-path

We are changing the approach of this by setting the correct test path on the code rather than on our configs (see bug 656435).

Comment 20

[Chris Cooper [:coop] (he/him)]

Comment on attachment 531725 [details] [diff] [review]
[buildbotcustom] call mochitest-ipcplugins with a --ipcplugins flag rather than with --test-path

Can we put the elif back in, please? I think the structure is cleaner with if-elif-else rather than two ifs.

Comment 21

[Armen [:armenzg]]

Comment on attachment 531725 [details] [diff] [review]
[buildbotcustom] call mochitest-ipcplugins with a --ipcplugins flag rather than with --test-path

Landed and merged into production branch:
http://hg.mozilla.org/build/buildbotcustom/rev/19c634c3114a

Comment 22

[Armen [:armenzg]]

Attachment: change test path for ipcplugins

This should be the patch that you need to land with attachment 529801 [details] [diff] [review].

Comment 23

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/f5ac9367f1e1

Comment 24

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/mozilla-central/rev/1f3777d4ed8b