Closed
Bug 578343
Opened 13 years ago
Closed 13 years ago
talos needs to be retrofitted for e10s
Categories
(Testing :: Talos, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jmaher, Assigned: jmaher)
References
Details
Attachments
(2 files, 3 obsolete files)
currently talos uses quit.js and this doesn't work when running inside of e10s. We need to look through all the talos code and make sure we proxy everything to chrome process.
Updated•13 years ago
|
Blocks: mobile-pool
Comment 1•13 years ago
|
||
Is there an equivalent bug for unit tests? Looks like we have false greens on maemo devices on unit tests (bug 579184); the browser isn't actually launched.
Assignee | ||
Comment 2•13 years ago
|
||
this patch requires the mobile_profile.zip (which I will upload next) and works on fennec desktop linux. This changes the profile for mobile from base_profile to mobile_profile and mobile_profile has an extension which adds an overlay (same technique as mochitest)
Assignee: nobody → jmaher
Attachment #461104 -
Flags: review?(aki)
Assignee | ||
Comment 3•13 years ago
|
||
this is the new profile directory.
Attachment #461106 -
Flags: review?(aki)
Comment 4•13 years ago
|
||
Comment on attachment 461106 [details]
the new mobile_profile with the talos e10s extension (1.0)
I'll be staging this and the other patches shortly.
One thing about this zip -- you included the CVS subdirectories, which means any changes or cvs actions on this directory tree will affect the directory you copied it from (base_profile) instead of the new subdirectory tree you want to create. So you want to remove mobile_profile/bookmarkbackups/CVS, mobile_profile/Cache/CVS and mobile_profile/CVS.
Updated•13 years ago
|
Attachment #461106 -
Attachment is patch: false
Attachment #461106 -
Attachment mime type: text/plain → application/octet-stream
Comment 5•13 years ago
|
||
Comment on attachment 461104 [details] [diff] [review] core changes required to get ts, twinopen working on talos (1.0) Things are looking good here, other than fennecmark (bug 582997). The mobile config changes are easy peasy to r+. Have you verified that the other changes won't break desktop?
Attachment #461104 -
Flags: review?(anodelman)
Comment 6•13 years ago
|
||
Comment on attachment 461106 [details] the new mobile_profile with the talos e10s extension (1.0) This appears to work. Do you know if this will conflict at all with the new pageloader xpi? I'm putting that in distribution/bundles (bug 580698) and that also has a quit.js. Other than that question and the CVS dirs, this looks good to me.
Assignee | ||
Comment 7•13 years ago
|
||
I don't know much about the pageloader xpi stuff. It could conflict, but when is that landing? This solution is more of a temporary shim until we have a more complete solution in place. As for the CVS dirs, I have updated the patch to just have the extensions directory (all other dirs just had CVS cruft in there)
Attachment #461106 -
Attachment is obsolete: true
Attachment #461638 -
Flags: review?(aki)
Attachment #461106 -
Flags: review?(aki)
Comment 8•13 years ago
|
||
(In reply to comment #7) > I don't know much about the pageloader xpi stuff. It could conflict, but when > is that landing? I'm testing it in conjunction with this. I can switch over to that pageloader at any point.
Updated•13 years ago
|
Attachment #461638 -
Attachment is patch: false
Attachment #461638 -
Attachment mime type: text/plain → application/octet-stream
Comment 9•13 years ago
|
||
Comment on attachment 461638 [details] the new mobile_profile with the talos e10s extension (2.0) Did you delete the zip before zipping everything up? zip will add to an archive, not overwrite it. new-host-3:foo asasaki$ unzip ../mobile_profile.zip Archive: ../mobile_profile.zip inflating: mobile_profile/localstore.rdf inflating: mobile_profile/prefs.js creating: mobile_profile/bookmarkbackups/ creating: mobile_profile/bookmarkbackups/CVS/ extracting: mobile_profile/bookmarkbackups/CVS/Entries inflating: mobile_profile/bookmarkbackups/CVS/Repository extracting: mobile_profile/bookmarkbackups/CVS/Root creating: mobile_profile/Cache/ creating: mobile_profile/Cache/CVS/ extracting: mobile_profile/Cache/CVS/Entries extracting: mobile_profile/Cache/CVS/Repository extracting: mobile_profile/Cache/CVS/Root creating: mobile_profile/CVS/ inflating: mobile_profile/CVS/Entries inflating: mobile_profile/CVS/Entries.Log extracting: mobile_profile/CVS/Repository extracting: mobile_profile/CVS/Root creating: mobile_profile/extensions/ creating: mobile_profile/extensions/electrolysis@mozilla.org/ creating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/ creating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ inflating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc-overlay.xul inflating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc.js inflating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/quit.js inflating: mobile_profile/extensions/electrolysis@mozilla.org/chrome.manifest inflating: mobile_profile/extensions/electrolysis@mozilla.org/install.rdf
Assignee | ||
Comment 10•13 years ago
|
||
updated .zip to be just the few files we need for the mobile_profile.
Attachment #461638 -
Attachment is obsolete: true
Attachment #463239 -
Flags: review?(aki)
Attachment #461638 -
Flags: review?(aki)
Comment 11•13 years ago
|
||
Comment on attachment 463239 [details]
[checked in]the new mobile_profile with the talos e10s extension (2.1)
/me changes zip from patch->application/octet-stream again :)
Attachment #463239 -
Attachment is patch: false
Attachment #463239 -
Attachment mime type: text/plain → application/octet-stream
Comment 12•13 years ago
|
||
Comment on attachment 463239 [details]
[checked in]the new mobile_profile with the talos e10s extension (2.1)
I'm going to stamp this, since it can't make anything worse, and I think even if it does conflict with the new pageloader, I did see some successful talos runs with this.
Attachment #463239 -
Flags: review?(aki) → review+
Comment 13•13 years ago
|
||
Comment on attachment 461104 [details] [diff] [review] core changes required to get ts, twinopen working on talos (1.0) This has wfm in testing (spotty, because of network flakiness, but I've seen greens). However, I'm still concerned that quit.js and getInfo.html changes might affect desktop talos negatively, so keeping the Alice r?
Attachment #461104 -
Flags: review?(aki) → review+
Comment 15•13 years ago
|
||
Have these changes been testing on staging talos?
Comment 16•13 years ago
|
||
They have not, afaik.
Comment 17•13 years ago
|
||
I would want at least a small run in staging before we consider changing the basic functionality.
Comment 18•13 years ago
|
||
I've agreed to do the staging for jmaher for this patch.
Assignee | ||
Comment 19•13 years ago
|
||
updated patch to work better with firefox desktop (in non ipc mode). Tested on linux desktop fennec (ipc on/off) and firefox (non ipc). I did not do any tp tests or pageloader tests. Alice if you could indicate when this might get a change at some talos staging that would be helpful.
Attachment #461104 -
Attachment is obsolete: true
Attachment #461104 -
Flags: review?(anodelman)
Comment 20•13 years ago
|
||
I will start a staging run on this tomorrow morning, so that I can leave my current staging test overnight to run to completion.
Comment 21•13 years ago
|
||
Staging run started.
Comment 22•13 years ago
|
||
So far I'm seeing green mixed in with quite a bit of orange related to failure to initialize the browser. I'm trying to sort through it now and I'll work with jmaher tomorrow to see if this can be resolved.
Assignee | ||
Comment 23•13 years ago
|
||
after looking over this code, it is the same code I am using in mochitest which has passed on all platforms and has been running in this fashion on all tinderbox unittests (for mochitest) for the last 7 weeks. With that said there still could be issues with the difference in setup and how Firefox is being run.
Comment 24•13 years ago
|
||
From the test runs overnight the issue is only on mac talos (snow leopard/leopard). The patch causes the browser to fail to be initialized and no tests are run. As such this is r- for production landing.
Assignee | ||
Comment 25•13 years ago
|
||
So I ran these tests on my MBP OSX and it completed a run (I didn't have the TP4 stuff setup), but ts, ts_cold, ts_shutdown, ts_places*, twinopen all ran and produced results (in the output/*.csv format.) I did nothing to change my code, I just used the patches applied to this bug. Could it be there was a bad build of OSX Firefox when testing these in staging?
Comment 26•13 years ago
|
||
I'm also seeing success when I manually ran on a single slave (talos-r3-leopard-002). This is going to require a second staging run to determine what is going on.
Comment 27•13 years ago
|
||
We are getting some mixed results in staging, but our confidence is high enough that we'd like to land during a downtime with a close, close eye on mac talos results.
Updated•13 years ago
|
Attachment #464577 -
Flags: review+
Comment 28•13 years ago
|
||
Checking in getInfo.html; /cvsroot/mozilla/testing/performance/talos/getInfo.html,v <-- getInfo.html new revision: 1.4; previous revision: 1.3 done Checking in mobile.config; /cvsroot/mozilla/testing/performance/talos/mobile.config,v <-- mobile.config new revision: 1.6; previous revision: 1.5 done RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/localstore.rdf,v done Checking in mobile_profile/localstore.rdf; /cvsroot/mozilla/testing/performance/talos/mobile_profile/localstore.rdf,v <-- localstore.rdf initial revision: 1.1 done RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/prefs.js,v done Checking in mobile_profile/prefs.js; /cvsroot/mozilla/testing/performance/talos/mobile_profile/prefs.js,v <-- prefs.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome.manifest,v done Checking in mobile_profile/extensions/electrolysis@mozilla.org/chrome.manifest; /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome.manifest,v <-- chrome.manifest initial revision: 1.1 done RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/install.rdf,v done Checking in mobile_profile/extensions/electrolysis@mozilla.org/install.rdf; /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/install.rdf,v <-- install.rdf initial revision: 1.1 done RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc-overlay.xul,v done Checking in mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc-overlay.xul; /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc-overlay.xul,v <-- ipc-overlay.xul initial revision: 1.1 done RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc.js,v done Checking in mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc.js; /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc.js,v <-- ipc.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/quit.js,v done Checking in mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/quit.js; /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/quit.js,v <-- quit.js initial revision: 1.1 done Checking in page_load_test/quit.js; /cvsroot/mozilla/testing/performance/talos/page_load_test/quit.js,v <-- quit.js new revision: 1.2; previous revision: 1.1 done Checking in startup_test/fennecmark/fennecmark.html; /cvsroot/mozilla/testing/performance/talos/startup_test/fennecmark/fennecmark.html,v <-- fennecmark.html new revision: 1.2; previous revision: 1.1 done
Updated•13 years ago
|
Attachment #463239 -
Attachment description: the new mobile_profile with the talos e10s extension (2.1) → [checked in]the new mobile_profile with the talos e10s extension (2.1)
Updated•13 years ago
|
Attachment #464577 -
Attachment description: core changes required to get ts, twinopen working on talos (2.0) → [checked in]core changes required to get ts, twinopen working on talos (2.0)
Comment 29•13 years ago
|
||
Joel, John, Alice: you guys rock.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•