Closed
Bug 1455872
Opened 7 years ago
Closed 7 years ago
Add taskcluster raptor job (tier3, OSX to start)
Categories
(Testing :: Raptor, enhancement)
Tracking
(firefox62 fixed)
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: rwood, Assigned: rwood)
References
Details
Attachments
(3 files)
Also includes adding support in tc try and scheduling for the new 'raptor' tests.
Assignee | ||
Comment 1•7 years ago
|
||
This is going to be more difficult than I thought, as it involves adding a completely new test type/suite that needs to be plugged into the build process (for creating 'raptor.tests.zip' etc).
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
Comment on attachment 8970553 [details]
Bug 1455872 - Add taskcluster configs for raptor on OSX;
On some try runs (combined with other raptor patches currently under review) the new taskcluster raptor job was created and started up on try no problem (tier 3).
However trying to proceed further I am stuck, with a broken build - because I need to add a new 'target.raptor.tests.zip' etc. and I don't know how to proceed with that.
https://taskcluster-artifacts.net/EqXIotc0TI-oTNUyPxgJYg/0/public/logs/live_backing.log
Attachment #8970553 -
Flags: feedback?(jmaher)
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8970553 [details]
Bug 1455872 - Add taskcluster configs for raptor on OSX;
https://reviewboard.mozilla.org/r/239314/#review245012
overall I like this- some smaller comments below.
::: taskcluster/ci/config.yml:19
(Diff revision 1)
> 'M-e10s': 'Mochitests with e10s'
> 'M-V': 'Mochitests on Valgrind'
> 'R': 'Reftests'
> 'R-e10s': 'Reftests with e10s'
> + 'RF': 'Raptor performance tests on Firefox'
> + 'RF-e10s': 'Raptor performance tests on Firefox with e10s'
why RF? Raptor Framework?
::: taskcluster/taskgraph/target_tasks.py:142
(Diff revision 1)
> task.attributes['profile'] = options.profile
>
> + # If the developer wants test raptor jobs to be rebuilt N times we add that value here
> + if options.raptor_trigger_tests > 1 and task.attributes.get('unittest_suite') == 'raptor':
> + task.attributes['task_duplicates'] = options.raptor_trigger_tests
> + task.attributes['profile'] = options.profile
one concern here is that people will have to learn a new syntax- folks associate Talos == perf; I like raptor, but adding in --rebuild-raptor seems overkill. I would like a one shot for all perf tests, will |-t all| work for all perf?
::: taskcluster/taskgraph/transforms/tests.py:743
(Diff revision 1)
> test['mozharness']['extra-options'].append('--add-option')
> test['mozharness']['extra-options'].append('--no-upload-results')
> test['mozharness']['extra-options'].append('--add-option')
> test['mozharness']['extra-options'].append('--tptimeout,15000')
> + if 'raptor' in test['test-name']:
> + test['max-run-time'] = 7200
do we really need 7200?
Attachment #8970553 -
Flags: review+
Assignee | ||
Comment 5•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8970553 [details]
Bug 1455872 - Add taskcluster configs for raptor on OSX;
https://reviewboard.mozilla.org/r/239314/#review245012
> why RF? Raptor Framework?
I was thinking "Raptor Firefox" and then have "RC" for "Raptor Chrome" - separate groups. However we could have one group like "Rap" and then differentiate the browser via the job name like 'tp6-f' and 'tp6-c'?
> one concern here is that people will have to learn a new syntax- folks associate Talos == perf; I like raptor, but adding in --rebuild-raptor seems overkill. I would like a one shot for all perf tests, will |-t all| work for all perf?
Interesting, good point. Maybe we should leave that for a future bug then? And first get a patch to get a raptor job running (without a --rebuild)?
> do we really need 7200?
Good point, no I'll reduce this, thanks
Comment 6•7 years ago
|
||
lets add --rebuild later on, possibly we go with your initial work here. My goal is to make something that isn't confusing for developers and sheriffs.
If we had groups, it could look like this:
RF(tp6 sp mm jet) RC(tp6 sp mm jet)
If we didn't have groups, I could see:
Rap(tp6 tp6-c sp sp-c mm mm-c jet jet-c)
thinking on a bigger picture if we did this on android or had a second version of chrome or a second type of browser in the future, what would make sense? we are also adding shell tests and might need to run shell tests against v8 or other engines- so the decisions here should set precedence for theoretically all test jobs running against multiple browsers.
In that sense, I think the idea of unique groups makes the most sense:
Rap(tp6...) Rap-C[tier-2](tp6...)
If we can keep the taskcluster configs simple (a transform to add a chrome path or something) it will make editing the tests a lot less error prone.
One other thought is there is a need for running all perf tests together |-t all|. I am on the fence with keeping that or having |-t all -r all|. This presents a challenge with chrome because we don't want to run chrome by default, and I don't think we want to run chrome tests on try (because we are not modifying the chrome source). Possibly we go forward with a solution now and be willing to change it in the near future.
My goal is to find a way to reduce the complexity of the taskcluster configs in general- between talos|raptor.yml, test-sets.yml and platforms.yml, we have so many opportunities for making mistakes, likewise keeping the view on treeherder and test selection process for try manageable. If we start with something that is ok, but lets budget time in a future milestone to refactor the configs :)
Updated•7 years ago
|
Attachment #8970553 -
Flags: feedback?(jmaher)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 16•7 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Comment 18•7 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Comment 20•7 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Comment 22•7 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Comment 24•7 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Comment 26•7 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Comment 28•7 years ago
|
||
Assignee | ||
Comment 29•7 years ago
|
||
Assignee | ||
Comment 30•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 34•7 years ago
|
||
mozreview-review |
Comment on attachment 8972384 [details]
Bug 1455872 - Raptor fixes for production;
https://reviewboard.mozilla.org/r/241004/#review247072
Looks good, but you should probably split the mozbuild/packaging parts into a separate commit and get a build peer review (you can flag the :build user to get it on their shared review queue). Let me know if you need help splitting the commit.
::: testing/testsuite-targets.mk:121
(Diff revision 12)
> common \
> cppunittest \
> mochitest \
> reftest \
> talos \
> + raptor \
Is this necessary? I'd expect this to work without this change.
Attachment #8972384 -
Flags: review?(ahalberstadt)
Assignee | ||
Comment 35•7 years ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #34)
> Comment on attachment 8972384 [details]
> Bug 1455872 - Add build support for raptor;
>
> https://reviewboard.mozilla.org/r/241004/#review247072
>
> Looks good, but you should probably split the mozbuild/packaging parts into
> a separate commit and get a build peer review (you can flag the :build user
> to get it on their shared review queue). Let me know if you need help
> splitting the commit.
>
> ::: testing/testsuite-targets.mk:121
> (Diff revision 12)
> > common \
> > cppunittest \
> > mochitest \
> > reftest \
> > talos \
> > + raptor \
>
> Is this necessary? I'd expect this to work without this change.
Thanks for the review, ok I'll split that commit into 2. Yes it doesn't work without that addition to testsuite-targets.mk, the build breaks without it, don't ask me why haha
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 40•7 years ago
|
||
Comment 41•7 years ago
|
||
mozreview-review |
Comment on attachment 8972384 [details]
Bug 1455872 - Raptor fixes for production;
https://reviewboard.mozilla.org/r/241004/#review247218
Thanks, lgtm!
Attachment #8972384 -
Flags: review?(ahalberstadt) → review+
Assignee | ||
Comment 42•7 years ago
|
||
Ping :build, any chance for a review please, I'd love to land this today if possible, thanks! :)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 49•7 years ago
|
||
Assignee | ||
Comment 50•7 years ago
|
||
(In reply to Robert Wood [:rwood] from comment #49)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef48eb36bc54812872de3aef6e6c0b06214599fa
Rebased, fixed conflict, landed latest on try ^
Updated•7 years ago
|
Attachment #8972702 -
Flags: review?(core-build-config-reviews) → review?(gps)
Comment 51•7 years ago
|
||
mozreview-review |
Comment on attachment 8972702 [details]
Bug 1455872 - Build support for the new raptor performance framework;
https://reviewboard.mozilla.org/r/241228/#review248660
Attachment #8972702 -
Flags: review?(gps) → review+
Assignee | ||
Comment 52•7 years ago
|
||
Thanks :gps!
Comment 53•7 years ago
|
||
Pushed by rwood@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32355d123d16
Add taskcluster configs for raptor on OSX; r=jmaher
https://hg.mozilla.org/integration/autoland/rev/b1ad2c2fdb80
Raptor fixes for production; r=ahal
https://hg.mozilla.org/integration/autoland/rev/74b75c80928d
Build support for the new raptor performance framework; r=gps
Comment 54•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/32355d123d16
https://hg.mozilla.org/mozilla-central/rev/b1ad2c2fdb80
https://hg.mozilla.org/mozilla-central/rev/74b75c80928d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•