Closed Bug 1458571 Opened 2 years ago Closed 2 years ago

Use base profiles from testing/profiles in talos and raptor

Categories

(Testing :: Talos, enhancement, P1)

Version 3
enhancement

Tracking

(firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

(Whiteboard: [PI:May])

Attachments

(4 files)

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).
Priority: -- → P1
Whiteboard: [P1:May]
Whiteboard: [P1:May] → [PI:May]
We'll need to include the profile data in raptor's test package. So we should block on bug 1455872 first.
Depends on: 1455872
Depends on: 1459598
Comment on attachment 8974685 [details]
Bug 1458571 - Use base testing/profiles in talos,

https://reviewboard.mozilla.org/r/243058/#review248894
Attachment #8974685 - Flags: review?(rwood) → review+
(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 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!
Attachment #8974686 - Flags: review?(rwood) → review+
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
Attachment #8974687 - Flags: review?(rwood)
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.
(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 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'
Attachment #8974687 - Flags: review?(rwood) → review+
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.
Attachment #8974688 - Flags: review?(rwood) → review+
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 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.
Pushed by ahalberstadt@mozilla.com:
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
You need to log in before you can comment on or make changes to this bug.