Bug 1451159 introduced this new concept of "base profiles". These are directories that live under testing/profiles which test harnesses can merge into their own profiles. This provides a way for us to share preferences and extensions across test harnesses. There are a couple problems with what landed: 1) It's not obvious which harnesses use which profiles without searching the code of every harness (which also requires knowing how they work to some degree). 2) The external web-platform-tests downloads the preference file from hgweb, and it needs to work across all Firefox versions. This means anytime the preference file changes names, we need to add a version check for the appropriate pref file. This is both fragile and annoying. Especially since I'd like to start supporting *multiple* preference files. To solve both problems, we can add a "profiles.json" file which maps suite names to a list of base profiles. It might look like: { "mochitest": ["common", "mochitest"], "wpt": ["common", "wpt"], } This way wpt can first download this file, figure out which profiles it should be using, and then download the relevant pref files after the fact. This will allow us to rename/split profiles without needing to make modifications to upstream wpt. Further, this will give developers a quick and simple way to figure out which harnesses will be affected by the changes they are making.

I need a bit of help testing this on upstream wpt. I've verified that this works on try and with |mach web-platform-tests|. However, when running: $ cd testing/web-platform/tests && ./wpt run firefox infrastructure I get: NameError: global name 'WindowsError' is not defined I've verified that this happens with or without my patch. It does happen after the pref setup code runs, so at least that works, but I'm not 100% sure that the prefs end up being applied when Firefox finally runs.

Comment on attachment 8973724 [details] Bug 1459598 - Use profiles.json file to map test suites to the base profiles they use, https://reviewboard.mozilla.org/r/242096/#review248158 Generally looks good, just a few issues with wpt to address. ::: testing/web-platform/tests/tools/wpt/browser.py:209 (Diff revision 3) > # Always use tip as the tag for nightly; this isn't quite right > # but to do better we need the actual build revision, which we > # can get if we have an application.ini file > tag = "tip" > > - return "%s/raw-file/%s/testing/profiles/common/user.js" % (repo, tag) > + return "%s/archive/%s.zip/testing/profiles/" % (repo, tag) Wow, that's some deep magic; nice :) ::: testing/web-platform/tests/tools/wpt/browser.py:218 (Diff revision 3) > > if dest is None: > dest = os.pwd > > - dest = os.path.join(dest, "profiles", "common") > - if not os.path.exists(dest): > + dest = os.path.join(dest, "profiles") > + cache_file = os.path.join(dest, "profiles.json") So this seems to do away with the per-version cache, and just have a single directory. But it occurs to me that we can probably just do something like `dest = os.pathjoin(dest, channel, version)` and just put the contents of `testing/profiles` in there directly. Then if the chanel != nightly, we can just check if the directory already exists and skip all the fetching if it does. ::: testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox.py:255 (Diff revision 3) > - if os.path.exists(prefs_path): > - prefs.add(Preferences.read_prefs(prefs_path)) > + prefs_general = os.path.join(self.prefs_root, 'prefs_general.js') > + if os.path.isfile(prefs_general): > + # Old preference file used in Firefox 60 and earlier (remove when no longer supported) > + pref_paths.append(prefs_general) > + > + profiles = os.path.join(self.prefs_root, 'profiles.json') It would be nice if this logic was put into mozprofile somehow e.g. `prefs.add_for_suite(sutie_name, profiles_path)`. That would require a mozprofile release ofc. Happy to defer if you want to do that later though.

Comment on attachment 8973724 [details] Bug 1459598 - Use profiles.json file to map test suites to the base profiles they use, https://reviewboard.mozilla.org/r/242096/#review248158 > So this seems to do away with the per-version cache, and just have a single directory. But it occurs to me that we can probably just do something like > > `dest = os.pathjoin(dest, channel, version)` > > and just put the contents of `testing/profiles` in there directly. Then if the chanel != nightly, we can just check if the directory already exists and skip all the fetching if it does. Good idea! > It would be nice if this logic was put into mozprofile somehow e.g. `prefs.add_for_suite(sutie_name, profiles_path)`. > > That would require a mozprofile release ofc. Happy to defer if you want to do that later though. This sort of exists with `Profile.merge("path/to/profile")`, but wpt doesn't use `Profile` objects here (and merge doesn't know about `profiles.json`). I agree there is a fair amount of duplication being added here around finding and reading `profiles.json`. But I think I'll take you up on that offer to defer it to a later time.

Comment on attachment 8973724 [details] Bug 1459598 - Use profiles.json file to map test suites to the base profiles they use, https://reviewboard.mozilla.org/r/242096/#review248188 ::: testing/web-platform/tests/tools/wpt/browser.py:218 (Diff revisions 3 - 4) > > if dest is None: > dest = os.pwd > > - dest = os.path.join(dest, "profiles") > + dest = os.path.join(dest, "profiles", channel, version) > cache_file = os.path.join(dest, "profiles.json") In theory this doesn't need to exist right? In particular it won't for the `prefs_general.js` case.

Comment on attachment 8973724 [details] Bug 1459598 - Use profiles.json file to map test suites to the base profiles they use, https://reviewboard.mozilla.org/r/242096/#review248250

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e4b9cb3abd07 Use profiles.json file to map test suites to the base profiles they use, r=jgraham

Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/10940 for changes under testing/web-platform/tests

Upstream PR merged