We want to move browser prefs that are common to firefox-ui-tests [1] and other desktop tests into Marionette. Original discussion: https://bugzilla.mozilla.org/show_bug.cgi?id=1212608#c20 [1] https://github.com/mozilla/firefox-ui-tests/blob/50527c612f3c063ab52046d0d01f610b64f5f003/firefox_ui_harness/runners/base.py#L14

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1349a0312ef

Bug 1219397 - Collect Firefox-Desktop-specific prefs in a subclass of GeckoInstance; r?automatedtester

https://reviewboard.mozilla.org/r/23597/#review21143 ::: testing/marionette/driver/marionette_driver/geckoinstance.py:200 (Diff revision 1) > + 'marionette.logging': False, This line is stil questionable as talked on IRC yesterday. If that is enabled by default we might follow that, but the problem is that the Marionette logging is way too detailed even in INFO level. I think it would be good to check if we can change Marionette logging so it only spits out those line for DEBUG level? Not sure about any affects for other harnesses. ::: testing/marionette/driver/marionette_driver/geckoinstance.py:205 (Diff revision 1) > + 'toolkit.telemetry.enabled': False, Also a lot of those prefs are not desktop only. Entries like telemetry or geolocation should be present for all applications. The pref focusmanager.testmode gets set in GeckoInstance already. As long as David is not around this week it might make sense to ask Jonathan for review.

Comment on attachment 8680204 [details] MozReview Request: Bug 1219397 - Collect Firefox-Desktop-specific prefs in a subclass of GeckoInstance; r?automatedtester https://reviewboard.mozilla.org/r/23597/#review21353 ::: testing/marionette/driver/marionette_driver/geckoinstance.py:186 (Diff revision 1) > + 'browser.warnOnQuit': False, There are a number of duplicates with `GeckoInstance`. If they are the same we should probably just leave it in there and only have ones that are different here.

Comment on attachment 8680204 [details] MozReview Request: Bug 1219397 - Collect Firefox-Desktop-specific prefs in a subclass of GeckoInstance; r?automatedtester Bug 1219397 - Collect Firefox-Desktop-specific prefs in a subclass of GeckoInstance; r?automatedtester

Thanks for the reviews. I removed the duplicates and moved the telemetry entry up into GeckoInstance. I'm not sure what else is not desktop specific: feedback welcome. As for the log-level for marionette logging, I will ask around tomorrow and maybe file a bug.

https://reviewboard.mozilla.org/r/23597/#review21479 ::: testing/marionette/driver/marionette_driver/geckoinstance.py:41 (Diff revision 2) > + 'toolkit.telemetry.enabled': False,} I would suggest to put the closing bracket into the next line, so if more prefs are getting added you will only see the addition and not an update to line 41. ::: testing/marionette/driver/marionette_driver/geckoinstance.py:195 (Diff revision 2) > + 'marionette.logging': False, If we go with False here we should add a comment why we do that given that it overrides the default logging value of GeckoInstance. I would still propose to discuss if we can reduce the amount of Marionette logging in INFO level. ::: testing/marionette/driver/marionette_driver/geckoinstance.py:212 (Diff revision 2) > + 'fxdesktop': DesktopInstance} Same here for the closing bracket. Some drive-by notes. Otherwise it looks fine to me. All of those prefs should be desktop related but apply to various apps like Firefox, Thunderbird, and SeaMonkey. So good for other subclassing if necessary later. Or shall we already have a FirefoxInstance?

Comment on attachment 8680204 [details] MozReview Request: Bug 1219397 - Collect Firefox-Desktop-specific prefs in a subclass of GeckoInstance; r?automatedtester Bug 1219397 - Collect Firefox-Desktop-specific prefs in a subclass of GeckoInstance; r?automatedtester

This last change just addresses Comment 7. Also, I filed Bug 1221187 re gecko.log

Comment on attachment 8680204 [details] MozReview Request: Bug 1219397 - Collect Firefox-Desktop-specific prefs in a subclass of GeckoInstance; r?automatedtester https://reviewboard.mozilla.org/r/23597/#review21565

Comment on attachment 8680204 [details] MozReview Request: Bug 1219397 - Collect Firefox-Desktop-specific prefs in a subclass of GeckoInstance; r?automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23597/diff/3-4/

Rebased to avoid merge conflict with Bug 1217557

Comment on attachment 8680204 [details] MozReview Request: Bug 1219397 - Collect Firefox-Desktop-specific prefs in a subclass of GeckoInstance; r?automatedtester https://reviewboard.mozilla.org/r/23597/#review21947