1192989 - Various webgl tests are permafailing now that Gecko 41 has been merged to Beta

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

https://treeherder.mozilla.org/logviewer.html#?job_id=451781&repo=mozilla-beta
https://treeherder.mozilla.org/logviewer.html#?job_id=451793&repo=mozilla-beta

jmuizelaar is working on fixing since these are currently blocking the 41b1 GTB.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

jmuizelaar gave up

Comment 2

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Milan, could you please help find an owner who can investigate this asap? Without that, I am afraid 40.0Beta1 release will get delayed. Please let me know if I can help in any way. Thanks!

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Jeff M told me to disable the tests for now.
https://hg.mozilla.org/releases/mozilla-beta/rev/1c65e0274b84

Comment 4

[Kelsey Gilbert [:jgilbert]]

I was worried about this. I'll take a look. I tried to have this handled, but looks like it wasn't right.

Comment 5

[Kelsey Gilbert [:jgilbert]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #0)
> https://treeherder.mozilla.org/logviewer.html#?job_id=451781&repo=mozilla-
> beta
> https://treeherder.mozilla.org/logviewer.html#?job_id=451793&repo=mozilla-
> beta
> 
> jmuizelaar is working on fixing since these are currently blocking the 41b1
> GTB.

These are two different issues:
A) chrome/test_webgl_debug_renderer_info.html:
> 12:13:37 INFO - 1423 INFO TEST-UNEXPECTED-FAIL
>               | dom/canvas/test/chrome/test_webgl_debug_renderer_info.html
>               | content iframe: WEBGL_debug_renderer_info should not be listed by getSupportedExtensions in non-chrome contexts - expected PASS
> 12:13:37 INFO - 1424 INFO TEST-UNEXPECTED-FAIL
>               | dom/canvas/test/chrome/test_webgl_debug_renderer_info.html
>               | content iframe: WEBGL_debug_renderer_info should not be available through getExtension in non-chrome contexts - expected PASS
> 12:13:37 INFO - 1425 INFO TEST-UNEXPECTED-FAIL
>               | dom/canvas/test/chrome/test_webgl_debug_renderer_info.html
>               | content iframe: Should not be able to query UNMASKED_VENDOR_WEBGL if enabling WEBGL_debug_renderer_info failed - expected PASS
> 12:13:37 INFO - 1426 INFO TEST-UNEXPECTED-FAIL
>               | dom/canvas/test/chrome/test_webgl_debug_renderer_info.html
>               | content iframe: Should not be able to query UNMASKED_RENDERER_WEBGL if enabling WEBGL_debug_renderer_info failed - expected PASS

B) webgl-mochitest/test_renderer_strings.html:
> 12:14:36 INFO - 688 INFO TEST-UNEXPECTED-FAIL
>               | dom/canvas/test/webgl-mochitest/test_renderer_strings.html
>               | Should have access to 'WEBGL_debug_renderer_info'. - expected PASS
> 12:14:36 INFO - 689 INFO TEST-PASS
>               | dom/canvas/test/webgl-mochitest/test_renderer_strings.html
>               | For start of test, expected 0x0, got 0x0
> 12:14:36 INFO - 690 INFO TEST-UNEXPECTED-FAIL
>               | dom/canvas/test/webgl-mochitest/test_renderer_strings.html
>               | uncaught exception - TypeError: ext is null at http://mochi.test:8888/tests/dom/canvas/test/webgl-mochitest/test_renderer_strings.html:29

In (A), we're allegedly exposing the extension in a non-chrome context. This should not happen on Beta/Release.
In (B), we're allegedly *not* exposing the extension when it's being expected by the test. This test is not correct for Beta/Release, where we indeed should not be exposing this yet.

Comment 6

[Kelsey Gilbert [:jgilbert]]

To be clear, this means that test A seems right, so that failure sounds like a real problem.
Test B is wrong here, and the behavior is correct.

Comment 7

[Kelsey Gilbert [:jgilbert]]

Ok, I think I have the solution.
I made test_renderer_strings deterministic by switching to always obeying the pref "webgl.enable-debug-renderer-info".
I think I introduced a bug in the previous patch by changing the non-chrome test url to use chrome:// instead of http://. I switched it back, and we'll see what Try thinks.

Comment 8

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Simplify-handling-by-ifdef-ing-all.js-s-webgl.enable.patch

This should work with both Beta (and eventually Release) and Nightly/Dev.

Comment 9

[Kelsey Gilbert [:jgilbert]]

Again explicitly:
Our behavior on Beta is correct, so disabling the tests was a fine band-aid.
Our in-tree tests are currently wrong.

Comment 10

[u480271]

Comment on attachment 8646720 [details] [diff] [review]
0001-Simplify-handling-by-ifdef-ing-all.js-s-webgl.enable.patch

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

LGTM but my understanding of this testing modes isn't great.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Re-skipped for beta 42: https://hg.mozilla.org/releases/mozilla-beta/rev/83d7cd647b54

Looks like this never actually landed. Lets actually land it on trunk and then try uplifting as far as we can go?

Comment 12

[Pulsebot]

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

Comment 13

[Kelsey Gilbert [:jgilbert]]

Yep. I've landed it on inbound. If inbound is fine, we'll push it as far as we can.

Comment 14

[Carsten Book [:Tomcat]]

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

Comment 15

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8646720 [details] [diff] [review]
0001-Simplify-handling-by-ifdef-ing-all.js-s-webgl.enable.patch

Approval Request Comment
[Feature/regressing bug #]: -
[User impact if declined]: None.
[Describe test coverage new/current, TreeHerder]: This should fix the tests so we don't need manual intervention every uplift to beta. (Changes from !RELEASE_BUILD to RELEASE_BUILD)
[Risks and why]: None
[String/UUID change made/needed]: none

We should take this on Beta to be sure that it works, instead of possibly having to revisit this in 6 weeks.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8646720 [details] [diff] [review]
0001-Simplify-handling-by-ifdef-ing-all.js-s-webgl.enable.patch

ok, thanks!

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cc969b0ffa4b

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/22bb71109bd6