Closed Bug 644626 Opened 9 years ago Closed 9 years ago

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


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






(Reporter: jaas, Assigned: benjamin)



(Whiteboard: [unittest])


(3 files, 1 obsolete file)

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.
Specifically, the "mochitest-ipcplugins" tests currently hardcode modules/plugin/test and will need to be updated in sync with the move.
Where modules/plugin/test is used:
Whiteboard: [unittest]
When do you guys want this change to be done?
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?
Priority: -- → P5
(from a superficial look at the code) Can we shift ipcplugins into mochitests/ so we don't have to special case at ?

Seems odd that ipcplugins isn't a full target like chrome, browserchrome, a11y, etc.
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?
Whiteboard: [unittest] → [unittest][owner needed]
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.
Whiteboard: [unittest][owner needed] → [unittest][owner needed][triagefollowup]
Assignee: nobody → armenzg
Priority: P5 → P3
Whiteboard: [unittest][owner needed][triagefollowup] → [unittest]
I am ready to take care of this.

Could we coordinate this for Monday morning?
There are releases going on this week.
Attachment #528651 - Flags: review?(coop)
Attachment #528651 - Flags: review?(coop) → review+
Josh, does this week for you?
Whiteboard: [unittest] → [unittest] waiting on scheduling
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?
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!
Attachment #529801 - Flags: review?(ted.mielczarek)
We can test it on staging.
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.
Attachment #529801 - Flags: review?(ted.mielczarek) → review+
I would like to run it through staging to be sure.

I will get back to you with how it went.
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.
As noted on IRC, you have to use `hg import` instead of `patch` to apply this patch because it contains file moves/copies.
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?
Yes. Obviously the branch trees need to keep the old path.
Depends on: 656435
We are changing the approach of this by setting the correct test path on the code rather than on our configs (see bug 656435).
Attachment #528651 - Attachment is obsolete: true
Attachment #531725 - Flags: review?(coop)
Whiteboard: [unittest] waiting on scheduling → [unittest]
Priority: P3 → P2
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.
Attachment #531725 - Flags: review?(coop) → review+
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:
Attachment #531725 - Flags: checked-in+
This should be the patch that you need to land with attachment 529801 [details] [diff] [review].
Attachment #532345 - Flags: review?(benjamin)
Closed: 9 years ago
Resolution: --- → FIXED
Assignee: armenzg → nobody
Component: Release Engineering → Plug-ins
Flags: checked-in+
Product: → Core
QA Contact: release → plugins
Target Milestone: --- → mozilla6
Version: other → Trunk
Attachment #532345 - Flags: review?(benjamin)
Depends on: 658450
Assignee: nobody → benjamin
You need to log in before you can comment on or make changes to this bug.