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

Custom checks showing up on all commits on master, comparing against master #866

Closed
foolip opened this issue Dec 6, 2018 · 10 comments · Fixed by #868
Closed

Custom checks showing up on all commits on master, comparing against master #866

foolip opened this issue Dec 6, 2018 · 10 comments · Fixed by #868

Comments

@foolip
Copy link
Member

foolip commented Dec 6, 2018

In https://github.com/web-platform-tests/wpt/commits/master, every commit gets wpt.fyi "chrome[experimental] and "firefox[experimental]" checks. Example:
https://github.com/web-platform-tests/wpt/runs/38327762
https://github.com/web-platform-tests/wpt/runs/38327768

This could be very useful if the comparison was against the previous commit on master.

@foolip
Copy link
Member Author

foolip commented Dec 6, 2018

A comparison against the previous run of the same product would in particular come in handy after landing a change like web-platform-tests/wpt#14309 where some regressions might have slipped through.

@lukebjerring
Copy link
Contributor

I think the checks you're pointing at are (incorrectly) comparing the same run against itself, since the "latest master run" is the run itself. I'll use this issue to follow up on ensuring that we find the previous run in those cases.

As for making it show up for everyone, you'll just need to enable checksAllUsers. This isn't added in the /admin/flags view just yet - will add it.

@Hexcles
Copy link
Member

Hexcles commented Dec 6, 2018

By the way, the user whitelist seems to be not working. All commits from all users on master have the wpt.fyi custom checks.

@lukebjerring
Copy link
Contributor

It was that way a couple days ago, so you might just be seeing leftovers from that bug, but if you scroll the list a few don't have the check (e.g. commits from youenff)

@foolip
Copy link
Member Author

foolip commented Dec 10, 2018

For commit 226f3b5 3 checks are showing up:
image

But for the preceding commit no checks. Not sure why :)

@lukebjerring
Copy link
Contributor

The pr author is whitelisted, so their merge triggers a check suite

@gsnedders
Copy link
Member

The pr author is whitelisted, so their merge triggers a check suite

Why does authorship matter when the commit is on the wpt repo itself? If a commit has been pushed to the WPT repo itself, it's been pushed by someone we should consider trustworthy.

@foolip
Copy link
Member Author

foolip commented Dec 12, 2018

The list was just to try it out with a small number of people, not for security reasons.

@lukebjerring
Copy link
Contributor

Yah, temporary measure to reduce noise during unstable development.

@lukebjerring
Copy link
Contributor

This was fixed by #872

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

Successfully merging a pull request may close this issue.

4 participants