Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED in mozilla6

Status

()

Core
Plug-ins
P2
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Josh Aas, Assigned: bsmedberg)

Tracking

Trunk
mozilla6
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [unittest])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.
(Assignee)

Comment 1

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

Comment 2

6 years ago
Where modules/plugin/test is used:
http://mxr.mozilla.org/build/source/buildbotcustom/steps/unittest.py#262
Whiteboard: [unittest]

Comment 3

6 years ago
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/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.
(Reporter)

Comment 6

6 years ago
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?

Updated

6 years ago
Whiteboard: [unittest] → [unittest][owner needed]
(Reporter)

Comment 7

6 years ago
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.

Updated

6 years ago
Whiteboard: [unittest][owner needed] → [unittest][owner needed][triagefollowup]

Updated

6 years ago
Assignee: nobody → armenzg
Priority: P5 → P3
Whiteboard: [unittest][owner needed][triagefollowup] → [unittest]

Comment 8

6 years ago
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.
Attachment #528651 - Flags: review?(coop)

Updated

6 years ago
Attachment #528651 - Flags: review?(coop) → review+

Comment 9

6 years ago
Josh, does this week for you?
Whiteboard: [unittest] → [unittest] waiting on scheduling
(Reporter)

Comment 10

6 years ago
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?
(Assignee)

Comment 11

6 years ago
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!
Attachment #529801 - Flags: review?(ted.mielczarek)

Comment 12

6 years ago
We can test it on staging.
(Assignee)

Comment 13

6 years ago
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+

Comment 14

6 years ago
I would like to run it through staging to be sure.

I will get back to you with how it went.

Comment 15

6 years ago
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
(Assignee)

Comment 16

6 years ago
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

6 years ago
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?
(Assignee)

Comment 18

6 years ago
Yes. Obviously the branch trees need to keep the old path.

Updated

6 years ago
Depends on: 656435

Comment 19

6 years ago
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).
Attachment #528651 - Attachment is obsolete: true
Attachment #531725 - Flags: review?(coop)

Updated

6 years ago
Whiteboard: [unittest] waiting on scheduling → [unittest]

Updated

6 years ago
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 21

6 years ago
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
Attachment #531725 - Flags: checked-in+

Comment 22

6 years ago
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].
Attachment #532345 - Flags: review?(benjamin)
(Assignee)

Comment 23

6 years ago
http://hg.mozilla.org/mozilla-central/rev/f5ac9367f1e1
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Assignee: armenzg → nobody
Component: Release Engineering → Plug-ins
Flags: checked-in+
Product: mozilla.org → Core
QA Contact: release → plugins
Target Milestone: --- → mozilla6
Version: other → Trunk
(Assignee)

Updated

6 years ago
Attachment #532345 - Flags: review?(benjamin)
http://hg.mozilla.org/mozilla-central/rev/1f3777d4ed8b
Depends on: 658450

Updated

6 years ago
Assignee: nobody → benjamin
You need to log in before you can comment on or make changes to this bug.