There's a few minor changes required before Thunderbird can run Marionette, and therefore Mochitest.

I *think* these are all the changes needed, but I can't rule out more later on. I'm only planning to use this for "browser-chrome" style tests, so I think I can get away with some thing just not working (for Thunderbird).

Comment on attachment 9024364 [details] [diff] [review] 1506523-marionette-1.diff Review of attachment 9024364 [details] [diff] [review]: ----------------------------------------------------------------- Please see my inline comments. Also I would like to see an additional review from Andreas because he currently works on refactoring the window handling. ::: testing/marionette/assert.js @@ +101,5 @@ > + * > + * @throws {UnsupportedOperationError} > + * If current browser is not Firefox or Thunderbird. > + */ > +assert.firefoxOrThunderbird = function(msg = "") { Maybe we should just call this `assert.desktop` which would include all desktop apps. ::: testing/marionette/components/marionette.js @@ +408,5 @@ > + // Thunderbird only, instead of sessionstore-windows-restored. > + case "mail-startup-done": > + this.finalUIStartup = true; > + this.init(); > + break; I don't know about the startup paths for Thunderbird. So it would be good to also get a review from a Thunderbird peer / module owner if it is fine to hook into that observer, or if we have to wait a bit longer. ::: testing/marionette/driver.js @@ +562,5 @@ > }; > > GeckoDriver.prototype.listeningPromise = function() { > + if (this.appId == APP_ID_THUNDERBIRD) { > + return Promise.resolve(); Is this because Thunderbird doesn't use e10s? If yes, we shouldn't check by app but if the e10s mode is enabled or not. @@ +722,5 @@ > let registerBrowsers = this.registerPromise(); > let browserListening = this.listeningPromise(); > > let waitForWindow = function() { > + let windowType = (this.appId == APP_ID_THUNDERBIRD) ? "mail:3pane" : "navigator:browser"; Please change that to a switch statement. @@ +767,5 @@ > if (this.mainFrame) { > this.mainFrame.focus(); > } > > + if (this.curBrowser.tab && this.curBrowser.contentBrowser) { Why do you need this change? @@ +3308,5 @@ > return {cause: await quitApplication}; > }; > > GeckoDriver.prototype.installAddon = function(cmd) { > + assert.firefoxOrThunderbird(); I assume this is the only command you make use of for now. So I'm fine with just changing only this one.

Comment on attachment 9024364 [details] [diff] [review] 1506523-marionette-1.diff Review of attachment 9024364 [details] [diff] [review]: ----------------------------------------------------------------- If you’re fine with this breaking at any point due to no upstream testing of Thunderbird in mozilla-central, I’m happy for Marionette to take on preliminary Thunderbird support. ::: testing/marionette/driver.js @@ +767,5 @@ > if (this.mainFrame) { > this.mainFrame.focus(); > } > > + if (this.curBrowser.tab && this.curBrowser.contentBrowser) { In this patch, I think this is the only part I’m worried about.

(In reply to Henrik Skupin (:whimboo) from comment #2) > ::: testing/marionette/driver.js > @@ +562,5 @@ > > }; > > > > GeckoDriver.prototype.listeningPromise = function() { > > + if (this.appId == APP_ID_THUNDERBIRD) { > > + return Promise.resolve(); > > Is this because Thunderbird doesn't use e10s? If yes, we shouldn't check by > app but if the e10s mode is enabled or not. (In reply to Andreas Tolfsen ❲:ato❳ from comment #3) > ::: testing/marionette/driver.js > @@ +767,5 @@ > > if (this.mainFrame) { > > this.mainFrame.focus(); > > } > > > > + if (this.curBrowser.tab && this.curBrowser.contentBrowser) { > > In this patch, I think this is the only part I’m worried about. I think I've managed to do away with both of these changes. Will post a new patch when I'm sure.

Comment on attachment 9025795 [details] [diff] [review] 1506523-marionette-2.diff Review of attachment 9025795 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the update! Those changes look fine. There is just a minor thing, which you can find as an inline comment. ::: testing/marionette/components/marionette.js @@ +411,5 @@ > + // Thunderbird only, instead of sessionstore-windows-restored. > + case "mail-startup-done": > + this.finalUIStartup = true; > + this.init(); > + break; Mind putting it right before `sessionstore-windows-restored`?

Done.

Pushed by csabou@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f9d8c6059114 Adapt Marionette so it can run on Thunderbird; r=whimboo