we want to run talos tests against google chrome so we have some form of competitive benchmarking. This means we need to: 1) obtain a nightly version of google chrome (some prior art in AWFY: https://github.com/mozilla/arewefastyet/blob/master/slave/download.py#L272) 2) setup profile and launch process (prior art for process: https://github.com/mozilla/arewefastyet/blob/master/slave/executors.py#L154, also web-platform-tests has prior art) 3) integration this into talos so we can run with a path to the binary and --google-chrome mode for managing preferences/env_vars/process/web_extension/reporting 4) add support in taskcluster to run tests (tp6, speedometer and motionmark jobs) with google chrome. I am not sure if we need a download job to obtain and cache the google-chrome nightly build. 5) schedule this to run on nightly builds on mozilla-central. 6) figure out how to post results to perfherder- since we don't have support for other products, I would recommend adding an option to existing tests as 'chrome' or 'google' so we would upload results for "tp6 stylo e10s google-chrome opt" and that should work with our existing toolchains. I am sure there are details I have overlooked, likewise some details I have added which are incorrect or not needed.

I have a couple of questions: 1) Should this be integrated with Raptor instead of Talos? If we plan to slowly start deprecating Talos and write all new tests in Raptor, then I think we'd get more value focusing there. 2) Should we build this with the wpt infrastructure or the AWFY infrastructure? The wpt implementation looks more robust, but depends heavily on webdriver. Is using webdriver an explicit non-goal? Would we ever conceivably want to share these tests with other vendors? I also have some observations: * This should be a standalone module, similar to mozrunner (or perhaps integrated into mozrunner) * We might be able to find a module for starting/stopping chrome in the chromium source code

yes for raptor, if it isn't ready have a proof of concept for talos. Talos will probably not go away, but will be simplified a lot. I don't mind which wpt or awfy- whichever is better, I suspect wpt is more robust and maintained. I have found that marionette doesn't work with talos historically, so not requiring webdriver, geckodriver, marionette would be good. I like the idea of a standalone module or some easy addition to mozrunner.

Ok, if we don't want to require webdriver it's probably best to start off with AWFY's implementation. That will also be the simpler solution, so easier to get a prototype working.

I had another clarification. The bullet list from comment 0 doesn't mention the tests themselves supporting Chrome. Can I assume that either the tests + pageloader (or however measurements are taken) can already run with chrome? Or at least that this is being worked on in a separate bug? As a follow-up, is there a way to run a talos test manually just so I can find one that works as something to work towards? Is there a test you'd like to see prioritized?

this will be used in the new raptor framework that :rwood is working on. If you want to integrate into there to start that would be fine. That framework will run the benchmarks (speedometer, jetstream,ares-6, etc.) and pageload tests (tp6) instead of talos.

Thanks for the clarification. Comment 2 made it sound like we wanted to support this in Talos as well as raptor, but I guess pageloader probably doesn't work in Chrome. I'll sync up with Rob to figure out the state of Chrome support.

More details in the pull request, but basically this just gets things using mozrunner. There is still no support for Chrome profiles, I'll file a follow-up to tackle that. This deletes a whole bunch of Talos code that wasn't really being used. We may need to re-implement some of it (for example running in 'debug' mode), but I think it will be easier to start from scratch than to try to shoe-horn the talos_process.py stuff into the mozrunner model. This is a pretty large change, so please ask if you have any questions.

Comment on attachment 8963373 [details] Use mozrunner to manager Firefox/Chrome processes Thanks Andrew. See comments in the PR. I say we go ahead and merge this, and figure out the issue of killing the browser (more specifically on google Chrome, since chrome web extensions are unable to dump output to stdout to signal the output handler to kill the browser) in a future PR.

Merged after your latest PR update: https://github.com/rwood-moz/raptor/commit/cabf707cef84de6afd37a494a378a775341590d7

I believe we are at the 50%+ mark on completing this- moving to April as we won't get it done in the next day or so.

Yes, we have chrome running via mozrunner, but no support for profiles or setting prefs yet. Haven't had a chance to start looking into this, but should in the next day or two.

Moving this to mozbase. I'll use this bug to get the mozbase specific parts landed in m-c first. The raptor parts will already be landed in github, and these mozbase pieces will start being used once that gets moved into m-c.

Comment on attachment 8967064 [details] Use mozprofile for managing chrome profiles LGTM and works great. Couple notes in the PR, I believe they are issues that maybe can be solved with google chrome prefs.

Thanks for the review! Once this is merged I'll start work on porting these changes to mozilla-central (in advance of the raptor merge). I'll call this bug resolved once the mozprofile/mozrunner changes hit m-c.

Great, thanks Andrew. Merged the Raptor PR: https://github.com/rwood-moz/raptor/commit/2c0c06532e101d5d822bbb3bc12f644b5535bda7

Something weird happened with that review request :/. The commit messages aren't showing up for some of the commits. I'll have to look into it next week. Also I'll be picking some "lucky" reviewers :)

Sorry for all the bugspam, something went wrong with the initial push. I discarded it and pushed again and it seems to be ok now.

Comment on attachment 8968198 [details] Bug 1445944 - [mozrunner] Convert mozrunner unittests to the pytest format https://reviewboard.mozilla.org/r/236888/#review242678

Comment on attachment 8968196 [details] Bug 1445944 - [moztest] Update shared test fixtures so they can work outside of mozilla-central https://reviewboard.mozilla.org/r/236884/#review242692

Comment on attachment 8968200 [details] Bug 1445944 - [mozprofile] Convert mozprofile unittests to the pytest format https://reviewboard.mozilla.org/r/236892/#review242680 ::: testing/mozbase/mozprofile/tests/test_addonid.py:150 (Diff revision 1) > + f.write(filecontents) > + f.close() > + return path > + > + > +def test_addonID(testlist): You could use a fixture to create the install.rdf, and yield the path. This fixture could use pytest's tmpdir to manage the temporary directory. It could then be parameterised based on testlist, so the test is invoked for each rdf file instead of looping over them. ::: testing/mozbase/mozprofile/tests/manifest.ini (Diff revision 1) > -[server_locations.py] > +[test_server_locations.py] > [test_preferences.py] > -[permissions.py] > -[bug758250.py] > +[test_permissions.py] > +[test_bug758250.py] > [test_nonce.py] > -[bug785146.py] Was this file intentionally removed? ::: testing/mozbase/mozprofile/tests/test_permissions.py:25 (Diff revision 1) > > - profile_dir = None > - locations_file = None > > - def setUp(self): > - self.profile_dir = tempfile.mkdtemp() > +@pytest.fixture > +def locations_file(tmpdir): It looks like tmpdir isn't used here. ::: testing/mozbase/mozprofile/tests/test_permissions.py:40 (Diff revision 1) > + return Permissions(tmpdir.mkdir('profile').strpath, locations_file.name) > + > + > +def test_create_permissions_db(perms): > + profile_dir = perms._profileDir > + perms_db_filename = os.path.join(profile_dir, 'permissions.sqlite') perms_db_filename could be a fixture as it's used in a couple of places. Alternatively (or additionally) the db connection could be a fixture. ::: testing/mozbase/mozprofile/tests/test_server_locations.py:51 (Diff revision 1) > + assert location.host == host > + assert location.port == port > + assert location.options == options > + > + > +def create_temp_file(contents): Could we use pytest's tmpdir fixture instead? ::: testing/mozbase/mozprofile/tests/test_server_locations.py:90 (Diff revision 1) > > - self.assertRaises(BadPortLocationError, locations.add_host, '127.0.0.1', > - port='abc') > + with pytest.raises(BadPortLocationError): > + locations.add_host('127.0.0.1', port='abc') > > - # test some errors in locations file > + # test some errors in locations file > - f = self.create_temp_file(self.locations_no_primary) > + f = create_temp_file(LOCATIONS_NO_PRIMARY) It feels like this should be a separate test, though I appreciate that may be out of scope for this bug. ::: testing/mozbase/mozprofile/tests/test_server_locations.py:103 (Diff revision 1) > - self.assertEqual(exc.err.__class__, MissingPrimaryLocationError) > - self.assertEqual(exc.lineno, 3) > + assert exc.err.__class__ == MissingPrimaryLocationError > + assert exc.lineno == 3 > > - # test bad port in a locations file to ensure lineno calculated > + # test bad port in a locations file to ensure lineno calculated > - # properly. > + # properly. > - f = self.create_temp_file(self.locations_bad_port) > + f = create_temp_file(LOCATIONS_BAD_PORT) As above, I feel like this should be a separate test. ::: testing/mozbase/mozprofile/tests/test_addons.py:188 (Diff revision 1) > > - self.profile.reset() > > - self.profile.addons.install(addon) > - self.assertEqual(self.profile.addons.installed_addons, [addon]) > +def test_install_backup(tmpdir, profile): > + tmpdir = tmpdir.strpath > + am = profile.addons We have this in most (if not all) of these tests. It's minor, but we could have am `am` fixture to save a few lines of duplication. ::: testing/mozbase/mozprofile/tests/test_addons.py:297 (Diff revision 1) > - addon_path = os.path.join(os.path.join(here, 'files'), 'not_an_addon.txt') > + addon_path = os.path.join(os.path.join(here, 'files'), 'not_an_addon.txt') > - self.assertRaises(mozprofile.addons.AddonFormatError, > - self.am.addon_details, addon_path) > + with pytest.raises(mozprofile.addons.AddonFormatError): > + am.addon_details(addon_path) > + > + > +@pytest.mark.skip("bug 900154") The associated bug has since been closed. ::: testing/mozbase/mozprofile/tests/test_clone_cleanup.py:56 (Diff revision 1) > - clone.cleanup() > + clone.cleanup() > > - # clone should be deleted > + # clone should be deleted > - self.assertFalse(os.path.exists(clone.profile)) > - self.assertTrue(counter[0] > 0) > + assert not os.path.exists(clone.profile) > + assert counter[0] > 0 > + mozfile.remove(clone.profile) We try to remove the directory after we've asserted that it doesn't exist. Also, if the clone is in `tmpdir` then pytest will remove it for us. ::: testing/mozbase/mozprofile/tests/test_clone_cleanup.py:66 (Diff revision 1) > + clone = Profile.clone(profile.profile, restore=False) > - clone.cleanup() > + clone.cleanup() > > - # clone should still be around on the filesystem > + # clone should still be around on the filesystem > - self.assertTrue(os.path.exists(clone.profile)) > + assert os.path.exists(clone.profile) > + mozfile.remove(clone.profile) If the clone is within `tmpdir` then we should be able to let pytest clean it up. ::: testing/mozbase/mozprofile/tests/test_profile_view.py:29 (Diff revision 1) > - keys = set(['Files', 'Path', 'user.js']) > + keys = set(['Files', 'Path', 'user.js']) > - ff_prefs = mozprofile.FirefoxProfile.preferences # shorthand > + ff_prefs = mozprofile.FirefoxProfile.preferences # shorthand > - pref_string = '\n'.join(['%s: %s' % (key, ff_prefs[key]) > + pref_string = '\n'.join(['%s: %s' % (key, ff_prefs[key]) > - for key in sorted(ff_prefs.keys())]) > + for key in sorted(ff_prefs.keys())]) > > - tempdir = tempfile.mkdtemp() > + tempdir = tempfile.mkdtemp() Could we use pytest's `tmpdir` fixture here? If we can, then we should also be able to remove the try/catch as pytest will handle cleaning up the temporary directory.

Comment on attachment 8968200 [details] Bug 1445944 - [mozprofile] Convert mozprofile unittests to the pytest format https://reviewboard.mozilla.org/r/236892/#review242680 > You could use a fixture to create the install.rdf, and yield the path. This fixture could use pytest's tmpdir to manage the temporary directory. It could then be parameterised based on testlist, so the test is invoked for each rdf file instead of looping over them. Yeah, I was being a bit lazy for this (and most of the other places you recommend using fixtures below) and keeping things as is. But you're right, greater use of fixtures will make all this more readable so I'll fix it up some more. > Was this file intentionally removed? Yes, I probably should have done this in a separate commit to make it more obvious. But this test was moved into test_permissions.py (as it is also testing the permissions module and uses many of the same fixtures as those tests). > The associated bug has since been closed. I'll either enable it or xfail it. p.s we should set the xfail_strict option globally for our tests, I'll file another bug to do that. > We try to remove the directory after we've asserted that it doesn't exist. Also, if the clone is in `tmpdir` then pytest will remove it for us. Good call, this should be in a finally. The clone uses `tempfile` but I guess we can set the root temp directory first. We'd still have to do cleanup to revert the root though, so probably not worth it.

Comment on attachment 8968199 [details] Bug 1445944 - [mozrunner] Create a base BlinkRuntimeRunner and add a ChromeRunner to the runners list https://reviewboard.mozilla.org/r/236890/#review242752 LGTM ::: testing/mozbase/mozrunner/mozrunner/runners.py:79 (Diff revision 1) > return GeckoRuntimeRunner(*args, **kwargs) > > > +def ChromeRunner(*args, **kwargs): > + """ > + Create a destkop Google Chrome runner. nit: typo

Comment on attachment 8968200 [details] Bug 1445944 - [mozprofile] Convert mozprofile unittests to the pytest format https://reviewboard.mozilla.org/r/236892/#review242680 > It feels like this should be a separate test, though I appreciate that may be out of scope for this bug. I agree with you, but I think I'd rather not make this patch more complex than it already is. I think in general our mozbase tests could use a lot of improvement.

Comment on attachment 8968201 [details] Bug 1445944 - [mozprofile] Add a 'create_profile' helper method for instanting an instance from an app https://reviewboard.mozilla.org/r/236894/#review242764 Cool!

Comment on attachment 8968202 [details] Bug 1445944 - [mozprofile] Pull functionality out of Profile and into an abstract 'BaseProfile' class https://reviewboard.mozilla.org/r/236896/#review242766

Comment on attachment 8968203 [details] Bug 1445944 - [mozprofile] Create a new ChromeProfile class for managing chrome profiles https://reviewboard.mozilla.org/r/236898/#review242770 Awesome

Comment on attachment 8968204 [details] Bug 1445944 - [mozbase] Bump mozprofile and mozrunner version numbers https://reviewboard.mozilla.org/r/236900/#review242774 LGTM

Comment on attachment 8968197 [details] Bug 1445944 - [mozrunner] Remove ability to specify the 'wrap_command' function on an Application context https://reviewboard.mozilla.org/r/236886/#review242788

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/92a5018d1ecf [moztest] Update shared test fixtures so they can work outside of mozilla-central r=rwood https://hg.mozilla.org/integration/autoland/rev/eeb6b72619b6 [mozrunner] Remove ability to specify the 'wrap_command' function on an Application context r=rwood https://hg.mozilla.org/integration/autoland/rev/917baaf08d71 [mozrunner] Convert mozrunner unittests to the pytest format r=davehunt https://hg.mozilla.org/integration/autoland/rev/df5d226d1df3 [mozrunner] Create a base BlinkRuntimeRunner and add a ChromeRunner to the runners list r=rwood https://hg.mozilla.org/integration/autoland/rev/74448840c21f [mozprofile] Convert mozprofile unittests to the pytest format r=davehunt https://hg.mozilla.org/integration/autoland/rev/96f6bb303871 [mozprofile] Add a 'create_profile' helper method for instanting an instance from an app r=rwood https://hg.mozilla.org/integration/autoland/rev/62c954751e65 [mozprofile] Pull functionality out of Profile and into an abstract 'BaseProfile' class r=rwood https://hg.mozilla.org/integration/autoland/rev/fdad1264866f [mozprofile] Create a new ChromeProfile class for managing chrome profiles r=rwood https://hg.mozilla.org/integration/autoland/rev/bfabe6333c85 [mozbase] Bump mozprofile and mozrunner version numbers r=rwood

I'll release the new mozrunner/mozprofile versions to pypi once this hits central.

https://hg.mozilla.org/mozilla-central/rev/92a5018d1ecf https://hg.mozilla.org/mozilla-central/rev/eeb6b72619b6 https://hg.mozilla.org/mozilla-central/rev/917baaf08d71 https://hg.mozilla.org/mozilla-central/rev/df5d226d1df3 https://hg.mozilla.org/mozilla-central/rev/74448840c21f https://hg.mozilla.org/mozilla-central/rev/96f6bb303871 https://hg.mozilla.org/mozilla-central/rev/62c954751e65 https://hg.mozilla.org/mozilla-central/rev/fdad1264866f https://hg.mozilla.org/mozilla-central/rev/bfabe6333c85

Released to pypi.