1143218 - Use mochitest subsuites to specify webgl tests

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Mark-mochetest-subsuite-webgl-as-subsuite-webgl.patch

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).

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: mozharness-subsuite-gl.diff

I took a stab at this, but I'm not sure I know what I'm doing when in mozharness. :)

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

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?

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

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?

Comment 4

[Kelsey Gilbert [:jgilbert]]

(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.

Comment 5

[Geoff Brown [:gbrown]]

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.

Comment 6

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Mark-mochetest-subsuite-webgl-as-subsuite-webgl.patch

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

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!

Comment 8

[Kelsey Gilbert [:jgilbert]]

Attachment: 0002-Use-subsuite-webgl.patch

Comment 9

[Kelsey Gilbert [:jgilbert]]

Attachment: 0003-Remove-now-unused-gl.json.patch

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8578270 [details] [diff] [review]
0002-Use-subsuite-webgl.patch

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

great stuff here!

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

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?

Comment 12

[Kelsey Gilbert [:jgilbert]]

Attachment: 0002-Use-subsuite-webgl.patch

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.

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

are we ready to land?  we are uplifting to aurora/beta and run into these issues.

Comment 14

[Kelsey Gilbert [:jgilbert]]

This should have the pinning right:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=85c10a98f19b

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

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.

Comment 16

[Andrew Halberstadt [:ahal]]

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?

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

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.

Comment 18

[Joel Maher ( :jmaher ) (UTC -8)]

when try opens, I will push to try with additional changes to support subsuite.

Comment 19

[Andrew Halberstadt [:ahal]]

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

Comment 20

[Joel Maher ( :jmaher ) (UTC -8)]

on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6be883ad210d

Comment 21

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: cumulative patch with runtests fixed (1.0)

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.

Comment 22

[(not currently active) Ted Mielczarek]

(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 23

[Geoff Brown [:gbrown]]

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.

Comment 24

[Joel Maher ( :jmaher ) (UTC -8)]

mozharness changes on default:
http://hg.mozilla.org/build/mozharness/rev/b7285d1f1272

Comment 25

[Jordan Lund (:jlund)]

(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

Comment 26

[Joel Maher ( :jmaher ) (UTC -8)]

inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3be61bda115

Comment 27

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/e3be61bda115

Comment 28

[Joel Maher ( :jmaher ) (UTC -8)]

we probably need to uplift this to aurora/beta- RyanVM, where do we need this?

Comment 29

[Ryan VanderMeulen [:RyanVM]]

I actually just kicked off Try pushes for this rebased on to Aurora, Beta, and Release.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bdfe1112a65c
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a58b8b594396
https://treeherder.mozilla.org/#/jobs?repo=try&revision=58fe203296af

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/93d75fb698ed

Comment 31

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0db9fff35382
https://hg.mozilla.org/mozilla-central/rev/95ec024d2e6b

Comment 32

[Ryan VanderMeulen [: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.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bdfe1112a65c
https://hg.mozilla.org/releases/mozilla-beta/rev/a58b8b594396
https://hg.mozilla.org/releases/mozilla-release/rev/58fe203296af

And of course, this changed the chunking around enough on Windows to cause bug 1146061. *sigh*

Comment 34

[Jordan Lund (:jlund)]

mozharness production tag moved to: https://hg.mozilla.org/build/mozharness/rev/production