Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upEnable wpt WebGL tests on Linux #12858
Conversation
|
Note: My testing was done on Mesa's i965 driver on Haswell. There's an interesting problem in the out-of-bounds tests where with a debug build of Mesa they assertion fail. I think we may just be getting lucky and not crashing in a release build, requiring better bounds checking on our draws. |
| @@ -2,6 +2,7 @@ | |||
| type: testharness | |||
| expected: | |||
| if os == "mac": CRASH | |||
This comment has been minimized.
This comment has been minimized.
Manishearth
Aug 14, 2016
•
Member
this should just be CRASH, since we only test on linux and osx? same for the next two
|
@bors-servo try |
Enable wpt WebGL tests on Linux <!-- Please describe your changes on the following line: --> Enables the WebGL tests in test-wpt on Linux. They have been disabled since the webgl conformance tests first showed up, and it looks like they should work now. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #9331 <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12858) <!-- Reviewable:end -->
|
|
|
Seem to be lots of failures on the server :/ |
|
Yeah, this PR was mostly an attempt to give the CI a shot at it, since its driver would be different from mine. Hopefully the next force-push will be cleaner on CI. |
|
@bors-servo: try |
Enable wpt WebGL tests on Linux <!-- Please describe your changes on the following line: --> Enables the WebGL tests in test-wpt on Linux. They have been disabled since the webgl conformance tests first showed up, and it looks like they should work now. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #9331 <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12858) <!-- Reviewable:end -->
|
Note the way I added the subtest FAILs in 7ef67c3 -- do I need to file those under linux only somehow? |
|
|
|
Looks like in round 2 the llvmpipe crashes didn't show up (some spurious failure?) but the texture-npot failures matched. Update dropped the two crashes and (hopefully correctly) added the texture-npot. |
|
@bors-servo: try |
Enable wpt WebGL tests on Linux <!-- Please describe your changes on the following line: --> Enables the WebGL tests in test-wpt on Linux. They have been disabled since the webgl conformance tests first showed up, and it looks like they should work now. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #9331 <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12858) <!-- Reviewable:end -->
|
|
highfive
commented
Aug 14, 2016
|
|
|
|
| @@ -1,5 +1,7 @@ | |||
| [tex-image-and-sub-image-2d-with-webgl-canvas-rgba4444.html] | |||
| type: testharness | |||
| disabled: | |||
| if os == "linux": https://github.com/servo/servo/issues/12864 | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
anholt
Aug 22, 2016
Author
Contributor
Oh, right.
I was just autopiloting here. Given that I haven't actually changed anything about MacOS in this series, we should probably drop this commit and ignore the macos failure.
This comment has been minimized.
This comment has been minimized.
anholt
Aug 22, 2016
Author
Contributor
Scratch that. The original timeout was linux, this failure was reported on Mac because of the syntax error.
| @@ -1,4 +1,6 @@ | |||
| [tex-image-and-sub-image-2d-with-image-rgba4444.html] | |||
| disabled: | |||
| if os == "linux" | |||
This comment has been minimized.
This comment has been minimized.
It looks like we should basically skip the call of readPixels() in this case.
Across many test runs, I haven't seen them crash.
These don't appear on the i965 driver, but seem to be reliable on CI.
These don't crash on the i965 driver or my local 11.2.2 llvmpipe, but have shown intermittent failures in CI.
|
@bors-servo r=emilio |
|
|
Enable wpt WebGL tests on Linux <!-- Please describe your changes on the following line: --> Enables the WebGL tests in test-wpt on Linux. They have been disabled since the webgl conformance tests first showed up, and it looks like they should work now. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #9331 <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12858) <!-- Reviewable:end -->
|
|
|
In that last round, the macos timeouts on readpixels tests are unrelated to my changes. There are also unexpected passes reported on gl-bindAttribLocation-aliasing.html. I had thought the testsuite was treating the expected-FAIL in subtests as "if the test fails overall, treat these subtests as expected fail and report any ones not listed". Is that not the case? Do we need to annotate each line of tests/wpt/metadata/webgl/conformance-1.0.3/conformance/attribs/gl-bindAttribLocation-aliasing.html.ini with "if linux, then expect this"? |
|
|
|
We've got so many errors different between MacOS and Linux that trying to keep the expecteds updated for both is going to be madness. I'm shelving this project for now, and instead working on improving our implementation so that we don't have so many expecteds to manage. |
anholt commentedAug 14, 2016
•
edited by larsbergstrom
Enables the WebGL tests in test-wpt on Linux. They have been disabled since the webgl conformance tests first showed up, and it looks like they should work now.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is