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 upPaint worklets: Add pref for blocking sleep to be enabled for wpt tests #19630
Conversation
highfive
commented
Dec 22, 2017
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon. |
In aa48a2c I added a timeout for paint worklet threads. However, the test was broken. The blocking sleep function that was implemented as part of that commit was guarded behind the `dom.worklet.blockingsleep.enabled` pref, and while I ran the wpt-tests with that pref enabled, the test runner for sure did not. I tried running the test _without_ the pref enabled, and the tests still pass. This is because even the reference in that reftest is that of a broken image background, and *both* the paintworklet thread timing out and `sleep()` not being in scope would render the same thing: a broken image, which compares equal to the reference. This patch makes sure that now the pref is enabled for wpt worklet tests, and that such we can distinguish an actual timeout (test pass) from an unexpected situation (when we should fail the test).
|
@bors-servo: r+ |
|
|
Paint worklets: Add pref for blocking sleep to be enabled for wpt tests In aa48a2c I added a timeout for paint worklet threads. However, the test was broken. The blocking sleep function that was implemented as part of that commit was guarded behind the `dom.worklet.blockingsleep.enabled` pref, and while I ran the wpt-tests with that pref enabled, the test runner for sure did not. I tried running the test _without_ the pref enabled, and the tests still pass. This is because even the reference in that reftest is that of a broken image background, and *both* the paintworklet thread timing out and `sleep()` not being in scope would render the same thing: a broken image, which compares equal to the reference. This patch makes sure that now the pref is enabled for wpt worklet tests, and that such we can distinguish an actual timeout (test pass) from an unexpected situation (when we should fail the test). <!-- Please describe your changes on the following line: --> --- <!-- 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 #__ (github issue number if applicable). (*not applicable*) <!-- Either: --> - [] There are tests for these changes OR - [X] These changes do not require tests because this patch fixes a broken test. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- 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/19630) <!-- Reviewable:end -->
|
|
|
@bors-servo: retry |
|
|
|
|
|
tests/wpt/mozilla/tests/mozilla/css-paint-api/paint2d-image.html appears to be failing. |
|
@bors-servo: retry |
|
|
|
|
|
phew, thanks @jdm for the kicks! |
|
Hi @yati-sagade, |
|
Hey, thanks for pointing out @tigercosmos. I did not know one can have multiple emails on a github account (added). |
|
nice! |
yati-sagade commentedDec 22, 2017
•
edited by SimonSapin
In aa48a2c I added a timeout for paint
worklet threads. However, the test was broken. The blocking sleep
function that was implemented as part of that commit was guarded behind
the
dom.worklet.blockingsleep.enabledpref, and while I ran thewpt-tests with that pref enabled, the test runner for sure did not.
I tried running the test without the pref enabled, and the tests still
pass. This is because even the reference in that reftest is that of
a broken image background, and both the paintworklet thread timing out
and
sleep()not being in scope would render the same thing: a brokenimage, which compares equal to the reference.
This patch makes sure that now the pref is enabled for wpt worklet
tests, and that such we can distinguish an actual timeout (test pass)
from an unexpected situation (when we should fail the test).
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is