fast/canvas/webgl/index-validation.html failed on Leopard Commit Bot Twice actually. Somehow lightning struck twice. https://bugs.webkit.org/show_bug.cgi?id=36876#c3 /tmp/layout-test-results/fast/canvas/webgl/index-validation-actual.txt 44 55 PASS gl.getError() is 0 66 PASS gl.drawElements(gl.TRIANGLES, 3, gl.UNSIGNED_SHORT, 0) is undefined. 7 PASS gl.getError() is 0 7 FAIL gl.getError() should be 0. Was 1286. 88 PASS successfullyParsed is true 99 1010 TEST COMPLETE I suspect it may be related to the commit-bot's hardware?
http://trac.webkit.org/browser/trunk/LayoutTests/fast/canvas/webgl/index-validation.html
I'm happy to send a full hardware report to anyone who might need one.
Grepping the commit-queue logs, I see the commit-queue having failed this test 9 times. We just happen to hit two failures in a row while trying to land Adam's patch https://bugs.webkit.org/show_bug.cgi?id=36876#c3
This happened once on my machine before, but not recently. I would suggest to use "antialias:false" in this test to resolve this random failure. However, we need to dig into the reason why this happens and find a fix for it.
Failed again: https://bugs.webkit.org/show_bug.cgi?id=36932#c4
Something happened today to make this failure much more common than previously.
Looks like the first time the Leopard Commit Bot ever saw this test fail was around 3:30PM this afternoon.
It was shortly after http://trac.webkit.org/changeset/56872, which is probably related. :)
This test has failed 39 times on the Leopard Commit Bot since it first failed this afternoon!
This failure rate is unacceptable. Can we rollback that rev and see if it fixes the problem?
*Please* don't roll back that revision. It is a large one that we've been trying to land for weeks, and the failure is ultimately caused by driver bugs and not the code. Can we temporarily disable this test on this bot? We may have another workaround we can land in a couple of hours.
> Can we temporarily disable this test on this bot? We don't have a way to disable tests on the commit-queue specifically. We can disable it more generally however. > We may have another workaround we can land in a couple of hours. Ok. By way of context, this test is flaky enough that it often fails twice in a row. We have something like 13k tests. If an appreciable fraction of them failed this often, the entire test suite would be useless.
(In reply to comment #12) > > Can we temporarily disable this test on this bot? > > We don't have a way to disable tests on the commit-queue specifically. We can > disable it more generally however. Can we add it to the Skipped list for mac-leopard for the moment? > > We may have another workaround we can land in a couple of hours. > > Ok. > > By way of context, this test is flaky enough that it often fails twice in a > row. We have something like 13k tests. If an appreciable fraction of them > failed this often, the entire test suite would be useless. Understood. I apologize for the flakiness introduced by this checkin.
Created attachment 52310 [details] patch This patch should stabilize the test on the bot. However, we need to dig deeper into the cause of this flaky behavior - very likely hardware/driver bugs - and find a better way to solve it. Will create another bug to track this down.
Just created a new bug to track this issue, but the patch should solve it for now. https://bugs.webkit.org/show_bug.cgi?id=36972
Comment on attachment 52310 [details] patch Looks good to me.
Comment on attachment 52310 [details] patch So is this work-around we intend to roll out eventually? Or is this a permanent change to this test? The ChangeLog doesn't really say. I'm fine with either of course.
(In reply to comment #12) > By way of context, this test is flaky enough that it often fails twice in a > row. We have something like 13k tests. If an appreciable fraction of them > failed this often, the entire test suite would be useless. Actually, the statement is stronger. After Adam's changes to speed up the commit-queue yesterday (which happened right after this regression landed) it now requires a flaky test to fail *three* times in a row for it to cause the commit queue to wrongly reject a patch. :)
(In reply to comment #17) > (From update of attachment 52310 [details]) > So is this work-around we intend to roll out eventually? Or is this a > permanent change to this test? The ChangeLog doesn't really say. I'm fine with > either of course. Right now we don't know why this OpenGL error is reported on this test, so the change is basically permanent.
Another victim: https://bugs.webkit.org/show_bug.cgi?id=36918#c3
Another victim: https://bugs.webkit.org/show_bug.cgi?id=36932#c6
Comment on attachment 52310 [details] patch Clearing flags on attachment: 52310 Committed r56925: <http://trac.webkit.org/changeset/56925>
All reviewed patches have been landed. Closing bug.
This test has still failed at least twice on the commit-queue since this fix went in. I can get you the failure diff.
The failure: /tmp/layout-test-results/fast/canvas/webgl/index-validation-actual.txt 44 55 PASS gl.getError() is 0 66 PASS gl.drawElements(gl.TRIANGLES, 3, gl.UNSIGNED_SHORT, 0) is undefined. 7 PASS gl.getError() is 0 7 FAIL gl.getError() should be 0. Was 1286. 88 PASS successfullyParsed is true 99 1010 TEST COMPLETE
After the previous patch, the only difference in this test before and after the multisample patch is the stencil buffer is on by default (before by default we only have color and depth buffer). I'll get another patch ready to turn off the stencil buffer for this test shortly.
Also, I am curious, after the previous patch in which I turned off antialias for this test, does the frequency of this random failure reduced on the bot or still the same?
It appears to be greatly reduced. There have been 9 failure on the bot since last night. Yesterday afternoon we had 66. The bot may simply be doing less now... but I think the failure incidence was reduced, but not eliminated by your previous change.
Created attachment 52421 [details] patch: gather extra information about failure This patch is an attempt to gather further information about the cause of this failure (sorry, but seems like the bot is the only machine that we get this failure at the moment). With this patch, the failure behavior won't be better or worse.
Eric, when this patch lands, and the bot failed this test again, could you send the output to me? Then I will upload another patch to fix the failure problem (hopefully).
Comment on attachment 52421 [details] patch: gather extra information about failure Looks good to me.
Comment on attachment 52421 [details] patch: gather extra information about failure OK.
Comment on attachment 52421 [details] patch: gather extra information about failure Clearing flags on attachment: 52421 Committed r57015: <http://trac.webkit.org/changeset/57015>
These tests are still failing on the commit-bot.
Created attachment 53444 [details] patch
The patch still has an OOPS about no new tests. Also, I think it's worth separately mentioning in the ChangeLog that this fixes an uninitialized variable bug and restores a glFinish call that used to be present.
Created attachment 53445 [details] revised patch : responding to Ken Russell's review
Looks good to me.
Comment on attachment 53445 [details] revised patch : responding to Ken Russell's review Me too.
Comment on attachment 53445 [details] revised patch : responding to Ken Russell's review Rejecting patch 53445 from commit-queue. Failed to run "['WebKitTools/Scripts/build-webkit', '--debug']" exit_code: 1 Last 500 characters of output: n/yacc /bin/sh -c /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebCore.build/Debug/WebCore.build/Script-5DF50887116F3077005202AB.sh ** BUILD FAILED ** The following build commands failed: WebCore: Distributed-CompileC /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebCore.build/Debug/WebCore.build/Objects-normal/i386/GraphicsContext3DMac.o /Users/eseidel/Projects/CommitQueue/WebCore/platform/graphics/mac/GraphicsContext3DMac.cpp normal i386 c++ com.apple.compilers.gcc.4_2 (1 failure) Full output: http://webkit-commit-queue.appspot.com/results/1711103
Created attachment 53458 [details] revised patch: initialize members in the same order as they are declared
LGTM
Comment on attachment 53458 [details] revised patch: initialize members in the same order as they are declared Ah, the old "order of initialization" check. :)
Comment on attachment 53458 [details] revised patch: initialize members in the same order as they are declared Clearing flags on attachment: 53458 Committed r57664: <http://trac.webkit.org/changeset/57664>