Last Comment Bug 629595 - [OMG NOT FIREFOX!!!!! SeaMonkey] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context"
: [OMG NOT FIREFOX!!!!! SeaMonkey] mochitests-1: permanent "test_webgl_conforma...
Status: VERIFIED FIXED
[perma-orange] [qa-]
:
Product: SeaMonkey
Classification: Client Software
Component: Release Engineering (show other bugs)
: Trunk
: All All
: P2 major (vote)
: seamonkey2.10
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
http://mxr.mozilla.org/mozilla-centra...
Depends on: 629957 646185 629933 636613 736696
Blocks: SmTestFail 582053
  Show dependency treegraph
 
Reported: 2011-01-27 21:28 PST by Serge Gautherie (:sgautherie)
Modified: 2012-03-16 20:50 PDT (History)
9 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
wontfix
wontfix
verified
verified


Attachments
(Av1) Fix 2 dump(), Add 1 ok(false) and 2 todo(false) (3.05 KB, patch)
2011-01-30 02:29 PST, Serge Gautherie (:sgautherie)
jacob.benoit.1: review+
Details | Diff | Review
(Av2) Remove dump()s, Add ok()s and todo()s, Do a few related rewrites (7.80 KB, patch)
2011-04-30 10:15 PDT, Serge Gautherie (:sgautherie)
no flags Details | Diff | Review
(Av3) Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation (11.41 KB, patch)
2011-04-30 22:52 PDT, Serge Gautherie (:sgautherie)
no flags Details | Diff | Review
(Av3a) test_webgl_conformance_test_suite.html: Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation (11.54 KB, patch)
2012-01-05 07:34 PST, Serge Gautherie (:sgautherie)
jacob.benoit.1: review-
Details | Diff | Review
(Bv1) test_webgl_conformance_test_suite.html: Ensure waitForExplicitFinish() is executed, Disable test on Windows 2000 and (if necessary) on SeaMonkey, Move Mac OSX version check to a better place [Checked in: See comments 57 and 65] (5.50 KB, patch)
2012-01-05 09:42 PST, Serge Gautherie (:sgautherie)
jacob.benoit.1: review+
jacob.benoit.1: feedback+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review
(Av3b) test_webgl_conformance_test_suite.html: Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation, Remove (obsolete) workaround for Windows 2000 [Checked in: Comment 67] (12.59 KB, patch)
2012-02-28 06:15 PST, Serge Gautherie (:sgautherie)
jacob.benoit.1: review+
Details | Diff | Review
(Cv1) Remove unwanted debug ok(false,) from patch Av3b, Fix nits [Checked in: Comment 73] (3.35 KB, patch)
2012-03-06 13:17 PST, Serge Gautherie (:sgautherie)
jacob.benoit.1: review+
Details | Diff | Review

Description Serge Gautherie (:sgautherie) 2011-01-27 21:28:38 PST
Was passing:
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1295008677.1295010924.25861.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2011/01/14 04:37:57

Fails:
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1295060229.1295062478.2919.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2011/01/14 18:57:09
{
34990 INFO TEST-START | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html
JavaScript warning: http://mochi.test:8888/tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html, line 311: WebGL: Can't get a usable WebGL context
34991 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
WebGL mochitest failed: Can't create a WebGL context
34992 INFO TEST-END | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | finished in 5173ms
}

That should be caused by bug 582053.
Comment 1 Serge Gautherie (:sgautherie) 2011-01-27 21:35:59 PST
Hopefully, this is only an OS requirement failure.

Yet, (if confirmed), it would be good if:
1) test error message gave a clue about it.
2) SeaMonkey boxes could be "updated".
Comment 2 Benoit Jacob [:bjacob] (mostly away) 2011-01-28 04:55:21 PST
What graphics card do the SeaMonkey Linux boxes have? What graphics driver are they using?

As of bug 624390 we blacklist everything but the NVIDIA proprietary driver.

You can define the environment variable MOZ_GLX_IGNORE_BLACKLIST to bypass this. We are currently in the process of determining what other drivers are able to pass the WebGL test suite without crashing, and that seems to be only, besides the NVIDIA blob, the very recent Mesa 7.10-based open source drivers (older drivers with Mesa 7.9 give us crashes) and very recent versions of the ATI blob.

Maybe the easiest is to disable this test on SeaMonkey on Linux?
Comment 3 Robert Kaiser (not working on stability any more) 2011-01-28 05:27:36 PST
(In reply to comment #1)
> 2) SeaMonkey boxes could be "updated".

They can't, we are running tests on the build boxes and so this is nailed down. We need to care that nothing requiring GL (or something not present on a CentOS5 VM) runs on those machines.
Comment 4 Robert Kaiser (not working on stability any more) 2011-01-28 05:33:13 PST
(that said, it would be ideal if such tests could detect that they run on an "unsupported" config and exit with a TODO() or so.)
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2011-01-28 05:45:48 PST
(In reply to comment #4)
> (that said, it would be ideal if such tests could detect that they run on an
> "unsupported" config and exit with a TODO() or so.)

The problem with that is that we would then no longer catch bugs in the blacklisting code. A while ago, a Mac blacklist bug disabled WebGL on Mac and we don't want this to happen again :)
Comment 6 Benoit Jacob [:bjacob] (mostly away) 2011-01-28 05:48:46 PST
Basically, if SeaMonkey Linux boxes are different from tinderboxen, then inevitably they have to use a different set of tests. So the most logic approach here is to disable this test on SeaMonkey Linux boxes for now. Or if they are actually able to run the WebGL test suite (with MOZ_GLX_IGNORE_BLACKLIST) then we can talk about writing for them a separate failing-tests.txt file (if they succeed/fail a different set of WebGL tests than the tinderboxen do) (see file failing-tests-linux.txt in same directory as this mochitest).
Comment 7 Robert Kaiser (not working on stability any more) 2011-01-28 05:53:27 PST
SeaMonkey Linux boxes should be the same as the Firefox build machines, but unless someone donates a sh*tload of hardware to the SeaMonkey project, we can't afford to set up a whole other set of Talos boxes to run tests, so we need to continue to run tests on the same pool of boxes that also do builds.

I agree that we should just disable them for SeaMonkey, though they still fail for people who are running tests on their own machines to check if their changes are sane, and things would be easier if tests would detect they cannot succeed and fail with a todo() - but that's out of the scope for this bug, I guess.
Comment 8 Benoit Jacob [:bjacob] (mostly away) 2011-01-28 06:15:57 PST
Yeah, so, I agree that it's kind of bad that tests are so tailored to tinderboxen, but at the same time we need to cover the graphics blacklisting in tests. Maybe the right solution is to detect tinderboxen in the test, and only then require that WebGL contexts succeed. Do you know how to detect these test slaves?
Comment 9 Justin Wood (:Callek) 2011-01-28 09:16:49 PST
(In reply to comment #8)
> Yeah, so, I agree that it's kind of bad that tests are so tailored to
> tinderboxen, but at the same time we need to cover the graphics blacklisting in
> tests. Maybe the right solution is to detect tinderboxen in the test, and only
> then require that WebGL contexts succeed. Do you know how to detect these test
> slaves?

CC-ing nthomas and bhearsum for this Question
Comment 10 Benoit Jacob [:bjacob] (mostly away) 2011-01-28 09:19:04 PST
Discussed on IRC; Ted proposed that we introduce a new environment variable that's only defined on test slaves, allowing to detect them.
Comment 11 Serge Gautherie (:sgautherie) 2011-01-28 10:38:32 PST
(In reply to comment #2)
> What graphics card do the SeaMonkey Linux boxes have? What graphics driver are
> they using?

http://mxr.mozilla.org/mozilla-central/source/widget/public/nsIGfxInfo.idl
(More or less as I suggested in (acceleration) bug 628589,)
I would suggest that this (WebGL) test could start by using that interface to output (detected) runtime setup, so it would become trivial to answer this kind of questions (on slaves and elsewhere).
Do you agree?

> You can define the environment variable MOZ_GLX_IGNORE_BLACKLIST to bypass
> this.

That might be worth trying (or not) manually, just fwiw, (only) if Callek/KaiRo can/want.

> Maybe the easiest is to disable this test on SeaMonkey on Linux?

Would be good for me as a temporary workaround.
But I would prefer a solution that let the test run unmodified on a Linux computer which meets the graphics runtime requirements.


(In reply to comment #3)
> (In reply to comment #1)
> > 2) SeaMonkey boxes could be "updated".
> 
> They can't

(I hoped the solution could be a matter of installing a newer driver version.
But if that's not the problem or a possible solution, then we'll live with it.)


(In reply to comment #5)
> The problem with that is that we would then no longer catch bugs in the
> blacklisting code.

Would some kind of blacklist test be realistic? (Just in case...)


(In reply to comment #10)
> Discussed on IRC; Ted proposed that we introduce a new environment variable
> that's only defined on test slaves, allowing to detect them.

Yes, could be done like that.
Though that should probably be more "RUN_WEBGL_TEST=1" than "I_AM_A_SLAVEBOX=1",
with a default "todo(false, "Test disabled as RUN_WEBGL_TEST env var is not set")".
Comment 12 Serge Gautherie (:sgautherie) 2011-01-29 16:13:46 PST
(In reply to comment #11)
> (In reply to comment #5)
> > The problem with that is that we would then no longer catch bugs in the
> > blacklisting code.
> 
> Would some kind of blacklist test be realistic? (Just in case...)

What about blacklist tests added in bug 625160?
Comment 13 Benoit Jacob [:bjacob] (mostly away) 2011-01-29 16:27:20 PST
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #5)
> > > The problem with that is that we would then no longer catch bugs in the
> > > blacklisting code.
> > 
> > Would some kind of blacklist test be realistic? (Just in case...)
> 
> What about blacklist tests added in bug 625160?

These only test imaginary vendor IDs such as 0xabcd. My understanding is that they only test that the downloaded-blacklist code works well, they don't test that actual configs that should work actually work.
Comment 14 Benoit Jacob [:bjacob] (mostly away) 2011-01-29 16:28:00 PST
But yes, it would be a great idea to extend them to check that.
Comment 15 Benoit Jacob [:bjacob] (mostly away) 2011-01-29 16:32:07 PST
...and yes thinking about it, that is really the right approach.

to summarize:
1) extend xpcshell tests to check that at least the test slaves config are not blocked
2) change webgl mochitest to first check if the system is blacklisted for WebGL, and if it is, exit without error. If it is not blacklisted, continue as before (generate error is WebGL contexts fail to create).
Comment 16 Serge Gautherie (:sgautherie) 2011-01-29 16:38:44 PST
(In reply to comment #11)
> (In reply to comment #2)
> > What graphics card do the SeaMonkey Linux boxes have? What graphics driver are
> > they using?
> 
> http://mxr.mozilla.org/mozilla-central/source/widget/public/nsIGfxInfo.idl
> (More or less as I suggested in (acceleration) bug 628589,)
> I would suggest that this (WebGL) test could start by using that interface to
> output (detected) runtime setup, so it would become trivial to answer this kind
> of questions (on slaves and elsewhere).
> Do you agree?

I filed bug 629957.
Comment 17 Serge Gautherie (:sgautherie) 2011-01-30 01:41:20 PST
Bug 629933 "enabled" this bug on MacOSX 10.6 too :->
Comment 18 Serge Gautherie (:sgautherie) 2011-01-30 02:29:30 PST
Created attachment 508214 [details] [diff] [review]
(Av1) Fix 2 dump(), Add 1 ok(false) and 2 todo(false)

("Noticed" from bug 629933 context.)

Example current log:
{
...
++DOMWINDOW == 40 (0x1a046464) [serial = 922] [outer = 0x18da91d0]
WebGL mochitest disabled on Mac OSX versions older than 10.634994 INFO TEST-END | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | finished in 1933ms
}

Please double-check, as I'm unfamiliar with this mix of dump() and SimpleTest.
Comment 19 Benoit Jacob [:bjacob] (mostly away) 2011-01-30 06:17:03 PST
Comment on attachment 508214 [details] [diff] [review]
(Av1) Fix 2 dump(), Add 1 ok(false) and 2 todo(false)

r=me with the following changes:

>From: Serge Gautherie <sgautherie.bz@free.fr>
>
>Bug 629595 - [SeaMonkey, Linux & OSX 10.6] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context";
>(Av1) Fix 2 dump(), Add 1 ok(false) and 2 todo(false).
>
>diff --git a/content/canvas/test/webgl/test_webgl_conformance_test_suite.html b/content/canvas/test/webgl/test_webgl_conformance_test_suite.html
>--- a/content/canvas/test/webgl/test_webgl_conformance_test_suite.html
>+++ b/content/canvas/test/webgl/test_webgl_conformance_test_suite.html
>@@ -171,16 +171,17 @@ function start() {
>       ++this.totalTimeouts;
>       if (this.isExpectedToFullyPass()) {
>         ok(false, this.errormsg('Unexpected timeout in this test page'));
>         window.dump('WebGL test error: test page timeout: ' + this.url + '\n');
>       }
>     } else if (this.totalSuccessful != this.totalTests) {
>       var css = 'testpagefail';
>       if (this.isExpectedToFullyPass()) {
>+        ok(false, this.errormsg('Unexpected failure in this test page'));
>         window.dump('WebGL test error: test page failure: ' + this.url + '\n');

Notice that ok() already has the effect of printing to the console. So it's redundant to use both ok() and dump().

So I agree with switching to a ok() here, but please remove the dump().

Notice that the (bad) reason why this finishPage() function was only using dump() here was that errors had already been reported using ok() for individial sub-tests in addResult().

Agree with your proposed change.

>       }
>       // failures have already been reported for the sub-tests
>     } else {
>       var css = 'testpagesuccess';
>       if (this.isExpectedToFullyPass()) {
>         ok(true, this.errormsg('Successful test page'));
>       }
>@@ -352,17 +353,18 @@ function start() {
>     try {
>       netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
>       var version = Components.classes["@mozilla.org/system-info;1"]
>                               .getService(Components.interfaces.nsIPropertyBag2)
>                               .getProperty("version");
>       isWinVistaOrHigher = (parseFloat(version) >= 6.0);
>     } catch (ex) {}
>     if (!isWinVistaOrHigher) {
>-      dump("WebGL mochitest disabled on Windows versions older than Vista");
>+      todo(false, "Test disabled on Windows versions older than Vista");
>+      dump("WebGL mochitest disabled on Windows versions older than Vista\n");

Same here: I agree with using a todo() here, but then please remove the dump(). (And thanks for catching the missing \n)

>       SimpleTest.finish();
>       return;
>     }
>   }
> 
>   // we currently disable this test on version of Mac OSX older than 10.6,
>   // due to various weird failures, including one making getRenderbufferParameter tests
>   // on DEPTH_STENCIL fail
>@@ -375,17 +377,18 @@ function start() {
>                               .getService(Components.interfaces.nsIPropertyBag2)
>                               .getProperty("version");
>       // the next line is correct: Mac OS 10.6 corresponds to Darwin version 10 !
>       // Mac OS 10.5 would be Darwin version 9. the |version| string we've got here
>       // is the Darwin version.
>       is106orHigher = (parseFloat(version) >= 10.0);
>     } catch (ex) { }
>     if (!is106orHigher) {
>-      dump("WebGL mochitest disabled on Mac OSX versions older than 10.6");
>+      todo(false, "Test disabled on Mac OSX versions older than 10.6");
>+      dump("WebGL mochitest disabled on Mac OSX versions older than 10.6\n");

Same.
Comment 20 Robert Kaiser (not working on stability any more) 2011-01-30 06:31:45 PST
We usually should not use dump() in mochitests at all, we should use info() instead, just as a note.
Comment 21 Treeherder Robot 2011-02-11 19:53:07 PST
mfinkle%mozilla.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1297479551.1297481753.20057.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-1/5 on 2011/02/11 18:59:11

s: talos-r3-snow-044
35854 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
Comment 22 Treeherder Robot 2011-02-11 19:55:19 PST
mfinkle%mozilla.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1297480577.1297481298.17998.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central opt test mochitests-1/5 on 2011/02/11 19:16:17

s: talos-r3-snow-003
35850 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
Comment 23 Treeherder Robot 2011-02-11 19:58:10 PST
mfinkle%mozilla.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1297481876.1297482635.23696.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central opt test mochitests-1/5 on 2011/02/11 19:37:56

s: talos-r3-snow-044
35850 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
Comment 24 Treeherder Robot 2011-02-11 19:59:45 PST
mfinkle%mozilla.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1297478623.1297480837.15998.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-1/5 on 2011/02/11 18:43:43

s: talos-r3-snow-033
35873 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
Comment 25 Phil Ringnalda (:philor) 2011-02-11 20:14:08 PST
Those non-SeaMonkey ones are all fixed-by-backout.
Comment 26 Serge Gautherie (:sgautherie) 2011-03-10 07:29:06 PST
This is now failing on Windows too.

Regression timeframe:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=75082063f8e6&tochange=c0b114d35e7b
Bug 636613!
Comment 27 Treeherder Robot 2011-03-26 09:06:06 PDT
ehsan%mozilla.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1301136973.1301139129.24593.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-1/5 on 2011/03/26 03:56:13

s: talos-r3-snow-007
36162 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
Comment 28 Treeherder Robot 2011-03-26 09:34:54 PDT
wtc%google.com
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1301154377.1301156612.5216.gz
Rev3 MacOSX Snow Leopard 10.6.2 tryserver debug test mochitests-1/5 on 2011/03/26 08:46:17

s: talos-r3-snow-045
36145 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
Comment 29 Bill Gianopoulos [:WG9s] 2011-03-26 10:13:43 PDT
(In reply to comment #27)
> ehsan%mozilla.com
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1301136973.1301139129.24593.gz
> Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-1/5 on
> 2011/03/26 03:56:13
> 
> s: talos-r3-snow-007
> 36162 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't
> create a WebGL context

Well, this is a random orange under Firefox.  i think this should really have been a new bug rather than tacked onto a a perma-orange under seamonkey bug.  (just my opinion, I could be wrong)
Comment 30 Treeherder Robot 2011-03-26 11:04:48 PDT
josh%joshmatthews.net
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1301159552.1301161959.26783.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-1/5 on 2011/03/26 10:12:32

s: talos-r3-snow-016
36142 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
Comment 31 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-03-26 12:22:57 PDT
comment 27 through comment 30 are actually bug 645376
Comment 32 Serge Gautherie (:sgautherie) 2011-04-29 23:22:57 PDT
(In reply to comment #20)
> We usually should not use dump() in mochitests at all, we should use info()
> instead, just as a note.

There isn't such an info() in mochitest harness (yet), is it?
Comment 33 Serge Gautherie (:sgautherie) 2011-04-30 10:15:54 PDT
Created attachment 529299 [details] [diff] [review]
(Av2) Remove dump()s, Add ok()s and todo()s, Do a few related rewrites

Av1, with comment 19 suggestion(s),
and some more.

This succeeded as http://tbpl.mozilla.org/?tree=Try&rev=edbd8a95ee8d.
I'm now Try'ing an improved version of this one.
Comment 34 Serge Gautherie (:sgautherie) 2011-04-30 22:52:58 PDT
Created attachment 529331 [details] [diff] [review]
(Av3) Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation

Av2, improved.
Comment 35 Serge Gautherie (:sgautherie) 2011-04-30 22:53:52 PDT
Comment on attachment 529331 [details] [diff] [review]
(Av3) Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation

Succeeded as
http://tbpl.mozilla.org/?tree=Try&rev=2999c237fad2
Comment 36 Benoit Jacob [:bjacob] (mostly away) 2011-07-25 11:58:42 PDT
What's the status of this? Seems like it got forgotten. There's stuff that I like in this patch, like adding ok() for context creation and simplifying the Mac version detection code. There's stuff I am less comfortable with such as replacing other dump()'s by ok(true, ...)'s as that was plain debugging stuff and I don't understand how dump() is less good that ok(true) for that. Also make sure that the replacement is as greppable as the original, e.g. I liked to be able to grep for "WebGL test error". Regarding the addition of todo()'s I don't have an opinion.
Comment 37 Benoit Jacob [:bjacob] (mostly away) 2011-08-27 10:59:29 PDT
Comment on attachment 529331 [details] [diff] [review]
(Av3) Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation

Removing the review? flag, see previous comment. Parts of this patch have already been applied on mozilla-central, other parts have bitrotted and are welcome to be submitted again as a fresh new patch.
Comment 38 Treeherder Robot 2011-10-19 07:52:26 PDT
kaze
https://tbpl.mozilla.org/php/getParsedLog.php?id=6895132&tree=Try
Rev4 MacOSX Snow Leopard 10.6 try opt test mochitests-1/5 on 2011-10-17 13:21:15

43567 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
Comment 39 Treeherder Robot 2011-12-19 15:57:37 PST
jonas%sicking.cc
https://tbpl.mozilla.org/php/getParsedLog.php?id=8041059&tree=Try
Rev4 MacOSX Snow Leopard 10.6 try debug test mochitests-1/5 on 2011-12-19 13:59:46

46201 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context
Comment 40 Serge Gautherie (:sgautherie) 2012-01-05 07:34:16 PST
Created attachment 586066 [details] [diff] [review]
(Av3a) test_webgl_conformance_test_suite.html: Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation

Av3, unbitrotted.


(In reply to Benoit Jacob [:bjacob] from comment #36)
> There's stuff I am less comfortable with
> such as replacing other dump()'s by ok(true, ...)'s as that was plain
> debugging stuff and I don't understand how dump() is less good that ok(true)
> for that. Also make sure that the replacement is as greppable as the
> original, e.g. I liked to be able to grep for "WebGL test error".

My point is to fully use mochitest harness (automatic log parsing, unified line format), just as "all" other tests do.
Relying on (custom) "debug" output and people manually checking every build log/line doesn't seem reliable.


(In reply to Benoit Jacob [:bjacob] from comment #37)
> Parts of this patch have already been applied on mozilla-central,
> other parts have bitrotted and are welcome to be submitted again

Afaict, apart from a s/ok(false)/todo(true)/ which I'm removing anyway,
nothing was already applied: simple bitrot.
Comment 41 Serge Gautherie (:sgautherie) 2012-01-05 09:42:11 PST
Created attachment 586117 [details] [diff] [review]
(Bv1) test_webgl_conformance_test_suite.html: Ensure waitForExplicitFinish() is executed, Disable test on Windows 2000 and (if necessary) on SeaMonkey, Move Mac OSX version check to a better place
[Checked in: See comments 57 and 65]

waitForExplicitFinish(): "must" be executed before onload(), not after.

Workarounds, until we can do something smarter:
*Win2000: I must be the only one caring at this point, but system crash is very unpleasant.
*SeaMonkey: get rid of this 1-year perma-orange, at last.
Comment 42 Serge Gautherie (:sgautherie) 2012-01-06 18:08:35 PST
Comment on attachment 586066 [details] [diff] [review]
(Av3a) test_webgl_conformance_test_suite.html: Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation

Succeeded as
https://tbpl.mozilla.org/?tree=Try&rev=4c081ba0088f
Comment 43 Serge Gautherie (:sgautherie) 2012-01-26 03:19:52 PST
Ping for reviews.
Comment 44 Benoit Jacob [:bjacob] (mostly away) 2012-01-26 05:25:27 PST
I'm a bad boy for not having done my reviews recently. will do today.
Comment 45 Benoit Jacob [:bjacob] (mostly away) 2012-01-26 14:43:29 PST
Comment on attachment 586066 [details] [diff] [review]
(Av3a) test_webgl_conformance_test_suite.html: Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation

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

r- for details, but there are several ideas that I like here. Especially how the reporting on unexpected-passing pages is improved. Please carry on.

::: content/canvas/test/webgl/test_webgl_conformance_test_suite.html
@@ +129,5 @@
>        // only few timeouts are actually caught here --- most are caught in finishPage().
>        if (this.isExpectedToFullyPass()) {
> +        ok(false, this.logMsg('Test timed out'), msg);
> +      } else {
> +        todo(false, this.logMsg('Test timed out'), msg);

Not convinced about the usefulness of this hunk. The timeout is already reported for the test page, and one can click on it to get details.

@@ +140,5 @@
> +        ok(true, this.logMsg('Test passed'), msg);
> +      } else {
> +        todo(false, this.logMsg('Test passed, but is ignored'), msg);
> +      }
> +      // Don't report individual success to UI, to keep it light.

I don't understand this: this hunk is reported on individual tests in the UI, contrary to what the comment says. Right?

(I still believe it's a bad idea to report on individual tests in the UI. You can click on test pages to get detailed results)

@@ +148,5 @@
>        var css = "fail";
>        if (this.isExpectedToFullyPass()) {
> +        ok(false, this.logMsg('Test failed'), msg);
> +      } else {
> +        todo(false, this.logMsg('Test failed'), msg);

Same.

@@ +195,2 @@
>        if (this.isExpectedToFullyPass()) {
> +        ok(false, this.logMsg(totalFailed + ' failure(s) and ' + this.totalTimeouts + ' timeout(s)'));

I really want to keep something easily greppable here like 'WebGL test error'

@@ +247,5 @@
>      this.pagesByURL[url] = this.currentPage;
>    };
>  
>    Reporter.prototype.startPage = function(url) {
> +    ok(true, '[' + url + '] Starting test page');

I really want to keep something easily greppable here like 'WebGL mochitest'. I use that often.
Comment 46 Benoit Jacob [:bjacob] (mostly away) 2012-01-26 14:46:23 PST
Comment on attachment 586117 [details] [diff] [review]
(Bv1) test_webgl_conformance_test_suite.html: Ensure waitForExplicitFinish() is executed, Disable test on Windows 2000 and (if necessary) on SeaMonkey, Move Mac OSX version check to a better place
[Checked in: See comments 57 and 65]

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

r+ but please answer my questions.

::: content/canvas/test/webgl/test_webgl_conformance_test_suite.html
@@ +59,5 @@
>                              .getProperty("version");
>      kIsWindowsVistaOrHigher = (parseFloat(version) >= 6.0);
> +    // Workaround for Windows 2000 (driver?) which may crash itself.
> +    if (parseFloat(version) <= 5.0) {
> +      todo(false, "Test disabled on Windows 2000 and older. (To prevent possible system crash.)");

alright.

@@ +80,5 @@
> +    // is the Darwin version.
> +    if (parseFloat(version) < 10.0) {
> +      todo(false, "Test disabled on Mac OSX versions older than 10.6.");
> +      SimpleTest.finish();
> +      return;

Why did you have to move this around?

@@ +426,5 @@
>  }
>  
>  </script>
>  </head>
> +<body onload="start();">

Is this useful?
Comment 47 Serge Gautherie (:sgautherie) 2012-01-27 00:07:27 PST
Comment on attachment 586117 [details] [diff] [review]
(Bv1) test_webgl_conformance_test_suite.html: Ensure waitForExplicitFinish() is executed, Disable test on Windows 2000 and (if necessary) on SeaMonkey, Move Mac OSX version check to a better place
[Checked in: See comments 57 and 65]

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

Just tell me if you want me not to check these two "nits" in.

::: content/canvas/test/webgl/test_webgl_conformance_test_suite.html
@@ +80,5 @@
> +    // is the Darwin version.
> +    if (parseFloat(version) < 10.0) {
> +      todo(false, "Test disabled on Mac OSX versions older than 10.6.");
> +      SimpleTest.finish();
> +      return;

No code change: it just seemed more logical to have all these platform checking blocks together.

@@ +426,5 @@
>  }
>  
>  </script>
>  </head>
> +<body onload="start();">

Just an optional nit...
Comment 48 Serge Gautherie (:sgautherie) 2012-02-05 06:33:10 PST
Ping for feedback.

NB: I will remove the Win2K check on Trunk afterwards, but I would like to port this workaround patch "as is" to aurora and beta...
Comment 49 Serge Gautherie (:sgautherie) 2012-02-15 10:02:36 PST
Ping for feedback.
Comment 50 Serge Gautherie (:sgautherie) 2012-02-24 01:11:13 PST
Ping for feedback.
Comment 51 Benoit Jacob [:bjacob] (mostly away) 2012-02-27 13:43:27 PST
Alright, sorry for the long delay. Horrible period for platform engs at the moment :/ also, I usually check only my review queue, not my feedback queue.
Comment 52 Benoit Jacob [:bjacob] (mostly away) 2012-02-27 13:45:44 PST
Comment on attachment 586117 [details] [diff] [review]
(Bv1) test_webgl_conformance_test_suite.html: Ensure waitForExplicitFinish() is executed, Disable test on Windows 2000 and (if necessary) on SeaMonkey, Move Mac OSX version check to a better place
[Checked in: See comments 57 and 65]

Alright, we shouldn't have stalled for so long just for these trivial nits. Sorry about that.
Comment 53 Benoit Jacob [:bjacob] (mostly away) 2012-02-27 13:50:36 PST
Wait! What about that:

-  SimpleTest.waitForExplicitFinish();

Pretty sure that we do need this call!

Aside from that, you're very welcome to land this patch.
Comment 54 Benoit Jacob [:bjacob] (mostly away) 2012-02-27 13:51:19 PST
Nevermind comment 53, I'm stupid. Please land.
Comment 55 Benoit Jacob [:bjacob] (mostly away) 2012-02-27 13:59:24 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/6a94d3142674
Comment 56 Benoit Jacob [:bjacob] (mostly away) 2012-02-27 15:32:25 PST
Backed out :-( it was too bitrotted. Please refresh it.
http://hg.mozilla.org/integration/mozilla-inbound/rev/f6d04ce89a9e
Comment 57 Serge Gautherie (:sgautherie) 2012-02-28 01:05:36 PST
Comment on attachment 586117 [details] [diff] [review]
(Bv1) test_webgl_conformance_test_suite.html: Ensure waitForExplicitFinish() is executed, Disable test on Windows 2000 and (if necessary) on SeaMonkey, Move Mac OSX version check to a better place
[Checked in: See comments 57 and 65]

https://hg.mozilla.org/mozilla-central/rev/023130d28f50
Bv1, unbitrotted and applying without patch A.


[Approval Request Comment]
Regression caused by (bug #): Bug 582053.
User impact if declined: None, but perma-orange on SeaMonkey.
Testing completed (on m-c, etc.): This comment.
Risk to taking this patch (and alternatives if risky): None, test only.
String changes made by this patch: None.
Comment 58 Serge Gautherie (:sgautherie) 2012-02-28 06:15:34 PST
Created attachment 601258 [details] [diff] [review]
(Av3b) test_webgl_conformance_test_suite.html: Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation, Remove (obsolete) workaround for Windows 2000
[Checked in: Comment 67]

Av3a, with comment 20 and comment 45 suggestion(s).


(In reply to Serge Gautherie (:sgautherie) from comment #32)
> There isn't such an info() in mochitest harness (yet), is it?

Now there is: since bug 670817 ;-)


(In reply to Benoit Jacob [:bjacob] from comment #45)

> ::: content/canvas/test/webgl/test_webgl_conformance_test_suite.html
> @@ +129,5 @@
> >        // only few timeouts are actually caught here --- most are caught in finishPage().
> 
> Not convinced about the usefulness of this hunk. The timeout is already
> reported for the test page, and one can click on it to get details.

I don't know about double report: see this comment...

> > +      // Don't report individual success to UI, to keep it light.
> 
> I don't understand this: this hunk is reported on individual tests in the
> UI, contrary to what the comment says. Right?

As I understand it, this comment refers to the .appendChild() code below.

> (I still believe it's a bad idea to report on individual tests in the UI.
> You can click on test pages to get detailed results)

I wonder whether you are confusing UI report which is below DOM code and harness logs which are SimpleTest functions...
(At least, that's how I understand this whole test was made.)

Maybe you should just test it and give me an testcase of unwanted output.
Comment 59 Serge Gautherie (:sgautherie) 2012-02-28 09:36:19 PST
(In reply to Serge Gautherie (:sgautherie) from comment #57)
> https://hg.mozilla.org/mozilla-central/rev/023130d28f50

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1330438426.1330440637.6984.gz&fulltext=1
OS X 10.6 comm-central-trunk debug test mochitests-1/5 on 2012/02/28 06:13:46
{
45560 INFO TEST-KNOWN-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | Can't create a WebGL context (This is expected on SeaMonkey (tinderboxes).)
}

V.Fixed wrt this patch (only).
Comment 60 Mozilla RelEng Bot 2012-02-28 09:45:25 PST
Try run for 6a00c1441b7a is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6a00c1441b7a
Results (out of 30 total builds):
    success: 27
    warnings: 2
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/sgautherie.bz@free.fr-6a00c1441b7a
Comment 61 Matt Brubeck (:mbrubeck) 2012-02-28 09:49:22 PST
https://hg.mozilla.org/mozilla-central/rev/6a94d3142674
Comment 62 Serge Gautherie (:sgautherie) 2012-02-28 10:00:52 PST
(In reply to Matt Brubeck (:mbrubeck) from comment #61)
> https://hg.mozilla.org/mozilla-central/rev/6a94d3142674

Eh! That was comment 55, which was backed out in comment 56...
Comment 63 Serge Gautherie (:sgautherie) 2012-02-28 10:02:41 PST
Comment on attachment 601258 [details] [diff] [review]
(Av3b) test_webgl_conformance_test_suite.html: Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation, Remove (obsolete) workaround for Windows 2000
[Checked in: Comment 67]

(In reply to Mozilla RelEng Bot from comment #60)
>     https://tbpl.mozilla.org/?tree=Try&rev=6a00c1441b7a

Still passing :-)
Comment 64 Alex Keybl [:akeybl] 2012-02-28 13:19:58 PST
Comment on attachment 586117 [details] [diff] [review]
(Bv1) test_webgl_conformance_test_suite.html: Ensure waitForExplicitFinish() is executed, Disable test on Windows 2000 and (if necessary) on SeaMonkey, Move Mac OSX version check to a better place
[Checked in: See comments 57 and 65]

[Triage Comment]
Test only change - approved for Aurora 12 and Beta 11.
Comment 65 Jens Hatlak (:InvisibleSmiley) 2012-02-28 15:02:51 PST
Comment on attachment 586117 [details] [diff] [review]
(Bv1) test_webgl_conformance_test_suite.html: Ensure waitForExplicitFinish() is executed, Disable test on Windows 2000 and (if necessary) on SeaMonkey, Move Mac OSX version check to a better place
[Checked in: See comments 57 and 65]

http://hg.mozilla.org/releases/mozilla-aurora/rev/5cda17e7c743
http://hg.mozilla.org/releases/mozilla-beta/rev/c392df98777f
Comment 66 Benoit Jacob [:bjacob] (mostly away) 2012-02-28 19:32:09 PST
Comment on attachment 601258 [details] [diff] [review]
(Av3b) test_webgl_conformance_test_suite.html: Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation, Remove (obsolete) workaround for Windows 2000
[Checked in: Comment 67]

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

Alright, this generally improves the quality of the error checking and reporting, r=me.
Comment 67 Serge Gautherie (:sgautherie) 2012-02-29 01:49:05 PST
Comment on attachment 601258 [details] [diff] [review]
(Av3b) test_webgl_conformance_test_suite.html: Remove dump()s, Add ok()s and todo()s, Do a few related rewrites, Fix totalFailed calculation, Remove (obsolete) workaround for Windows 2000
[Checked in: Comment 67]

https://hg.mozilla.org/mozilla-central/rev/cb01e23f83cf
Comment 68 Serge Gautherie (:sgautherie) 2012-02-29 03:10:55 PST
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #65)

> http://hg.mozilla.org/releases/mozilla-aurora/rev/5cda17e7c743

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1330506697.1330508791.3818.gz
Linux comm-aurora debug test mochitests-1/5 on 2012/02/29 01:11:37

> http://hg.mozilla.org/releases/mozilla-beta/rev/c392df98777f

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1330485866.1330490454.5475.gz
OS X 10.6 comm-beta debug test mochitests-1/5 on 2012/02/28 19:24:26

seamonkey2.9 and seamonkey2.8: verified.
Comment 69 Serge Gautherie (:sgautherie) 2012-02-29 13:52:46 PST
(In reply to Serge Gautherie (:sgautherie) from comment #67)
> https://hg.mozilla.org/mozilla-central/rev/cb01e23f83cf

https://tbpl.mozilla.org/php/getParsedLog.php?id=9713670&tree=Firefox&full=1
Rev3 Fedora 12x64 mozilla-central opt test mochitests-1/5 on 2012-02-29 02:10:59 PST for <a href="../?branch=mozilla-central&rev=cb01e23f83cf">push cb01e23f83cf

Some output examples:
{
45568 INFO TEST-INFO | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/attribs/gl-enable-vertex-attrib.html] (WebGL mochitest) Starting test page

45569 INFO TEST-PASS | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/attribs/gl-enable-vertex-attrib.html] Test passed - getError was expected value: NO_ERROR :

45572 INFO TEST-PASS | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/attribs/gl-enable-vertex-attrib.html] All 3 test(s) passed

47829 INFO TEST-KNOWN-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/context/premultiplyalpha-test.html] Test failed - should draw with 128,128,128,255

47831 INFO TEST-KNOWN-FAIL | /tests/content/canvas/test/webgl/test_webgl_conformance_test_suite.html | [conformance/context/premultiplyalpha-test.html] (WebGL test error) 5 failure(s) and 0 timeout(s)
}

V.Fixed, wrt this patch (too).
Comment 70 Justin Wood (:Callek) 2012-03-05 14:14:31 PST
status-11/12 fixed: Bv1
Comment 71 Serge Gautherie (:sgautherie) 2012-03-06 13:17:36 PST
Created attachment 603433 [details] [diff] [review]
(Cv1) Remove unwanted debug ok(false,) from patch Av3b, Fix nits
[Checked in: Comment 73]

An unwanted line sneaked in when I unbirotted patch A...
Doing a little rewrite while here (again).
Comment 72 Serge Gautherie (:sgautherie) 2012-03-06 13:18:54 PST
Comment on attachment 603433 [details] [diff] [review]
(Cv1) Remove unwanted debug ok(false,) from patch Av3b, Fix nits
[Checked in: Comment 73]

Succeeded as
https://tbpl.mozilla.org/?tree=Try&rev=4b820c75469a
Comment 73 Serge Gautherie (:sgautherie) 2012-03-07 13:20:39 PST
Comment on attachment 603433 [details] [diff] [review]
(Cv1) Remove unwanted debug ok(false,) from patch Av3b, Fix nits
[Checked in: Comment 73]

https://hg.mozilla.org/mozilla-central/rev/2f6368ca605e
Comment 74 Benoit Jacob [:bjacob] (mostly away) 2012-03-07 13:51:47 PST
Please use mozilla-inbound for all changesets except what really needs to hit mozilla-central immediately:

https://wiki.mozilla.org/Tree_Rules/Inbound
Comment 75 Serge Gautherie (:sgautherie) 2012-03-07 13:59:54 PST
(In reply to Benoit Jacob [:bjacob] from comment #74)
> Please use mozilla-inbound for all changesets except what really needs to
> hit mozilla-central immediately:

Why? I don't have a local repo to push to Inbound.
Comment 76 Benoit Jacob [:bjacob] (mostly away) 2012-03-07 14:13:40 PST
It's pretty cheap to make one, you don't have to re-clone from scratch. You can cp -r your existing mozilla-central clone, edit its .hg/hgrc to point to inbound instead, and hg pull -u.

inbound is the best thing that happened to the tree in a long time: our rate of tree closures has gone down dramatically since we're using inbound. Also, by pushing to inbound, you're not required anymore to watch and babysit TBPL, like you are when you push to central.
Comment 77 Serge Gautherie (:sgautherie) 2012-03-07 15:16:49 PST
(In reply to Benoit Jacob [:bjacob] from comment #76)

> It's pretty cheap to make one

I lack disk space.

> inbound is the best thing that happened to the tree in a long time

I concur. Yet it's not forbidden to push to m-c, is it?

Note You need to log in before you can comment on or make changes to this bug.