The default bug view has changed. See this FAQ.

[OMG NOT FIREFOX!!!!! SeaMonkey] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context"

VERIFIED FIXED in Firefox 11

Status

SeaMonkey
Release Engineering
P2
major
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Depends on: 2 bugs, Blocks: 1 bug)

Trunk
seamonkey2.10
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox11 fixed, firefox12 fixed, seamonkey2.1 wontfix, seamonkey2.7 wontfix, seamonkey2.8 verified, seamonkey2.9 verified)

Details

(Whiteboard: [perma-orange] [qa-], URL)

Attachments

(3 attachments, 4 obsolete attachments)

5.50 KB, patch
bjacob
: review+
bjacob
: feedback+
Details | Diff | Splinter Review
12.59 KB, patch
bjacob
: review+
Details | Diff | Splinter Review
3.35 KB, patch
bjacob
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
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".
status2.0: --- → ?
status-seamonkey2.1: --- → ?
(Assignee)

Updated

6 years ago
Summary: [SeaMonkey, Linux] permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context" → [SeaMonkey, Linux] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context"
(Assignee)

Updated

6 years ago
Component: Testing Infrastructure → Release Engineering
QA Contact: testing-infrastructure → release
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

6 years ago
(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

6 years ago
(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.)
(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 :)
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

6 years ago
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.
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?
(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
Discussed on IRC; Ted proposed that we introduce a new environment variable that's only defined on test slaves, allowing to detect them.
(Assignee)

Comment 11

6 years ago
(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")".
(Assignee)

Comment 12

6 years ago
(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?
(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.
But yes, it would be a great idea to extend them to check that.
...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).
(Assignee)

Updated

6 years ago
Depends on: 629957
(Assignee)

Comment 16

6 years ago
(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.
(Assignee)

Updated

6 years ago
Depends on: 629933
(Assignee)

Comment 17

6 years ago
Bug 629933 "enabled" this bug on MacOSX 10.6 too :->
Summary: [SeaMonkey, Linux] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context" → [SeaMonkey, Linux & OSX 10.6] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context"
(Assignee)

Comment 18

6 years ago
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.
Attachment #508214 - Flags: review?(bjacob)
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.
Attachment #508214 - Flags: review?(bjacob) → review+

Comment 20

6 years ago
We usually should not use dump() in mochitests at all, we should use info() instead, just as a note.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Those non-SeaMonkey ones are all fixed-by-backout.
(Assignee)

Updated

6 years ago
Depends on: 636613
(Assignee)

Comment 26

6 years ago
This is now failing on Windows too.

Regression timeframe:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=75082063f8e6&tochange=c0b114d35e7b
Bug 636613!
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(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 hidden (Treeherder Robot)
Summary: [SeaMonkey, Linux & OSX 10.6] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context" → [OMG NOT FIREFOX!!!!! SeaMonkey, Linux & OSX 10.6] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context"
comment 27 through comment 30 are actually bug 645376

Updated

6 years ago
Depends on: 646185
(Assignee)

Updated

6 years ago
OS: Linux → All
Summary: [OMG NOT FIREFOX!!!!! SeaMonkey, Linux & OSX 10.6] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context" → [OMG NOT FIREFOX!!!!! SeaMonkey] mochitests-1: permanent "test_webgl_conformance_test_suite.html | Can't create a WebGL context"
(Assignee)

Comment 32

6 years ago
(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?
(Assignee)

Comment 33

6 years ago
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.
Attachment #508214 - Attachment is obsolete: true
Attachment #529299 - Flags: feedback?(bjacob)
(Assignee)

Comment 34

6 years ago
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.
Attachment #529299 - Attachment is obsolete: true
Attachment #529331 - Flags: review?(bjacob)
Attachment #529299 - Flags: feedback?(bjacob)
(Assignee)

Comment 35

6 years ago
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

Updated

6 years ago
status-seamonkey2.1: ? → wontfix
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 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.
Attachment #529331 - Flags: review?(bjacob)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 40

5 years ago
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.
Attachment #529331 - Attachment is obsolete: true
Attachment #586066 - Flags: review?(bjacob)
(Assignee)

Updated

5 years ago
status2.0: ? → ---
Hardware: x86 → All
(Assignee)

Comment 41

5 years ago
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.
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #586117 - Flags: review?(bjacob)
(Assignee)

Comment 42

5 years ago
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
(Assignee)

Updated

5 years ago
Priority: -- → P2
(Assignee)

Comment 43

5 years ago
Ping for reviews.
I'm a bad boy for not having done my reviews recently. will do today.
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.
Attachment #586066 - Flags: review?(bjacob) → review-
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?
Attachment #586117 - Flags: review?(bjacob) → review+
(Assignee)

Comment 47

5 years ago
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...
Attachment #586117 - Flags: feedback?(bjacob)
(Assignee)

Comment 48

5 years ago
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...
(Assignee)

Comment 49

5 years ago
Ping for feedback.
(Assignee)

Comment 50

5 years ago
Ping for feedback.
(Assignee)

Updated

5 years ago
Attachment #586117 - Flags: feedback?(jgilbert)
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 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.
Attachment #586117 - Flags: feedback?(jgilbert)
Attachment #586117 - Flags: feedback?(bjacob)
Attachment #586117 - Flags: feedback+
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.
Nevermind comment 53, I'm stupid. Please land.
http://hg.mozilla.org/integration/mozilla-inbound/rev/6a94d3142674
Backed out :-( it was too bitrotted. Please refresh it.
http://hg.mozilla.org/integration/mozilla-inbound/rev/f6d04ce89a9e
(Assignee)

Comment 57

5 years ago
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.
Attachment #586117 - Attachment description: (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 → (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 comment 57]
Attachment #586117 - Flags: approval-mozilla-beta?
Attachment #586117 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

5 years ago
status-seamonkey2.7: --- → wontfix
status-seamonkey2.8: --- → affected
status-seamonkey2.9: --- → affected
Flags: in-testsuite+
Target Milestone: --- → seamonkey2.10
(Assignee)

Comment 58

5 years ago
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.
Attachment #586066 - Attachment is obsolete: true
Attachment #601258 - Flags: review?(bjacob)
(Assignee)

Comment 59

5 years ago
(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

5 years ago
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
https://hg.mozilla.org/mozilla-central/rev/6a94d3142674
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 62

5 years ago
(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...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 63

5 years ago
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 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.
Attachment #586117 - Flags: approval-mozilla-beta?
Attachment #586117 - Flags: approval-mozilla-beta+
Attachment #586117 - Flags: approval-mozilla-aurora?
Attachment #586117 - Flags: approval-mozilla-aurora+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Whiteboard: [perma-orange] → [c-n: 023130d28f50 to m-a and m-b] [perma-orange]
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
Attachment #586117 - Attachment description: (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 comment 57] → (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]
Keywords: checkin-needed
Whiteboard: [c-n: 023130d28f50 to m-a and m-b] [perma-orange] → [perma-orange]
(Assignee)

Updated

5 years ago
Status: REOPENED → ASSIGNED
status-seamonkey2.8: affected → fixed
status-seamonkey2.9: affected → fixed
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.
Attachment #601258 - Flags: review?(bjacob) → review+
(Assignee)

Comment 67

5 years ago
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
Attachment #601258 - Attachment description: (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 → (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]
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 68

5 years ago
(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.
status-seamonkey2.8: fixed → verified
status-seamonkey2.9: fixed → verified
(Assignee)

Comment 69

5 years ago
(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).
Status: RESOLVED → VERIFIED
status-11/12 fixed: Bv1
status-firefox11: --- → fixed
status-firefox12: --- → fixed
Whiteboard: [perma-orange] → [perma-orange] [qa-]
(Assignee)

Comment 71

5 years ago
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).
Attachment #603433 - Flags: review?(bjacob)
(Assignee)

Comment 72

5 years ago
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
Attachment #603433 - Flags: review?(bjacob) → review+
(Assignee)

Comment 73

5 years ago
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
Attachment #603433 - Attachment description: (Cv1) Remove unwanted debug ok(false,) from patch Av3b, Fix nits → (Cv1) Remove unwanted debug ok(false,) from patch Av3b, Fix nits [Checked in: Comment 73]
Please use mozilla-inbound for all changesets except what really needs to hit mozilla-central immediately:

https://wiki.mozilla.org/Tree_Rules/Inbound
(Assignee)

Comment 75

5 years ago
(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.
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.
(Assignee)

Comment 77

5 years ago
(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?
(Assignee)

Updated

5 years ago
Depends on: 736696
You need to log in before you can comment on or make changes to this bug.