Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update reversing-an-animation test #20559

Merged
merged 1 commit into from
Dec 5, 2019
Merged

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Dec 2, 2019

Double rAF start of the test to avoid starting the animation while too
busy to handle updates in a timely manner.

Bug: 1029541
Change-Id: I3e0e665f0b4b9dd6f6a87d84d207b0c18a1b51c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1946668
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721486}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1946668 branch 5 times, most recently from d7f11da to 0f28b92 Compare December 4, 2019 15:08
Double rAF start of the test to avoid starting the animation while too
busy to handle updates in a timely manner.

Bug: 1029541
Change-Id: I3e0e665f0b4b9dd6f6a87d84d207b0c18a1b51c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1946668
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721486}
@stephenmcgruer
Copy link
Contributor

Looks like the test was flaky:

0:39.89 INFO |                                   Test                                   | Subtest |          Results           | Messages |
 0:39.89 INFO |--------------------------------------------------------------------------|---------|----------------------------|----------|
 0:39.89 INFO | `/web-animations/timing-model/animations/reverse-running-animation.html` |         | **FAIL: 6/10, PASS: 4/10** |          |

Seems like pre-existing flake: https://wpt.fyi/results/web-animations/timing-model/animations?label=master&label=experimental&max-count=10&product=chrome&q=seq%28%28status%3APASS%7Cstatus%3AOK%29%20%28status%3A%21PASS%26status%3A%21OK%26status%3A%21unknown%29%29%20seq%28%28status%3A%21PASS%26status%3A%21OK%26status%3A%21unknown%29%20%28status%3APASS%7Cstatus%3AOK%29%29

XazgG15zrLR

Admin merging, and will ping CL author as an FYI.

@stephenmcgruer stephenmcgruer merged commit 85a8c5a into master Dec 5, 2019
@stephenmcgruer stephenmcgruer deleted the chromium-export-cl-1946668 branch December 5, 2019 12:47
@hiikezoe
Copy link
Contributor

The test actually pretty flaky on Firefox, https://bugzilla.mozilla.org/show_bug.cgi?id=1601322.

I suppose the test expects that the snapshot is taken after the animation in question finished? If so, can we wait for an animation finish event instead of the flaky 900ms delay?
It may be possible that anim.reverse() call in a callback of setTimout(500) is another source of the flakiness. Given that setTimeout callbacks aren't reliably invoked, it might be too late to call reverse().

@hiikezoe
Copy link
Contributor

@stephenmcgruer can you please this issue bounce to the test author Kevin? I couldn't find his account on github.

@birtles
Copy link
Contributor

birtles commented Dec 18, 2019

@stephenmcgruer can you please this issue bounce to the test author Kevin? I couldn't find his account on github.

I think Kevin might be @kevers-google

@stephenmcgruer
Copy link
Contributor

@hiikezoe - thanks for bringing this to my attention. This is being tracked in https://crbug.com/1029543

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants