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

Notify users when regressions are detected #912

Open
foolip opened this issue Dec 13, 2018 · 9 comments
Open

Notify users when regressions are detected #912

foolip opened this issue Dec 13, 2018 · 9 comments
Labels
enhancement New feature or request

Comments

@foolip
Copy link
Member

foolip commented Dec 13, 2018

Related to #866

@beaufortfrancois has reported this recent regression in picture-in-picture:
good: https://wpt.fyi/results/picture-in-picture?product=chrome[experimental,taskcluster]&sha=5dd605cdfa
bad: https://wpt.fyi/results/picture-in-picture?product=chrome[experimental,taskcluster]&sha=c522b884f7

The full diff between those runs:
https://wpt.fyi/results/?label=experimental&product=chrome%5Btaskcluster%5D%405dd605cdfa&product=chrome%5Btaskcluster%5D%40c522b884f7&diff

There are lots of regressions, introduced by web-platform-tests/wpt#13966. We should have detected that before landing the changes, but if it had been contained to picture-in-picture we should still have detected the regression when it happened. In addition to changes in the wpt repo, infrastructure changes and browser releases can also cause regresssions.

@jgraham @gsnedders FYI

@foolip
Copy link
Member Author

foolip commented Dec 13, 2018

Constructing the diff URLs in web-platform-tests/wpt#14495 was quite time-consuming, so automatically comparing all pairs of master runs for a configuration would help immensely.

@foolip
Copy link
Member Author

foolip commented Dec 13, 2018

Doing web-platform-tests/wpt#13263 would mitigate the need for this somewhat, but I'd still love to have links to diffs for each master run, even if there aren't regressions.

@lukebjerring
Copy link
Contributor

@foolip - the wpt.fyi custom check does compare each full master run to the most recent master run for the same product, and surfaces the regressions. It's noisy due to flakiness, but is that what this issue was asking for?

@foolip
Copy link
Member Author

foolip commented Jan 16, 2019

Flakiness isn't what I wanted to discover, what I have in mind is a change to testharness.js or something in tools/ breaking a lot of tests. In other words, we'd need to compare two runs, somehow account for flakiness, and see if there are regressions outside of the affected tests. If there are, some human should be notified.

@lukebjerring
Copy link
Contributor

OK - Then this isn't a wpt.fyi issue. wpt.fyi does detect regressions, but it gets ignored because of the flaky tests being noisy. The remaining work lies in fixing the flaky tests.

@foolip
Copy link
Member Author

foolip commented Jan 17, 2019

Well, dealing with flakiness might be a blocker to make this useful, but there's still wpt.fyi work:

  • see if there are regressions outside of the affected tests (or just if there are very many regressions)
  • human should be notified

Currently, even if 1000 tests started to fail on master, we'd only notice it by the numbers or colors on wpt.fyi looking unfamiliar to us.

@foolip
Copy link
Member Author

foolip commented Jan 17, 2019

Travis and Azure Pipelines can notify failures by email, maybe something like that could work? @lukebjerring you had some idea about notifications for other purposes already I think?

@lukebjerring
Copy link
Contributor

I'm confused. The custom checks run for the full test suite comparison, on master for all commits.
Example: https://github.com/web-platform-tests/wpt/runs/51187555
https://github.com/web-platform-tests/wpt/commits/master

Changing the title to reflect the change that you're asking for.

@lukebjerring lukebjerring reopened this Jan 17, 2019
@lukebjerring lukebjerring changed the title Detect regressions on master Notify users when regressions are detected Jan 17, 2019
@foolip
Copy link
Member Author

foolip commented Jan 17, 2019

Right, I know that the checks exist, but you have to go looking for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants