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

Improve visual testing status #43127

Closed
liza-mae opened this issue Aug 12, 2019 · 2 comments · Fixed by #139252
Closed

Improve visual testing status #43127

liza-mae opened this issue Aug 12, 2019 · 2 comments · Fixed by #139252

Comments

@liza-mae
Copy link
Contributor

liza-mae commented Aug 12, 2019

Had a discussion with @nreese about #42989 and how currently the Percy check is done at the Github level, which is working, however the Jenkins CI job sends an email about the test status, which does not look at the Percy results. So you can get a Github failure check on the Percy tests, but an email that is passing (green heart) which can be confusing. Also according to Nathan, the email is the primary tool used to merge their PR and it might be missed seeing the red 'X' on the PR itself, so we would need to add a validation so it's part of the email notifications too.

cc: @spalger

Part of #33817

@spalger
Copy link
Contributor

spalger commented Aug 14, 2019

I personally don't see the fact that Percy validation status is separate from the Jenkins test status as something that we should fix. Just like the CLA check, up-to-date check, or label checks, the status of the Percy validation is not managed or tracked by Jenkins.

When a user intentionally makes visual changes to Kibana the only way to update the snapshots is to run CI, visit the Percy UI, and then approve the changes. If we included the percy status in the Jenkins status then users would also have to re-run Jenkins in order to have a complete CI run without any visual changes, which would be a bad user experience.

Github already solves this for us by merging the different statuses from the Github checks into a single status for the whole commit/PR, I don't think we need to invent anything new here.

Related: #37962

@liza-mae
Copy link
Contributor Author

This is mainly for developers who rely on email notifications on whether their PR passed or failed, it would be nice to add some indication to those emails if there are visual regressions, as right now they would have to check it in Github. If only visual regression tests failed, then they don't need to rerun, they just need to approve the changes in the Percy dashboard and the Github check will turn green for the PR.

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

Successfully merging a pull request may close this issue.

2 participants