1458571 - Use base profiles from testing/profiles in talos and raptor

Status: RESOLVED FIXED

(Testing :: Talos, enhancement, P1)

Description

[Andrew Halberstadt [:ahal]]

This is follow-up work to bug 1451159. We can create a new testing/profiles/perf profile that both talos and raptor can use. This has a few benefits:

1) We can test the perf impact extensions have by dropping them into testing/profiles/perf/extensions and pushing to try.

2) We can have single place to share prefs between the two frameworks (I think the set of prefs are identical at the moment).

3) We can share prefs with other harnesses too (if desired).

Comment 1

[Andrew Halberstadt [:ahal]]

We'll need to include the profile data in raptor's test package. So we should block on bug 1455872 first.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1458571 - Use base testing/profiles in talos,

This moves all of the global prefs that were previously defined
in testing/talos/talos/config.py, into a new "perf" profile under
testing/profiles/perf/user.js.

This perf profile will be shared with raptor, so changes to one
framework will result in changes to the other.

Review commit: https://reviewboard.mozilla.org/r/243058/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/243058/

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1458571 - Use base testing/profiles in raptor,

This gets raptor to use the newly created "perf" profile that talos
also uses. There is a single pref that raptor sets that we can't set
in talos. To that end, this also creates a "raptor" specific profile.

This means to set a pref in talos and raptor, edit:
testing/profiles/perf/user.js

To set a pref in raptor only, edit:
testing/profiles/raptor/user.js

The performance of extensions can now be tested by dropping the
extension into:
testing/profiles/perf/extensions

Review commit: https://reviewboard.mozilla.org/r/243060/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/243060/

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1458571 - Add a script for diffing and sorting preferences,

The main purpose of this script is to gain a view into the state of prefs
between various profiles or suites. There are three commands:

- show: Display prefs for the given profile or suite
- diff: Display differences of prefs between two profiles or suites
- sort: Sort pref files alphabetically for the given suite (this takes comments into account)

This also sorts common/user.js.

Review commit: https://reviewboard.mozilla.org/r/243062/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/243062/

Comment 5

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1458571 - Make talos and raptor use the 'common' base profile,

This factors out the prefs that are common across raptor, talos and
mochitest/web-platform-tests. In order to do this a new "unittest" profile has
been created.

This means, to set a pref across everything that uses this system, edit:
testing/profiles/common/user.js

To set a pref across unittest frameworks (which excludes raptor and talos),
edit:
testing/profiles/unittest/user.js

Setting a pref for perf frameworks only remains the same. Extensions follow
the same rules (drop them in <profile>/extensions).

Review commit: https://reviewboard.mozilla.org/r/243064/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/243064/

Comment 6

[Robert Wood [:rwood]]

Comment on attachment 8974685 [details]
Bug 1458571 - Use base testing/profiles in talos,

https://reviewboard.mozilla.org/r/243058/#review248894

Comment 7

[Robert Wood [:rwood]]

(In reply to Robert Wood [:rwood] from comment #6)
> Comment on attachment 8974685 [details]
> Bug 1458571 - Use base testing/profiles in talos,
> 
> https://reviewboard.mozilla.org/r/243058/#review248894

(For the talos patch I applied the patch locally, blew away the talos-venv, ran talos tp6 twice)

Comment 8

[Robert Wood [:rwood]]

Comment on attachment 8974686 [details]
Bug 1458571 - Use base testing/profiles in raptor,

https://reviewboard.mozilla.org/r/243060/#review248940

As well as reviewing the code, I applied the raptor patch locally and:

- blew away the testing/raptor-venv
- ran raptor-test raptor-firefox-tp6 test twice (pass)
- changed the *non_blank_* pref in raptor/user.js to false, ran raptor-firefox-tp6, test failed as expected
- set the pref back, ran raptor-firefox-tp6, test passed as expected
- dropped a basic web extension (talos dummy webext) into testing/profiles/raptor/extensions and ran raptor-firefox-tp6 and verified the webext was installed
- removed the webext from /raptor/extensions and ran raptor-firefox-tp6, verified extension was removed
- dropped a basic web extension (talos dummy webext) into testing/profiles/perf/extensions and ran raptor-firefox-tp6 and verified the webext was installed
- removed the webext from /perf/extensions and ran raptor-firefox-tp6, verified extension was removed

Awesome looks great!

Comment 9

[Robert Wood [:rwood]]

Comment on attachment 8974687 [details]
Bug 1458571 - Add a script for diffing and sorting preferences,

https://reviewboard.mozilla.org/r/243062/#review248996

Can't seem to get it to work on OSX...


(VENV) Roberts-MacBook-Pro-1927:profiles rwood$ python profile
Traceback (most recent call last):
  File "profile", line 27, in <module>
    from mozbuild.base import MozbuildObject
  File "/Users/rwood/mozilla-unified/testing/profiles/VENV/lib/python2.7/site-packages/mozbuild/base.py", line 14, in <module>
    import which
ImportError: No module named which
(VENV) Roberts-MacBook-Pro-1927:profiles rwood$ pip install which
Collecting which
  Could not find a version that satisfies the requirement which (from versions: )
No matching distribution found for which

::: testing/profiles/profile:28
(Diff revision 1)
> +
> +try:
> +    import jsondiff
> +except ImportError:
> +    from mozbuild.base import MozbuildObject
> +    build = MozbuildObject.from_environment(cwd=here)

Do you want to have a requirements.txt?

To get the script to start I had to create a virtualenv and install mozprofile and mozbuild

Comment 10

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974687 [details]
Bug 1458571 - Add a script for diffing and sorting preferences,

https://reviewboard.mozilla.org/r/243062/#review248996

Weird, `which` is vendored in-tree and should be available automatically. This script uses the `mach` binary on your $PATH, is it possible you have a binary from a really long time ago on your $PATH?

I guess I shouldn't be relying on people having it in their $PATH. Could you try changing the top line to `#!../../mach python`? If that works, I'll re-push with that fix.

> Do you want to have a requirements.txt?
> 
> To get the script to start I had to create a virtualenv and install mozprofile and mozbuild

Hm, I think maybe the script is using the wrong `mach` binary for you or something. The only thing that it should need to download is jsondiff. Let me know if my suggestion above fixes this.

Comment 11

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974685 [details]
Bug 1458571 - Use base testing/profiles in talos,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243058/diff/1-2/

Comment 12

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974686 [details]
Bug 1458571 - Use base testing/profiles in raptor,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243060/diff/1-2/

Comment 13

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974687 [details]
Bug 1458571 - Add a script for diffing and sorting preferences,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243062/diff/1-2/

Comment 14

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974688 [details]
Bug 1458571 - Make talos and raptor use the 'common' base profile,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243064/diff/1-2/

Comment 15

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974687 [details]
Bug 1458571 - Add a script for diffing and sorting preferences,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243062/diff/2-3/

Comment 16

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974688 [details]
Bug 1458571 - Make talos and raptor use the 'common' base profile,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243064/diff/2-3/

Comment 17

[Robert Wood [:rwood]]

(In reply to Andrew Halberstadt [:ahal] from comment #5)
> Created attachment 8974688 [details]

Just FYI, there's a merge conflict in testing/profiles/common/user.js with the latest inbound

Comment 18

[Robert Wood [:rwood]]

Comment on attachment 8974687 [details]
Bug 1458571 - Add a script for diffing and sorting preferences,

https://reviewboard.mozilla.org/r/243062/#review249016

Code LGTM just a nit. Tested the script on OSX:

./profile show raptor
changed pref value in profiles/raptor/user.js
./profile show raptor again, new value reported
added pref in profiles/perf/user.js
./profile show raptor again, new pref reported
added pref in profiles/common/user.js
./profile show raptor again, new pref reported
./profile diff raptor talos, confirmed reported correctly
./profile diff raptor raptor, yep no output as expected

Found one issue:

added pref at bottom that starts with “a” to profiles/raptor/user.js
./profile sort raptor

the resulting /raptor/user/js has the new pref on the same line as the top one like this:

// Preferences file used by the raptor harness
/* globals user_pref */
user_pref("another.one", true);user_pref("dom.performance.time_to_non_blank_paint.enabled", true);

R+ with that sort issue fixed.

::: testing/profiles/profile:138
(Diff revision 3)
> +        for k, v in sorted(prefs_a.items()):
> +            print("  {}: {}".format(k, repr(v)))
> +
> +
> +def sort_file(path):
> +    """Sort the given pref file alphabetically, preserving preserving

nit 'preserving preserving'

Comment 19

[Robert Wood [:rwood]]

Comment on attachment 8974688 [details]
Bug 1458571 - Make talos and raptor use the 'common' base profile,

https://reviewboard.mozilla.org/r/243064/#review249020

Yep LGTM!

Also tried it out by changing a pref in /profiles/common/user.js (set 'xpinstall.signatures.required' to 'true') and confirmed that change broke talos and raptor; therefore the common prefs are being applied to talos and raptor. COOL!

Note: I did not test modifying prefs in testing/profiles/unittest/user.js and verifying they were applied in unittests.

Comment 20

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974687 [details]
Bug 1458571 - Add a script for diffing and sorting preferences,

https://reviewboard.mozilla.org/r/243062/#review249016

Thanks again for the in-depth testing, I appreciate it!

Re: the sort issue, interestingly I wasn't able to reproduce this. I added the same pref to the bottom of `raptor/user.js`, ran `./profile sort raptor` and it came out as expected.

p.s please leave an issue for this kind of stuff in the future, I almost landed without noticing this!

Comment 21

[Andrew Halberstadt [:ahal]]

Here's a try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f7e89567f050312f6b3c2873b54040b2e73f50d

Comment 22

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974687 [details]
Bug 1458571 - Add a script for diffing and sorting preferences,

https://reviewboard.mozilla.org/r/243062/#review249016

Ah, I was able to reproduce. I think it wasn't happening for me because my editor automatically adds newlines to the end of files.

Comment 23

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974685 [details]
Bug 1458571 - Use base testing/profiles in talos,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243058/diff/2-3/

Comment 24

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974686 [details]
Bug 1458571 - Use base testing/profiles in raptor,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243060/diff/2-3/

Comment 25

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974687 [details]
Bug 1458571 - Add a script for diffing and sorting preferences,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243062/diff/3-4/

Comment 26

[Andrew Halberstadt [:ahal]]

Comment on attachment 8974688 [details]
Bug 1458571 - Make talos and raptor use the 'common' base profile,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243064/diff/3-4/

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d800f84be526
Use base testing/profiles in talos, r=rwood
https://hg.mozilla.org/integration/autoland/rev/59c1fb5c02c8
Use base testing/profiles in raptor, r=rwood
https://hg.mozilla.org/integration/autoland/rev/cd8746ad56d1
Add a script for diffing and sorting preferences, r=rwood
https://hg.mozilla.org/integration/autoland/rev/1e972b3b45d1
Make talos and raptor use the 'common' base profile, r=rwood

Comment 28

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/d800f84be526
https://hg.mozilla.org/mozilla-central/rev/59c1fb5c02c8
https://hg.mozilla.org/mozilla-central/rev/cd8746ad56d1
https://hg.mozilla.org/mozilla-central/rev/1e972b3b45d1