The webgl conformance tests have caused ongoing stability issues for years now. Running as part of mochitest-2 on desktop, we've reached a point where on Linux, it doesn't meet default visibility requirements due to the high failure rate due mostly to bug 777574. We've attempted to work around the issues in the past by selectively disabling sub-tests, but that only pushed the failures to other tests. I don't want to hide mochitest-2 by default due to the high collateral damage that would cause with all the other tests that happen to run in that suite. Therefore, I would like to split out the webgl tests into their own suite like bug 865443 did for Android, so they can be hidden by default and left to the WebGL maintainers to monitor until they can be sufficiently stabilized for meeting the job visibility requirements.

+1

Sorry, comment 0 should have said mochitest-1, not mochitest-2.

:jmaher: what needs to change on the test manifest side of things?

+1 on splitting out a mochitest-gl. I could accept hiding mochitest-gl, but we *must* run it. We cannot afford to disable it.

We can't afford to hide it either, actually, as we do care about these tests not only running but actually passing. If the intermittent rate is so high that it's a burden to tree sheriffs and they need to hide it, that means that we've been relying on sheriff's goodwill and energy too much and we should fix the tests; that does *not* mean that we didn't care about the test coverage that that gave us.

(In reply to Benoit Jacob [:bjacob] from comment #5) > We can't afford to hide it either, actually, as we do care about these tests > not only running but actually passing. If the intermittent rate is so high > that it's a burden to tree sheriffs and they need to hide it, that means > that we've been relying on sheriff's goodwill and energy too much and we > should fix the tests; that does *not* mean that we didn't care about the > test coverage that that gave us. I'm working on fixing it, but I can handle the tests being hidden for a week while I fix this. Maybe it's optimistic, but I think splitting the single mega-test up will make things more manageable.

Now that bug 777574 is fixed, I assume that we're no longer talking of having to hide WebGL tests. Splitting them into a separate suite would still be nice, though.

We are talking about: dom/canvas/test/webgl-conformance/mochitest.ini this is a standalone file and we could mark it as a subsuite (as we did in bug 984930 for mochitest-devtools). As a subsuite it won't run by default and our builder for mochitest-gl could just add --subsuite=webgl. As you can see in bug 984930 there was a lot of stuff that had to happen (some was renaming and some was changing the number of chunks, both of which wouldn't need to happen here)

tested in staging on ubuntu32 slave

enable on cedar to start

Comment on attachment 8481490 [details] [diff] [review] bug1051886mh.patch Review of attachment 8481490 [details] [diff] [review]: ----------------------------------------------------------------- IIUC, this should pretty much do it :) r+ if you confirm 1) mac/win is different than linux and all args are needed or 2) you remove/add the un-needed args :D ::: configs/unittests/mac_unittest.py @@ +57,5 @@ > "browser-chrome-1": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=1"], > "browser-chrome-2": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=2"], > "browser-chrome-3": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=3"], > "browser-chrome-chunked": ["--browser-chrome", "--chunk-by-dir=5"], > + "mochitest-webgl": ["--mochitest-webgl --subsuite=webgl"], does runtests.py need --mochitest-webgl ? If it does, do we need to add it to linux_unittest.py? Do spaces work within a string here? Since this is a list, I suspect we use this in run_command() via a list of strings over one string with spaces. ::: configs/unittests/win_unittest.py @@ +67,5 @@ > "browser-chrome-1": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=1"], > "browser-chrome-2": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=2"], > "browser-chrome-3": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=3"], > "browser-chrome-chunked": ["--browser-chrome", "--chunk-by-dir=5"], > + "mochitest-webgl": ["--mochitest-webgl --subsuite=webgl"], same Q as mac_unittest.py

Comment on attachment 8481492 [details] [diff] [review] bug1051886bbconfigs.patch Review of attachment 8481492 [details] [diff] [review]: ----------------------------------------------------------------- lgtm! I'm guessing 1) we are not running this against debug yet? and 2) we will continue running webgl tests in mochi-1 (even on cedar) for now?

(In reply to Jordan Lund (:jlund) from comment #12) > Comment on attachment 8481492 [details] [diff] [review] > bug1051886bbconfigs.patch > > Review of attachment 8481492 [details] [diff] [review]: > ----------------------------------------------------------------- > > lgtm! I'm guessing 1) we are not running this against debug yet? and 2) we > will continue running webgl tests in mochi-1 (even on cedar) for now? We run on debug on Desktop platforms. This cannot be regressed.

:jlund thanks for the reviews, this revealed a problem with my patches that I have been trying to address today. I thought my tests were working but looking at the logs more closely, they were not working as expected. :jmaher: Instead of using subsuite (which I can't seem to get to work even though I have been modifying runtests.py) could I just use manifest instead (tests/dom/canvas/test/webgl-mochitest/mochitest.ini)?

:kmoir, yes, we could just point at the ini file. We should chat in IRC or outside of this bug as to why you are not getting subsuite to work. My only concern would be- will we still be running the webgl tests in mochitest-2

added debug

changed from using a subsuite to a manifest

Comment on attachment 8483581 [details] [diff] [review] bug1051886mh-2.patch Review of attachment 8483581 [details] [diff] [review]: ----------------------------------------------------------------- if these inputs to runtests wfy it wfm :)

Comment on attachment 8483580 [details] [diff] [review] bug1051886bbconfigs-2.patch Review of attachment 8483580 [details] [diff] [review]: ----------------------------------------------------------------- lgtm :) do we need a tbpl patch?

Merged to production, and deployed.

Comment on attachment 8483581 [details] [diff] [review] bug1051886mh-2.patch Review of attachment 8483581 [details] [diff] [review]: ----------------------------------------------------------------- ::: configs/unittests/win_unittest.py @@ +67,5 @@ > "browser-chrome-1": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=1"], > "browser-chrome-2": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=2"], > "browser-chrome-3": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=3"], > "browser-chrome-chunked": ["--browser-chrome", "--chunk-by-dir=5"], > + "mochitest-webgl": ["--manifest=tests/mochitest/tests/dom/canvas/test/webgl-mochitest/mochitest.ini"], This is just the webgl-mochitest dir tests. The main body of webgl tests is in 'webgl-conformance/_mochitest.ini'

Also, when should this be taking effect? I don't see any non-numbered mochitests running on desktop runs on central or inbound, even in self-serve API.

It was just enabled on cedar today, I'll fix the .ini file in mozharness, sorry about that.

correct manifest -bash-4.1$ unzip -l firefox-34.0a1.en-US.linux-i686.tests.zip | grep webgl-conformance | grep webgl-conformance | grep mochitest.ini 697 08-28-2014 03:35 mochitest/tests/dom/canvas/test/webgl-conformance/mochitest.ini tested in staging green tests http://dev-master1.srv.releng.scl3.mozilla.com:8036/builders/Ubuntu%20VM%2012.04%20cedar%20opt%20test%20mochitest-webgl/builds/29/steps/run_script/logs/stdio

Comment on attachment 8485335 [details] [diff] [review] bug1051886fixmh.patch Review of attachment 8485335 [details] [diff] [review]: ----------------------------------------------------------------- ::: configs/unittests/win_unittest.py @@ +67,5 @@ > "browser-chrome-1": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=1"], > "browser-chrome-2": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=2"], > "browser-chrome-3": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=3"], > "browser-chrome-chunked": ["--browser-chrome", "--chunk-by-dir=5"], > + "mochitest-webgl": ["--manifest=tests/mochitest/tests/dom/canvas/test/webgl-conformance/mochitest.ini"], This is no longer the correct mochitest manifest. Please use '_mochitest.ini'. I haven't landed the deletion of the old one yet. Also, ideally, mochitest-webgl would cover dom/canvas/test/webgl-*/, not just one of webgl-conformance or webgl-mochitest.

Okay, I didn't see _mochitest.ini in the tests zip I was using for testing that's why I used mochitest.ini. I'll fix it.

regarding >Also, ideally, mochitest-webgl would cover dom/canvas/test/webgl-*/, not just one of webgl-conformance or >webgl-mochitest. Currently you can only specify one manifest per test suite in buildbot so I have to either refactor how this works or look at subsuites again. Investigating.

(In reply to Kim Moir [:kmoir] from comment #27) > regarding > > >Also, ideally, mochitest-webgl would cover dom/canvas/test/webgl-*/, not just one of webgl-conformance or >webgl-mochitest. > > Currently you can only specify one manifest per test suite in buildbot so I > have to either refactor how this works or look at subsuites again. > Investigating. Can I give you a new dom/canvas/test/mochitest-webgl.ini which brings both of these in, and use that mochitest manifest for the subsuite?

Yes that would be great. thanks!

(In reply to Kim Moir [:kmoir] from comment #29) > Yes that would be great. thanks! Bug 1064432 is this, and should land shortly.

Comment on attachment 8485335 [details] [diff] [review] bug1051886fixmh.patch Review of attachment 8485335 [details] [diff] [review]: ----------------------------------------------------------------- Clearing my queue. Feel free to ask for review on the next patch. If it is just a manifest path change, maybe jgilbert would be better suited :)

same change in mh tested in staging

Comment on attachment 8493161 [details] [diff] [review] glrenamemh.patch lgtm

Comment on attachment 8493159 [details] [diff] [review] rename mochitest-webgl to mochitest-gl to match Android as per RyanVM lgtm

Comment on attachment 8493159 [details] [diff] [review] rename mochitest-webgl to mochitest-gl to match Android as per RyanVM Review of attachment 8493159 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozilla-tests/config.py @@ +940,1 @@ > 'config_files': ["unittests/linux_unittest.py"], The surrounding blocks here use win_unittest. Should this be linux_unittest anyways? @@ +1313,1 @@ > 'config_files': ["unittests/linux_unittest.py"], Same as above, but surrounded by mac_unittest.

Also, can we get mochitest-gl on B2G? With these patches, on Ceder I see mochitest-gl on windows/mac/linux/android, but not b2g. The Linux runs (at least) appear to be running non-webgl tests in mochitest-gl: > 5180 INFO TEST-UNEXPECTED-ERROR | /tests/dom/ipc/tests/test_CrashService_crash.html | This test did not leave any crash dumps behind, but we were expecting some! > 5181 INFO TEST-UNEXPECTED-ERROR | /tests/dom/ipc/tests/test_CrashService_crash.html | This test left crash dumps behind, but we weren't expecting it to! > 5183 INFO TEST-UNEXPECTED-FAIL | /tests/dom/ipc/tests/test_CrashService_crash.html | undefined assertion name - Result logged after SimpleTest.finish() > 5184 INFO TEST-UNEXPECTED-FAIL | /tests/dom/ipc/tests/test_CrashService_crash.html | undefined assertion name - Result logged after SimpleTest.finish() > 5312 INFO TEST-UNEXPECTED-FAIL | /tests/dom/media/tests/ipc/test_ipc.html | test_dataChannel_basicAudioVideo.html - This test left crash dumps behind, but we weren't expecting it to!

Something here landed in production today: https://wiki.mozilla.org/ReleaseEngineering/Maintenance#Reconfigs_.2F_Deployments

The mochitest-gl jobs on Cedar are definitely running non-webgl mochitests. Can someone fix this?

the webgl jobs are referencing a manifest which doesn't exist. here is the command we execute: /builds/slave/test/build/venv/bin/python -u /builds/slave/test/build/tests/mochitest/runtests.py --appname=/builds/slave/test/build/application/firefox/firefox --utility-path=tests/bin --extra-profile-file=tests/bin/plugins --symbols-path=https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/cedar-linux/1413220385/firefox-35.0a1.en-US.linux-i686.crashreporter-symbols.zip --certificate-path=tests/certs --autorun --close-when-done --console-level=INFO --setpref=webgl.force-enabled=true --quiet --log-raw=/builds/slave/test/build/blobber_upload_dir/raw_structured_logs.log --use-test-media-devices --manifest=tests/mochitest/tests/dom/canvas/test/webgl-mochitest/mochitest.ini If you look, we are expecting this file: tests/mochitest/tests/dom/canvas/test/webgl-mochitest/mochitest.ini but what we really want is: tests/mochitest/tests/dom/canvas/test/webgl-mochitest.ini so we need to modify the script to point to the correct manifest, this patch has pointers of what files to change: https://bug1051886.bugzilla.mozilla.org/attachment.cgi?id=8493161 this is in the mozharness repo: http://hg.mozilla.org/build/mozharness/file/d6ef7a71bdce/configs/unittests

Ah, ok. That's the wrong manifest. This is the correct manifest: http://mxr.mozilla.org/mozilla-central/source/dom/canvas/test/mochitest-subsuite-webgl.ini

Sorry for the delay in looking at this bug, I have been at a releng workshop all week

Comment on attachment 8505777 [details] [diff] [review] bug1051886subsuite.patch Review of attachment 8505777 [details] [diff] [review]: ----------------------------------------------------------------- Wrong directory. I had to move the manifest down a directory. ::: configs/unittests/win_unittest.py @@ +67,5 @@ > "browser-chrome-1": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=1"], > "browser-chrome-2": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=2"], > "browser-chrome-3": ["--browser-chrome", "--chunk-by-dir=5", "--total-chunks=3", "--this-chunk=3"], > "browser-chrome-chunked": ["--browser-chrome", "--chunk-by-dir=5"], > + "mochitest-gl": ["--manifest=tests/mochitest/tests/dom/canvas/test/webgl-mochitest/mochitest-subsuite-webgl.ini"], tests/mochitest/tests/dom/canvas/test/mochitest-subsuite-webgl.ini

Comment on attachment 8505880 [details] [diff] [review] bug1051886subsuite-2.patch Review of attachment 8505880 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks!

In production: https://hg.mozilla.org/build/mozharness/rev/132b59dcbe39

Comment on attachment 8493159 [details] [diff] [review] rename mochitest-webgl to mochitest-gl to match Android as per RyanVM Review of attachment 8493159 [details] [diff] [review]: ----------------------------------------------------------------- mochitest-gl is not running properly on mac or windows, and I bet this is why.

In production: https://hg.mozilla.org/build/buildbot-configs/rev/227fc1edc191

The newest cedar run looks like it's running mochitest-gl satisfactorily on all desktop platforms: https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&revision=8d185e7c78ee It is still running the same tests in mochitest-2, so we do need to disable those. When can we see this on inbound/central/try?

Thanks for the work, too! :)

I'll write a patch to enable on inbound/central/try

I have a patch to enable them on where gecko is at least 36, which enables them on inbound/central/try + associated project branches. Usually we enable tests to ride the trains vs just enabling them on specific branches - is this okay for you?

(In reply to Kim Moir [:kmoir] from comment #53) > I have a patch to enable them on where gecko is at least 36, which enables > them on inbound/central/try + associated project branches. Usually we > enable tests to ride the trains vs just enabling them on specific branches - > is this okay for you? Yes, thanks!

tested in staging, will attach builder diff

builder diff

Comment on attachment 8511052 [details] [diff] [review] bug1051886ridetrains.patch lgtm. sanity check, does https://bugzilla.mozilla.org/show_bug.cgi?id=1051886#c50 mean releng needs to disable mochitest chunk 2 or ateam needs to disable specific tests within mochitest chunk 2?

I assumed that https://bugzilla.mozilla.org/show_bug.cgi?id=1051886#c50 meant ateam disabling specific tests in the manifest. :jgilbert, can you confirm?

(In reply to Kim Moir [:kmoir] from comment #58) > I assumed that https://bugzilla.mozilla.org/show_bug.cgi?id=1051886#c50 > meant ateam disabling specific tests in the manifest. :jgilbert, can you > confirm? Yep, just those specific tests. I'm not sure how the list that is chunked is created. Maybe if we remove the entries from moz.build?

The list of chunked tests is created by mozharness given the manifest for the tests. If the gl tests are in a new manifest, you should be able to comment them out in the original one, but please confirm.

(In reply to Kim Moir [:kmoir] from comment #60) > The list of chunked tests is created by mozharness given the manifest for > the tests. If the gl tests are in a new manifest, you should be able to > comment them out in the original one, but please confirm. Ok, I'll need to move files around then.

In production: https://hg.mozilla.org/build/buildbot-configs/rev/d04c32aec135

So the gl tests are green on those branches on treeherder so I think the only work left to do is to update the manifests to exclude those tests from running as part of m-2

jgilbert: could you update the manifests as you mentioned in comment #61?

(In reply to Kim Moir [:kmoir] from comment #64) > jgilbert: could you update the manifests as you mentioned in comment #61? This approach has yielded bug 1141771. I'm going to mark this fixed, since the issue in the bug summary has been fixed. Work on not running the tests twice should continue in a different bug. I filed bug 1141778 for this.