Follow-up work from bug 1451159. This will move reftest preferences in layout/tools/reftest-preferences.js into the new testing/profiles system.

Comment on attachment 8976323 [details] Bug 1460912 - [testing/profiles] Add ability to diff and show multiple profiles with '+' https://reviewboard.mozilla.org/r/244504/#review250696 LGTM and tested it out locally on OSX: ./mach python testing/profiles/profile diff perf+common raptor ==> only diff is the pref for non_blank_paint ./mach python testing/profiles/profile diff raptor+perf+common perf+common ==> only diff is the pref for non_blank_paint ./mach python testing/profiles/profile diff raptor+perf+common common+raptor+perf ==> no diffs ./mach python testing/profiles/profile show common+perf > ../Desktop/1.txt ./mach python testing/profiles/profile show raptor > ../Desktop/2.txt diff ../Desktop/1.txt ../Desktop/2.txt 60a61 dom.performance.time_to_non_blank_paint.enabled: True ::: testing/profiles/profile:13 (Diff revision 2) > > # The beginning of this script is both valid shell and valid python, > # such that the script starts with the shell and is reexecuted python > '''which' mach > /dev/null 2>&1 && exec mach python "$0" "$@" || > -echo "mach not found, either add it to your \$PATH or run this script via ./mach python testing/profiles/profile"; exit > +echo "mach not found, either add it to your \$PATH or run this script via ./mach python testing/profiles/profile"; exit # noqa > ''' just curious what the '# noqa' comment is?

Comment on attachment 8976338 [details] Bug 1460912 - [testing/profiles] Add --format options to ./profile diff https://reviewboard.mozilla.org/r/244516/#review250722 LGTM, one comment below ::: testing/profiles/profile:244 (Diff revision 1) > help="Path to the first profile or suite name to diff.") > diff_parser.add_argument('b', metavar='B', > help="Path to the second profile or suite name to diff.") > + diff_parser.add_argument('-f', '--format', dest='fmt', default='pretty', > + choices=['pretty', 'json', 'names'], > + help="Format to dump diff in (default: pretty)") I find getting help in the tool itself is a bit awkward. ./mach python testing/profiles/profile -- --help usage: profile [-h] {diff,sort,show} ... positional arguments: {diff,sort,show} optional arguments: -h, --help show this help message and exit So then I try to get help for 'diff' so I can see the '--format' option, so I try: ./mach python testing/profiles/profile -h diff usage: mach [global arguments] python [command arguments] Run Python. ... but that gives help for the mach python command, not the 'profile diff' command.

Comment on attachment 8976338 [details] Bug 1460912 - [testing/profiles] Add --format options to ./profile diff https://reviewboard.mozilla.org/r/244516/#review250730 ::: testing/profiles/profile:60 (Diff revision 1) > + 'pretty': ( > + '{pref}: {value}', > + '{pref}: {value_a} => {value_b}' > + ), > +} > + One other thing I noticed. If I do: ./mach python testing/profiles/profile diff common+perf raptor The console output is nicely alphabetical within each section (delete, insert, same). However if I do: ./mach python testing/profiles/profile diff common+perf raptor --format json Inside the json sections (i.e. delete, insert, same) the prefs seem random and aren't in alphabetical order which is a bit annoying.

Comment on attachment 8976323 [details] Bug 1460912 - [testing/profiles] Add ability to diff and show multiple profiles with '+' https://reviewboard.mozilla.org/r/244504/#review250696 > just curious what the '# noqa' comment is? # noqa tells flake8 to ignore that specific line

Comment on attachment 8976324 [details] Bug 1460912 - [testing/profiles] Add option to limit ./profile diff to a specified key https://reviewboard.mozilla.org/r/244506/#review250778 Yep LGTM and works great

Comment on attachment 8976327 [details] Bug 1460912 - [testing/profiles] Sort testing/profiles/reftest/user.js https://reviewboard.mozilla.org/r/244512/#review250790

Comment on attachment 8976325 [details] Bug 1460912 - [testing/profiles] Add a ./profile rm subcommand for removing prefs from a profile https://reviewboard.mozilla.org/r/244508/#review250792 LGTM, tested locally on OSX: rwood$ cat ../Desktop/pref_to_remove.txt dom.performance.time_to_non_blank_paint.enabled rwood$ cat testing/profiles/raptor/user.js // Preferences file used by the raptor harness /* globals user_pref */ user_pref("dom.performance.time_to_non_blank_paint.enabled", true); rwood$ ./mach python testing/profiles/profile rm raptor --pref-file ../Desktop/pref_to_remove.txt rwood$ cat testing/profiles/raptor/user.js // Preferences file used by the raptor harness /* globals user_pref */ And reverted raptor prefs, then: rwood$ cat testing/profiles/raptor/user.js // Preferences file used by the raptor harness /* globals user_pref */ user_pref("dom.performance.time_to_non_blank_paint.enabled", true); rwood$ ./mach python testing/profiles/profile diff common+perf raptor -f names -k insert | ./mach python testing/profiles/profile rm raptor rwood$ cat testing/profiles/raptor/user.js // Preferences file used by the raptor harness /* globals user_pref */ Nice!

Does this change the preference settings used in reftest? If so, I'd be interested in reviewing a diff of the settings.

Comment on attachment 8976328 [details] Bug 1460912 - [testing/profiles] Use the 'common' profile in reftest https://reviewboard.mozilla.org/r/244514/#review250794 It is sad to see how many prefs are not common...but that's another matter, and this is a great step in the right direction.

Comment on attachment 8976326 [details] Bug 1460912 - [reftest] Use base profiles in reftest https://reviewboard.mozilla.org/r/244510/#review250798

(In reply to David Baron :dbaron: ⌚UTC-7 from comment #21) > Does this change the preference settings used in reftest? If so, I'd be > interested in reviewing a diff of the settings. No, I made sure the set of prefs were identical by dumping the profile immediately before first run with and without my patch. For extra caution I'll do it again before pushing to make sure I didn't mishandle a conflict. Tangentially related, it might be a good idea if we could upload the user.js files from our test harnesses as artifacts for each run. That way we could run diffs by pointing to try urls.

Couple try runs.. All reftests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a26e56b3b24c8bd38476825840f6a1af0a5230e7 Sample of harnesses: https://treeherder.mozilla.org/#/jobs?repo=try&revision=353e11e413767fa8ebd4cf4177a0d68bc6e6bae1

Comment on attachment 8976338 [details] Bug 1460912 - [testing/profiles] Add --format options to ./profile diff https://reviewboard.mozilla.org/r/244516/#review250730 > One other thing I noticed. If I do: > > ./mach python testing/profiles/profile diff common+perf raptor > > The console output is nicely alphabetical within each section (delete, insert, same). > > However if I do: > > ./mach python testing/profiles/profile diff common+perf raptor --format json > > Inside the json sections (i.e. delete, insert, same) the prefs seem random and aren't in alphabetical order which is a bit annoying. Good idea, I can add sorted keys in. p.s here's a nifty little trick for the next time you come across hard to read json: ./profile diff common+perf raptor --format json | python -m json.tool That will both indent and sort any json output. I have `python -m json.tool` aliased to `indent` in my shell.

Comment on attachment 8976338 [details] Bug 1460912 - [testing/profiles] Add --format options to ./profile diff https://reviewboard.mozilla.org/r/244516/#review250722 > I find getting help in the tool itself is a bit awkward. > > ./mach python testing/profiles/profile -- --help > usage: profile [-h] {diff,sort,show} ... > > positional arguments: > {diff,sort,show} > > optional arguments: > -h, --help show this help message and exit > > So then I try to get help for 'diff' so I can see the '--format' option, so I try: > > ./mach python testing/profiles/profile -h diff > usage: mach [global arguments] python [command arguments] > > Run Python. > ... > > but that gives help for the mach python command, not the 'profile diff' command. Yeah, this is a side-effect of running `mach python <script>`. I think the only way to fix it would be to turn the profile script into a full blown mach command. Maybe that's how I should have implemented it from the start, but if we do go that route I'd prefer it happen in a follow up. So if you don't mind I'd like to drop this issue for this bug.

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2600a2947d25 [testing/profiles] Add ability to diff and show multiple profiles with '+' r=rwood https://hg.mozilla.org/integration/autoland/rev/0b6ae2cdbeba [testing/profiles] Add --format options to ./profile diff r=rwood https://hg.mozilla.org/integration/autoland/rev/794cca0138c4 [testing/profiles] Add option to limit ./profile diff to a specified key r=rwood https://hg.mozilla.org/integration/autoland/rev/631008a3bf29 [testing/profiles] Add a ./profile rm subcommand for removing prefs from a profile r=rwood https://hg.mozilla.org/integration/autoland/rev/24fc34c2bb34 [reftest] Use base profiles in reftest r=gbrown https://hg.mozilla.org/integration/autoland/rev/d8e9cd275290 [testing/profiles] Sort testing/profiles/reftest/user.js r=gbrown https://hg.mozilla.org/integration/autoland/rev/451f6e440971 [testing/profiles] Use the 'common' profile in reftest r=gbrown

https://hg.mozilla.org/mozilla-central/rev/2600a2947d25 https://hg.mozilla.org/mozilla-central/rev/0b6ae2cdbeba https://hg.mozilla.org/mozilla-central/rev/794cca0138c4 https://hg.mozilla.org/mozilla-central/rev/631008a3bf29 https://hg.mozilla.org/mozilla-central/rev/24fc34c2bb34 https://hg.mozilla.org/mozilla-central/rev/d8e9cd275290 https://hg.mozilla.org/mozilla-central/rev/451f6e440971