Summary: | REGRESSION (Lazy tree creation): css3/calc/transitions-dependent.html is frequently failing | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||
Component: | CSS | Assignee: | Diego Pino <dpino> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, dino, dpino, eocanha, graouts, kling, koivisto, Morningstar, simon.fraser, webkit-bug-importer | ||||||||
Priority: | P1 | Keywords: | InRadar, Regression | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=239990 | ||||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2013-12-22 17:47:07 PST
@@ -1,12 +1,12 @@ This tests that calc() expressions depending on transitioning elements behave correctly. -PASS - "width" property for "inner" element at 0s was: 60 -PASS - "width" property for "inner" element at 0.25s was: 110 -PASS - "width" property for "inner" element at 0.5s was: 160 -PASS - "width" property for "inner" element at 0.75s was: 210 +FAIL - "width" property for "inner" element at 0s expected: 60 but saw: 260 +FAIL - "width" property for "inner" element at 0.25s expected: 110 but saw: 260 +FAIL - "width" property for "inner" element at 0.5s expected: 160 but saw: 260 +FAIL - "width" property for "inner" element at 0.75s expected: 210 but saw: 260 PASS - "width" property for "inner" element at 1s was: 260 -PASS - "width" property for "innerTransition" element at 0s was: 20 -PASS - "width" property for "innerTransition" element at 0.25s was: 70 -PASS - "width" property for "innerTransition" element at 0.5s was: 165 -PASS - "width" property for "innerTransition" element at 0.75s was: 305 +FAIL - "width" property for "innerTransition" element at 0s expected: 20 but saw: 490 +FAIL - "width" property for "innerTransition" element at 0.25s expected: 70 but saw: 490 +FAIL - "width" property for "innerTransition" element at 0.5s expected: 165 but saw: 490 +FAIL - "width" property for "innerTransition" element at 0.75s expected: 305 but saw: 490 PASS - "width" property for "innerTransition" element at 1s was: 490 Marked the test as flaky in <http://trac.webkit.org/r161073>. Antti, did you have a chance to look into this? *** Bug 132182 has been marked as a duplicate of this bug. *** Created attachment 435670 [details]
Patch
Do the new results match other browsers? Not sure how to tell if the test is failing or passing now. Created attachment 444485 [details]
Patch
I modernized the code of the test to be able to run it in other browsers. I run it in Chrome and Firefox and I got the same expected results. Comment on attachment 444485 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444485&action=review > LayoutTests/TestExpectations:1209 > +css3/calc/transitions-dependent.html [ DumpJSConsoleLogInStdErr ] What does this test dump to console? DumpJSConsoleLogInStdErr is supposed to only be used for imported tests, which we cannot modify. > LayoutTests/css3/calc/transitions-dependent.html:77 > + let element = document.getElementById(elementId); Looks like existing code in this test uses four-space indentation. > LayoutTests/css3/calc/transitions-dependent.html:111 > + window.testRunner.waitUntilDone(); Ditto. Also, no need to prefix with window. on this line. Created attachment 448823 [details]
Patch
(In reply to Alexey Proskuryakov from comment #10) > Comment on attachment 444485 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444485&action=review > > > LayoutTests/TestExpectations:1209 > > +css3/calc/transitions-dependent.html [ DumpJSConsoleLogInStdErr ] > > What does this test dump to console? DumpJSConsoleLogInStdErr is supposed to > only be used for imported tests, which we cannot modify. > The `pauseTransitionAtTimeOnElement` call was printing out several messages on console: https://webkit-search.igalia.com/webkit/source/LayoutTests/transitions/resources/transition-test-helpers.js#374 It seems the first 5 expected values should only check the `dependsOn` property while the last 5 expected values should check the `elementId` property. I changed the test accordingly. Committed r287874 (245918@main): <https://commits.webkit.org/245918@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 448823 [details]. *** Bug 239254 has been marked as a duplicate of this bug. *** (In reply to Diego Pino from comment #12) > (In reply to Alexey Proskuryakov from comment #10) > > Comment on attachment 444485 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=444485&action=review > > > > > LayoutTests/TestExpectations:1209 > > > +css3/calc/transitions-dependent.html [ DumpJSConsoleLogInStdErr ] > > > > What does this test dump to console? DumpJSConsoleLogInStdErr is supposed to > > only be used for imported tests, which we cannot modify. > > > > The `pauseTransitionAtTimeOnElement` call was printing out several messages > on console: > > https://webkit-search.igalia.com/webkit/source/LayoutTests/transitions/ > resources/transition-test-helpers.js#374 > > It seems the first 5 expected values should only check the `dependsOn` > property while the last 5 expected values should check the `elementId` > property. > > I changed the test accordingly. I am still seeing a failure and the following console messages: +CONSOLE MESSAGE: A transition for property width could not be found +CONSOLE MESSAGE: A transition for property width could not be found +CONSOLE MESSAGE: A transition for property width could not be found +CONSOLE MESSAGE: A transition for property width could not be found +CONSOLE MESSAGE: A transition for property width could not be found +CONSOLE MESSAGE: A transition for property width could not be found +CONSOLE MESSAGE: A transition for property width could not be found +CONSOLE MESSAGE: A transition for property width could not be found +CONSOLE MESSAGE: A transition for property width could not be found +CONSOLE MESSAGE: A transition for property width could not be found I had created a new bug here https://bugs.webkit.org/show_bug.cgi?id=239254 But rather I shall mark that as a dupe and re-open this one. That appears to be a new different issue, so it should be tracked in a new bug. Please disregard my comment, I didn't analyze the history enough to tell if it's a different issue, or same one continued. Pull request: https://github.com/webkit/webkit/pull/3346 The regression is 100% reproducible when running the test at least 2 times: ``` $ Tools/Scripts/run-webkit-tests --release --gtk --iterations=2 css3/calc/transitions-dependent.html ``` In the cases where the actual results didn't match the expected results, I noticed it seemed that the issue was that the animation had already finished. For instance: https://build.webkit.org/results/GTK-Linux-64-bit-Debug-Tests/253440@main%20(6596)/css3/calc/transitions-dependent-pretty-diff.html In this test result, the outer animation as well as the innerTransition animation have for each of their intervals a width that corresponds to the value of the width when the animation ends. To prevent this error what I did in the patch was to set the animation on pause as soon as it's initialized and also create and initialize the animations separately (first the outer element and then innerTransition element). Once that is done I can no longer reproduce the error even by repeating the test several times: ``` $ Tools/Scripts/run-webkit-tests --release --gtk --iterations=2 --repeat=50 css3/calc/transitions-dependent.html ``` I think this will solve the flakiness of the test. Committed 255618@main (eae54080e781): <https://commits.webkit.org/255618@main> Reviewed commits have been landed. Closing PR #3346 and removing active labels. |