Closed Bug 1417496 Opened 4 years ago Closed 3 years ago

Enable reftests for Windows coverage build

Categories

(Testing :: Code Coverage, enhancement)

enhancement
Not set
normal

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: marco, Assigned: marco)

References

Details

(Whiteboard: [stockwell disabled])

Attachments

(1 file, 3 obsolete files)

reftests are defined on buildbot.
catlee, can you help?

The build definition is being added in bug 1417436.
Flags: needinfo?(catlee)
Kim, maybe you can help?

We need to get reftests running for the windows10-64-ccov build.
Flags: needinfo?(kmoir)
Do you want to run these on trunk starting with the current version of gecko?

The definitions for these in taskcluster reside here 

https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/test/reftest.yml?q=reftest.yml&redirect_type=direct#80

Since these tests run on bare metal hardware, they run on buildbot bridge so we need to define them in buildbot as well. 

Look at 
https://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla-tests/config.py

for examples for defining tests for other win10 platforms
Flags: needinfo?(kmoir)
(In reply to Kim Moir [:kmoir] ET from comment #3)
> Do you want to run these on trunk starting with the current version of gecko?

Yes, we want to run them on mozilla-central and try only.

> The definitions for these in taskcluster reside here 
> 
> https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/test/reftest.
> yml?q=reftest.yml&redirect_type=direct#80

These are already defined for windows10-64-ccov (as windows10-64.* matches windows10-64-ccov), so I guess the only thing left is defining them on buildbot.

> 
> Since these tests run on bare metal hardware, they run on buildbot bridge so
> we need to define them in buildbot as well. 
> 
> Look at 
> https://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla-tests/config.
> py
> 
> for examples for defining tests for other win10 platforms

Can you help me with this part? The file is huge and I'm afraid I might miss things. I also have no idea how to test my changes.
Flags: needinfo?(catlee) → needinfo?(kmoir)
Since I'm going on PTO soon, I'm going to ask the buildduty folks to look at this bug.
Flags: needinfo?(kmoir) → needinfo?(aselagea)
I'll take a look.
Flags: needinfo?(aselagea)
Attached patch bug1417496_bbconfigs.patch (obsolete) — Splinter Review
Builder diff:

--- old_sorted  2017-12-22 00:49:12.514261789 -0800
+++ new_sorted  2017-12-22 01:04:08.265546652 -0800
+Windows 10 64-bit Code Coverage mozilla-central debug test reftest-e10s-1 ScriptFactory
+Windows 10 64-bit Code Coverage mozilla-central debug test reftest-e10s-2 ScriptFactory
+Windows 10 64-bit Code Coverage mozilla-central debug test reftest-no-accel-e10s-1 ScriptFactory
+Windows 10 64-bit Code Coverage mozilla-central debug test reftest-no-accel-e10s-2 ScriptFactory
+Windows 10 64-bit Code Coverage try debug test reftest-e10s-1 ScriptFactory
+Windows 10 64-bit Code Coverage try debug test reftest-e10s-2 ScriptFactory
+Windows 10 64-bit Code Coverage try debug test reftest-no-accel-e10s-1 ScriptFactory
+Windows 10 64-bit Code Coverage try debug test reftest-no-accel-e10s-2 ScriptFactory
Attached patch bug1417496_puppet.patch (obsolete) — Splinter Review
Attached patch bug1417496_tools.patch (obsolete) — Splinter Review
Note: the patches are ready, but we'll need a person to review them - which is a bit more difficult to do now due to the winter holidays.

@jlund: since Kim is on PTO until January 8 and I won't be around next year, can you please address the next steps for this bug? Basically, what we need is review + push the patches.
Thanks!
Flags: needinfo?(jlund)
as we are still a couple months out from having full capacity for having windows 10 hardware running in taskcluster, we should go forward with making this work.

:jlund, can you help find a review for this code?
Comment on attachment 8938618 [details] [diff] [review]
bug1417496_bbconfigs.patch

Review of attachment 8938618 [details] [diff] [review]:
-----------------------------------------------------------------

Apologies for the delay. lgtm

::: mozilla-tests/config.py
@@ +3526,5 @@
>              tests['opt_unittest_suites'] = [t for t in tests['opt_unittest_suites'] if t in opt]
>              tests['debug_unittest_suites'] = [t for t in tests['debug_unittest_suites'] if t in debug]
>  
> +# Bug 1417496 - Enable reftests for Windows coverage build
> +for name, branch in items_at_least(BRANCHES, 'gecko_version', 57):

I wonder if we should just explicitly add them to mozilla-central and try automatically. Rather than pivot off >=57 loop. Doesn't really matter I suppose.
Attachment #8938618 - Flags: review+
Attachment #8938619 - Flags: review+
Attachment #8938620 - Flags: review+
I was hoping to use this bug to help onboard new buildduty team but we have been delayed getting them access. I'll land these patches tomorrow as I don't want to introduce any more risk into the releases that are in flight.
Flags: needinfo?(jlund)
:jlund, can we land the patches now?
Flags: needinfo?(jlund)
(In reply to Marco Castelluccio [:marco] from comment #14)
> :jlund, can we land the patches now?

Sorry that this dragged on. My fault. It should be live now.
Flags: needinfo?(jlund)
A couple of problems, from https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=7b46ef2ae1412b15ed45e7d2367ca491344729f7:
1) They are failing because "Required config file not set! (use --config-file option)".
2) They are shown on Treeherder as "Windows 10 x64 cc" instead of "Windows 10 x64 ccov".
Jordan, can you look into this or suggest someone who can?
Flags: needinfo?(jlund)
Hm, from initial poking. Hard to see what's going on here. So much of this is legacy and not many people would be familiar with it.

How much of a priority is this vs waiting until everything is in Taskcluster, roughly start of March?
Flags: needinfo?(jlund)
(In reply to Jordan Lund (:jlund) from comment #23)
> Hm, from initial poking. Hard to see what's going on here. So much of this
> is legacy and not many people would be familiar with it.
> 
> How much of a priority is this vs waiting until everything is in
> Taskcluster, roughly start of March?

ni: Hey marco, I'm trying to gauge priority so I know how to triage this. Do you have context here?
Flags: needinfo?(mcastelluccio)
(In reply to Jordan Lund (:jlund) from comment #25)
> (In reply to Jordan Lund (:jlund) from comment #23)
> > Hm, from initial poking. Hard to see what's going on here. So much of this
> > is legacy and not many people would be familiar with it.
> > 
> > How much of a priority is this vs waiting until everything is in
> > Taskcluster, roughly start of March?
> 
> ni: Hey marco, I'm trying to gauge priority so I know how to triage this. Do
> you have context here?

If it is start of March, I think we can wait :)
Flags: needinfo?(mcastelluccio)
can we make this tier-3 or disable this?  we are running jobs that perma fail and don't need to clutter orange factor or the trees.
Depends on: 1435844
Flags: needinfo?(mcastelluccio)
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #30)
> can we make this tier-3 or disable this?  we are running jobs that perma
> fail and don't need to clutter orange factor or the trees.

Yes, I was suggesting the same over in bug 1435414 comment 3.
Should we backout the patches from buildbot config?
Flags: needinfo?(mcastelluccio) → needinfo?(jlund)
(In reply to Marco Castelluccio [:marco] from comment #31)
> (In reply to Joel Maher ( :jmaher) (UTC-5) from comment #30)
> > can we make this tier-3 or disable this?  we are running jobs that perma
> > fail and don't need to clutter orange factor or the trees.
> 
> Yes, I was suggesting the same over in bug 1435414 comment 3.
> Should we backout the patches from buildbot config?

Will back this out with buildduty tomorrow.
(In reply to Jordan Lund (:jlund) from comment #33)
> (In reply to Marco Castelluccio [:marco] from comment #31)
> > (In reply to Joel Maher ( :jmaher) (UTC-5) from comment #30)
> > > can we make this tier-3 or disable this?  we are running jobs that perma
> > > fail and don't need to clutter orange factor or the trees.
> > 
> > Yes, I was suggesting the same over in bug 1435414 comment 3.
> > Should we backout the patches from buildbot config?
> 
> Will back this out with buildduty tomorrow.

@zsolt - want to try and back this out? We can walk through the motions in slack.
Flags: needinfo?(jlund) → needinfo?(zfay)
Flags: needinfo?(zfay)
zsolt needs level 3 to back out. I've filed that separately and backed out this patch myself.
Whiteboard: [stockwell disable-recommended] → [stockwell disabled]
We now (after bug 1435844) have reftests working in Windows coverage builds, but they are timing out.
They hit the maximum run time of 60 minutes.

Joel, do we want to increase the number of chunks (currently 4) or the max run time?
Flags: needinfo?(jmaher)
I think increase to 90 minutes, if it takes longer than that, then we should split into more chunks- probably 8.
Flags: needinfo?(jmaher)
Assignee: nobody → mcastelluccio
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
I've set the limit to 90 minutes and increased the chunks from 4 to 6.
With 6 chunks, each chunk takes around 70 minutes. Do you want me to increase the chunks more?

Try build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3809b030778f195eb7a65473336f85033fad0ab.
Attachment #8938618 - Attachment is obsolete: true
Attachment #8938619 - Attachment is obsolete: true
Attachment #8938620 - Attachment is obsolete: true
Attachment #8964723 - Flags: review?(jmaher)
Comment on attachment 8964723 [details] [diff] [review]
Patch

Review of attachment 8964723 [details] [diff] [review]:
-----------------------------------------------------------------

thanks!
Attachment #8964723 - Flags: review?(jmaher) → review+
Pushed by mcastelluccio@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/53192186c81f
Increase reftest chunks to 6 and timeout to 90 minutes for Windows coverage builds. r=jmaher
https://hg.mozilla.org/mozilla-central/rev/53192186c81f
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.