Closed
Bug 844865
Opened 12 years ago
Closed 9 years ago
Update library modules to be ready for the removal of Panorama
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect, P3)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: whimboo, Unassigned)
References
Details
(Whiteboard: [lib] s=130225 u=failure c=lib p=1)
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
AndreeaMatei
:
review+
|
Details | Diff | Splinter Review |
Based on the work on bug 836758 our tests will be broken soon given that some of the lookup strings we make use of in our libs can't be resolved anymore. We should have a patch ready to land whenever the patch on bug 836758 lands on central.
This has to be our P1 this week and needs to be fixed ASAP.
Updated•12 years ago
|
Whiteboard: [lib] s=130121 u=failure c=lib p=1 → [lib] s=130221 u=failure c=lib p=1
Updated•12 years ago
|
Assignee: nobody → andrei.eftimie
Status: NEW → ASSIGNED
Updated•12 years ago
|
Whiteboard: [lib] s=130221 u=failure c=lib p=1 → [lib] s=130225 u=failure c=lib p=1
Comment 1•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #0)
> This has to be our P1 this week and needs to be fixed ASAP.
I don't think bug 836758 will be landing this week, we still have to figure out what the transition plan will be.
Reporter | ||
Comment 2•12 years ago
|
||
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #1)
> I don't think bug 836758 will be landing this week, we still have to figure
> out what the transition plan will be.
We want to be prepared for this change and do not have to suffer from failures for 1 or 2 weeks if we weren't able to land our changes in time. Having a patch ready we can land alongside the patch on bug 836758 will make our test results stay green. So once ready we will wait for sure, it's not a problem for us when Panorama gets removed.
Comment 3•12 years ago
|
||
Part 1 - removing all references to tabView (panorama)
This patch will have our testrun working correctly after the tabview
functionality is taken out of FF
Functional testrun: http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4c139c6f
Endurance testrun: http://mozmill-crowd.blargon7.com/#/endurance/report/66aad0ebfa7a01580628b3af4c13e47a
*These testruns are on the latests Nightly Builds (which still have tabView
funct). The patches do not longer apply cleanly, so its a bit more work to get a
build with the funct removed. I am about 95% sure this will work, but we should
have a testrun on a build with these removed. working on this.
** a Part 2 will follow which will reenable our functional tests as addon tests
for the new Addon that assumes the removed functionality.
Attachment #718893 -
Flags: review?(andreea.matei)
Reporter | ||
Comment 4•12 years ago
|
||
I don't think we should remove the lib but move it into the addons folder given that we want to run add-on tests for the new addon. That will keep us the history of the file.
Comment 5•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #4)
> I don't think we should remove the lib but move it into the addons folder
> given that we want to run add-on tests for the new addon. That will keep us
> the history of the file.
I agree with keeping the history for the file.
We will not be able to have a complete Addon test until the addon is finished and available.
I will leave patches in 2 parts so we can quickly disable these tests when the tabView removal lands, but I will work on having them merged. Hopefully we can have them ready at the same time. (Maybe we should just skip the tests if we need to disable them and don't have the Addons part ready).
I now have a build with the tabView removed, and the Part 1 patch is working fine:
# FF build with tabView removed
## unpatched testsuite
functional: http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4c176da8
endurance: http://mozmill-crowd.blargon7.com/#/endurance/report/66aad0ebfa7a01580628b3af4c1a07f8
## patched testsuite
functional: http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4c1c4a14
endurance: http://mozmill-crowd.blargon7.com/#/endurance/report/66aad0ebfa7a01580628b3af4c1ae8c7
Working on getting the Addons patch ready.
One thing that we will need is a download location for the xpi.
Reporter | ||
Comment 6•12 years ago
|
||
Tim could most likely point us to a current version of the addon we could use for testing purposes.
Comment 7•12 years ago
|
||
It seems I was underconfident: here is a fully working patch.
We are removing the endurance tests, and migrating the functional tests to an Addon suite.
For an addons testrun to pass, you will need a patched build of FF with the tabView functionality removed. (It will fail with the latest Nightly)
I am only asking for feedback here because we still need the canonical location of the xpi. ATM I have forked the addon repo, added the xpi and linked it directly from github.
Attachment #718893 -
Attachment is obsolete: true
Attachment #718893 -
Flags: review?(andreea.matei)
Attachment #719046 -
Flags: feedback?(hskupin)
Attachment #719046 -
Flags: feedback?(dave.hunt)
Attachment #719046 -
Flags: feedback?(andreea.matei)
Comment 8•12 years ago
|
||
*Note*
We might use a local Addon for this, but it will require someone to maintain/update it regularly.
I would prefer to use the canonical xpi location (I am guessing it will eventually be in addons.mozilla.org)
We can also land this patch as-is, as the xpi is hosted on github atm, but we will need to update the location later on.
Comment 9•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #6)
> Tim could most likely point us to a current version of the addon we could
> use for testing purposes.
Here it is: https://github.com/ttaubert/firefox-tabgroups
This add-on will interfere with the current Panorama if you add it to a build without the patches that remove Panorama.
Reporter | ||
Comment 10•12 years ago
|
||
Comment on attachment 719046 [details] [diff] [review]
moveTabViewTestToAddon_v1
Review of attachment 719046 [details] [diff] [review]:
-----------------------------------------------------------------
Looks fine. Please address the items I have mentioned.
::: tests/addons/firefox-tabgroups@mozilla.com/addon.ini
@@ +1,4 @@
> +[download]
> +linux=https://github.com/andreieftimie/firefox-tabgroups/blob/master/firefox-tabgroups.xpi?raw=true
> +mac=https://github.com/andreieftimie/firefox-tabgroups/blob/master/firefox-tabgroups.xpi?raw=true
> +win=https://github.com/andreieftimie/firefox-tabgroups/blob/master/firefox-tabgroups.xpi?raw=true
Please update to the link Tim has given. Once it's on AMO we will use the final URL.
::: lib/tabview.js
@@ +432,5 @@
> *
> * @returns Array of external DTD urls
> * @type [string]
> */
> getDtds : function tabView_getDtds() {
Hm, any reason why we have a getter called 'dtds' and this method in the same class?
::: tests/addons/firefox-tabgroups@mozilla.com/manifest.ini
@@ +1,1 @@
> +[include:tests/manifest.ini]
I miss this manifest in the patch. Also you want to remove it from the functional tests manifest.
Attachment #719046 -
Flags: feedback?(hskupin)
Attachment #719046 -
Flags: feedback?(dave.hunt)
Attachment #719046 -
Flags: feedback?(andreea.matei)
Attachment #719046 -
Flags: feedback+
Comment 11•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #10)
> Please update to the link Tim has given. Once it's on AMO we will use the
> final URL.
Tim's repo does not contain the xpi file.
I will ask Tim to see if he'll add the xpi to the repo so we can use it directly
> ::: lib/tabview.js
> @@ +432,5 @@
> > *
> > * @returns Array of external DTD urls
> > * @type [string]
> > */
> > getDtds : function tabView_getDtds() {
>
> Hm, any reason why we have a getter called 'dtds' and this method in the
> same class?
I have not questioned the library code.
It does seem weird as the getter and the getDtd method do not return the same
thing.
> ::: tests/addons/firefox-tabgroups@mozilla.com/manifest.ini
> @@ +1,1 @@
> > +[include:tests/manifest.ini]
>
> I miss this manifest in the patch. Also you want to remove it from the
> functional tests manifest.
1. I have removed the orphaned manifest include.
2. The "tests/manifest.ini" file is in the patch (check line 143 and 144).
You might have missed it because it is just a rename.
Attachment #719046 -
Attachment is obsolete: true
Comment 12•12 years ago
|
||
Tim, would you please add the firefox-tabgroups addon xpi file to the github repo.
We need a location from which to download and install it directly from our test suitel and untill it is published on AMO this is probably the best location to serve it from.
Thanks
Flags: needinfo?(ttaubert)
Comment 13•12 years ago
|
||
(In reply to Andrei Eftimie from comment #12)
> Tim, would you please add the firefox-tabgroups addon xpi file to the github
> repo.
Done.
Flags: needinfo?(ttaubert)
Comment 14•12 years ago
|
||
Added correct xpi download location.
Attachment #719921 -
Attachment is obsolete: true
Attachment #719925 -
Flags: review?(andreea.matei)
Comment 15•12 years ago
|
||
Comment on attachment 719925 [details] [diff] [review]
moveTabViewTestToAddon_v3
Review of attachment 719925 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good.
I feel though that getter get dtds() and method detDtds in tabview.js class are really strange, giving that the dtd from one is repeated in the second. Maybe we should file a bug to clean that up, it would be a candidate for contributors.
Attachment #719925 -
Flags: review?(andreea.matei) → review+
Comment 16•11 years ago
|
||
This has been covered already and until the removal happens for landing, no need to keep as P1.
Priority: P1 → P3
Comment 17•10 years ago
|
||
Unassigning. We'll revisit when the actual changes are done.
Assignee: andrei.eftimie → nobody
Status: ASSIGNED → NEW
Reporter | ||
Comment 18•9 years ago
|
||
Panorama will be dead soon - and Mozmill tests are dead for a while now. Nothing we want to spend any time in getting it updated or removed.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•