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.

Sign up for GitHub

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

Jump to bottom

Percy test appears flaky with three dots on visual diff #10022

Closed
honeybadgerdontcare opened this issue Jun 19, 2017 · 6 comments · Fixed by #10078
Closed

Percy test appears flaky with three dots on visual diff #10022

honeybadgerdontcare opened this issue Jun 19, 2017 · 6 comments · Fixed by #10078
Assignees
@rsimha
Labels
P1: High Priority WG: infra
Milestone
Sprint H2 June

Comments

@honeybadgerdontcare
Copy link
Contributor

Several PRs are getting the same visual diff issue and causes me to think it may be a flaky test with percy.

Examples:
https://percy.io/ampproject/amphtml/builds/256910
https://percy.io/ampproject/amphtml/builds/256713
https://percy.io/ampproject/amphtml/builds/256555
https://percy.io/ampproject/amphtml/builds/256464
https://percy.io/ampproject/amphtml/builds/255472

More can be found at:
https://percy.io/ampproject/amphtml
@rsimha
Copy link
Contributor

rsimha commented Jun 19, 2017

@honeybadgerdontcare, I noticed this, searched for an existing issue, didn't find one, and filed #10024, only to find out that you had filed this a few seconds before me :)

@cramforce
Copy link
Member

@rsimha-amp Waiting for window.onload will help. To make it more robust you will need something like this code from performance-impl.js:

this.resources_.getResourcesInRect(
        this.win, rect, /* isInPrerender */ false)
        .then(resources => Promise.all(resources.map(r => r.loadedOnce())))

@rsimha
Copy link
Contributor

rsimha commented Jun 19, 2017

I believe there's a way to have percy/capybara wait for a particular CSS element to be visible on a page before taking a snapshot. Will take a look.

http://www.rubydoc.info/github/jnicklas/capybara#asynchronous-javascript-ajax-and-friends

@rsimha rsimha added this to the Sprint H2 June milestone Jun 19, 2017
@cramforce
Copy link
Member

@rsimha-amp I don't think AMP has a clear signal, but it would be OK to add one.

@rsimha rsimha mentioned this issue Jun 21, 2017
Merged
rsimha added a commit that referenced this issue Jun 21, 2017
@rsimha
Wait for page load before Percy snapshot (#10078)
c3edf47 
Speculative fix that I believe should work as an implicit wait for the page to fully load.

Tested this by searching for .i-amphtml-layout, which is a CSS element that does exist on the page, and the call to has_no_css waited for a couple of seconds before returning false.

Fixes #10022
This was referenced Jun 21, 2017
Closed
Merged
Merged
@rsimha
Copy link
Contributor

rsimha commented Jun 22, 2017 •
edited

25 straight green Percy builds so far, since #10078 was merged. Looks like the fix worked. Will keep an eye out for more timing related failures.

@rsimha
Copy link
Contributor

rsimha commented Jun 23, 2017

100+ green runs in a row. Consider this fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rsimha rsimha

Labels
P1: High Priority WG: infra
Projects
None yet
Milestone
Sprint H2 June
Development

Successfully merging a pull request may close this issue.

Wait for page load before Percy snapshot
3 participants
@cramforce @honeybadgerdontcare @rsimha