Closed Bug 1143218 Opened 5 years ago Closed 5 years ago

Use mochitest subsuites to specify webgl tests

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(firefox36 fixed, firefox37 fixed, firefox38 fixed, firefox39 fixed)

RESOLVED FIXED
Tracking Status
firefox36 --- fixed
firefox37 --- fixed
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: jgilbert, Assigned: jgilbert)

References

Details

Attachments

(2 files, 5 obsolete files)

We already have a "mochitest-subsuite-webgl.ini" file. We should hook it up to the subsuite system and use that to specify tests for mochitest-gl(s).
Attachment #8577510 - Flags: review?(jmaher)
See Also: → 1141771
I took a stab at this, but I'm not sure I know what I'm doing when in mozharness. :)
Assignee: nobody → jgilbert
Attachment #8577517 - Flags: review?(jmaher)
Comment on attachment 8577510 [details] [diff] [review]
0001-Mark-mochetest-subsuite-webgl-as-subsuite-webgl.patch

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

our problem comes with https://dxr.mozilla.org/mozilla-central/source/dom/canvas/test/_webgl-conformance.ini, is there a way we could put the subsuite = webgl in there as well?
Attachment #8577510 - Flags: review?(jmaher) → review-
Comment on attachment 8577517 [details] [diff] [review]
mozharness-subsuite-gl.diff

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

These are the right changes!

I think we have a couple of extra files to consider changing in m-c:
testing/config/mozharness/android_arm_config.py
testing/config/mozharness/android_arm_4_3_config.py

:gbrown, do you see anything else we should change as well?
Attachment #8577517 - Flags: review?(jmaher)
Attachment #8577517 - Flags: review?(gbrown)
Attachment #8577517 - Flags: review+
(In reply to Joel Maher (:jmaher) from comment #2)
> Comment on attachment 8577510 [details] [diff] [review]
> 0001-Mark-mochetest-subsuite-webgl-as-subsuite-webgl.patch
> 
> Review of attachment 8577510 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> our problem comes with
> https://dxr.mozilla.org/mozilla-central/source/dom/canvas/test/_webgl-
> conformance.ini, is there a way we could put the subsuite = webgl in there
> as well?

Yep, I can make this happen.
Flags: needinfo?(jgilbert)
Comment on attachment 8577517 [details] [diff] [review]
mozharness-subsuite-gl.diff

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

I agree that android_arm_config.py (Android 2.3) and android_arm_4_3_config.py (new Android 4.3 platform coming soon) should also be included.
Attachment #8577517 - Flags: review?(gbrown) → review+
Attachment #8577510 - Attachment is obsolete: true
Flags: needinfo?(jgilbert)
Attachment #8578261 - Flags: review?(jmaher)
Comment on attachment 8578261 [details] [diff] [review]
0001-Mark-mochetest-subsuite-webgl-as-subsuite-webgl.patch

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

this looks great- now to get this landed :)

First thing to do is to test this on try.  You can create a custom mozharness repo with your mozharness patch:
http://armenzg.blogspot.com/2014/12/test-mozharness-changes-on-try.html

then adjust testing/mozharness/mozharness.json to point to your repo, and then these changes into the patch queue and push to try.  

Make sure to test android and desktop!
Attachment #8578261 - Flags: review?(jmaher) → review+
Attached patch 0002-Use-subsuite-webgl.patch (obsolete) — Splinter Review
Attachment #8578270 - Flags: review?(jmaher)
Attachment #8578271 - Flags: review?(jmaher)
Comment on attachment 8578270 [details] [diff] [review]
0002-Use-subsuite-webgl.patch

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

great stuff here!
Attachment #8578270 - Flags: review?(jmaher) → review+
Comment on attachment 8578271 [details] [diff] [review]
0003-Remove-now-unused-gl.json.patch

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

I don't see gl.json in the tree, nor a reference to it in moz.build.  Is your tree out of date?
Attachment #8578271 - Flags: review?(jmaher) → review+
Attached patch 0002-Use-subsuite-webgl.patch (obsolete) — Splinter Review
r=jmaher

And yes, I was on trunk from a number of days ago. Patch 3 here is already in the tree in some form.
Attachment #8578270 - Attachment is obsolete: true
Attachment #8578271 - Attachment is obsolete: true
Attachment #8578335 - Flags: review+
are we ready to land?  we are uplifting to aurora/beta and run into these issues.
ok, one thing we overlooked:
https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#585

--subsuite is assumed to be browser-chrome.

A couple of ideas here:
1) require --subsuite=devtools, to have --browser-chrome as well
2) special case --subsuite=devtools to be browser chrome and the rest to be plain

I am sure there is something else we could do.
Flags: needinfo?(ted)
Flags: needinfo?(ahalberstadt)
I don't understand why --subsuite needs to be browser-chrome, that just seems.. wrong. Can we fix runtests.py so --subsuite can be applied to any flavor?
Flags: needinfo?(ahalberstadt)
we can fix it, I wanted to know what the preferred way to fix it would be?  I just looked at the devtools stuff and they still specify --browser-chrome.  this might be an easy fix.
when try opens, I will push to try with additional changes to support subsuite.
Flags: needinfo?(ted)
Yeah, so it might just be a matter of removing options.subsuite from here:
https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#585
this patch has the runtests.py changes, but it also has the changes for mozharness to point to the custom user repo.  We would need to land the mozharness changes, then take this patch and update mozharness.json to point to the new revision.

The try run looks good including looking at 8-10 raw logs and verifying we ran the right tests in the right jobs.
Attachment #8578261 - Attachment is obsolete: true
Attachment #8578335 - Attachment is obsolete: true
Attachment #8580584 - Flags: review?(gbrown)
(In reply to Joel Maher (:jmaher) from comment #15)
> A couple of ideas here:
> 1) require --subsuite=devtools, to have --browser-chrome as well
> 2) special case --subsuite=devtools to be browser chrome and the rest to be
> plain

Either of these seem fine, I'm not sure why this is like this.
Comment on attachment 8580584 [details] [diff] [review]
cumulative patch with runtests fixed (1.0)

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

I agree the try run looks good. Good stuff here!

Since this is loosely related, would you mind setting the mochitest-gl total-chunks in android_arm_4_3_config.py to 4? This was updated recently for 2.3 and the 4.3 config update was missed.
Attachment #8580584 - Flags: review?(gbrown) → review+
(In reply to Joel Maher (:jmaher) from comment #24)
> mozharness changes on default:
> http://hg.mozilla.org/build/mozharness/rev/b7285d1f1272

cherry-picked to production:
https://hg.mozilla.org/build/mozharness/rev/7b5d3fcc48c6
https://hg.mozilla.org/mozilla-central/rev/e3be61bda115
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
we probably need to uplift this to aurora/beta- RyanVM, where do we need this?
Flags: needinfo?(ryanvm)
(In reply to Carsten Book [:Tomcat] from comment #31)
> https://hg.mozilla.org/mozilla-central/rev/0db9fff35382
> https://hg.mozilla.org/mozilla-central/rev/95ec024d2e6b

These were from bug 1142318 in case anybody comes here looking for them.
Duplicate of this bug: 1141778
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.