Last Comment Bug 644626 - move plugin tests from "/modules/plugin/test" to "/dom/plugins/test"
: move plugin tests from "/modules/plugin/test" to "/dom/plugins/test"
Status: RESOLVED FIXED
[unittest]
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla6
Assigned To: Benjamin Smedberg [:bsmedberg]
:
Mentors:
Depends on: 656435 658450
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-24 09:28 PDT by Josh Aas
Modified: 2011-06-12 02:44 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
change modules/plugin/test to dom/plugins/test (730 bytes, patch)
2011-04-27 11:35 PDT, Armen Zambrano [:armenzg] (EDT/UTC-4)
coop: review+
Details | Diff | Splinter Review
Code changes, rev. 1 (44.76 KB, patch)
2011-05-03 12:18 PDT, Benjamin Smedberg [:bsmedberg]
ted: review+
Details | Diff | Splinter Review
[buildbotcustom] call mochitest-ipcplugins with a --ipcplugins flag rather than with --test-path (785 bytes, patch)
2011-05-11 13:23 PDT, Armen Zambrano [:armenzg] (EDT/UTC-4)
coop: review+
Details | Diff | Splinter Review
change test path for ipcplugins (918 bytes, patch)
2011-05-13 14:08 PDT, Armen Zambrano [:armenzg] (EDT/UTC-4)
no flags Details | Diff | Splinter Review

Description Josh Aas 2011-03-24 09:28:50 PDT
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 [:bsmedberg] 2011-03-24 09:32:15 PDT
Specifically, the "mochitest-ipcplugins" tests currently hardcode modules/plugin/test and will need to be updated in sync with the move.
Comment 2 Armen Zambrano [:armenzg] (EDT/UTC-4) 2011-03-24 10:20:49 PDT
Where modules/plugin/test is used:
http://mxr.mozilla.org/build/source/buildbotcustom/steps/unittest.py#262
Comment 3 Armen Zambrano [:armenzg] (EDT/UTC-4) 2011-03-24 10:39:47 PDT
When do you guys want this change to be done?
Comment 4 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2011-03-24 14:00:14 PDT
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] 2011-03-24 14:17:39 PDT
(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 2011-04-01 16:27:52 PDT
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 2011-04-18 12:52:28 PDT
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 Zambrano [:armenzg] (EDT/UTC-4) 2011-04-27 11:35:49 PDT
Created attachment 528651 [details] [diff] [review]
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 Zambrano [:armenzg] (EDT/UTC-4) 2011-05-02 07:06:05 PDT
Josh, does this week for you?
Comment 10 Josh Aas 2011-05-03 07:24:50 PDT
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 [:bsmedberg] 2011-05-03 12:18:29 PDT
Created attachment 529801 [details] [diff] [review]
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 Zambrano [:armenzg] (EDT/UTC-4) 2011-05-03 13:13:30 PDT
We can test it on staging.
Comment 13 Benjamin Smedberg [:bsmedberg] 2011-05-04 09:40:35 PDT
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 Zambrano [:armenzg] (EDT/UTC-4) 2011-05-04 11:59:06 PDT
I would like to run it through staging to be sure.

I will get back to you with how it went.
Comment 15 Armen Zambrano [:armenzg] (EDT/UTC-4) 2011-05-09 12:26:21 PDT
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 [:bsmedberg] 2011-05-09 12:43:34 PDT
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 Zambrano [:armenzg] (EDT/UTC-4) 2011-05-10 08:18:53 PDT
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 [:bsmedberg] 2011-05-10 09:38:14 PDT
Yes. Obviously the branch trees need to keep the old path.
Comment 19 Armen Zambrano [:armenzg] (EDT/UTC-4) 2011-05-11 13:23:37 PDT
Created attachment 531725 [details] [diff] [review]
[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] 2011-05-13 06:51:02 PDT
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 Zambrano [:armenzg] (EDT/UTC-4) 2011-05-13 07:30:57 PDT
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 Zambrano [:armenzg] (EDT/UTC-4) 2011-05-13 14:08:12 PDT
Created attachment 532345 [details] [diff] [review]
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 [:bsmedberg] 2011-05-18 08:20:50 PDT
http://hg.mozilla.org/mozilla-central/rev/f5ac9367f1e1
Comment 24 :Ms2ger (⌚ UTC+1/+2) 2011-05-19 02:25:03 PDT
http://hg.mozilla.org/mozilla-central/rev/1f3777d4ed8b

Note You need to log in before you can comment on or make changes to this bug.