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

consider disallowing skip_report: false for non-blocking jobs #4932

Closed
BenTheElder opened this issue Oct 9, 2017 · 7 comments
Closed

consider disallowing skip_report: false for non-blocking jobs #4932

BenTheElder opened this issue Oct 9, 2017 · 7 comments
Labels
area/jobs area/prow Issues or PRs related to prow lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Milestone

Comments

@BenTheElder
Copy link
Member

I think at least currently these jobs have largely been a source of confusion rather than a useful signal. Jobs can always still be checked via gubernator / deck / test-grid without having their status on GitHub. We should consider updating the config presubmits to disallow non-blocking jobs being set to report to GitHub.

Previously brought up here: #4913 (comment)

/area prow

@k8s-ci-robot k8s-ci-robot added the area/prow Issues or PRs related to prow label Oct 9, 2017
@BenTheElder BenTheElder changed the title disallow skip_report: false for non-blocking jobs consider disallowing skip_report: false for non-blocking jobs Oct 9, 2017
@k8s-ci-robot k8s-ci-robot added the area/prow Issues or PRs related to prow label Oct 9, 2017
@0xmichalis
Copy link
Contributor

cc @stevekuznetsov

@stevekuznetsov
Copy link
Contributor

Generally I think the comment should be re-structured ... see #4602

If we create a comment that has clear messaging on "these are the tests that are currently valid for your PR, tested against your latest HEAD, please look at Gubernator for historical data" I believe you are correct @BenTheElder that only blocking tests should publish statuses

@BenTheElder
Copy link
Member Author

I think besides the bot messages, I've been seeing developers concerned with getting a full set of "green checks" for their PRs and that showing jobs they don't need one for has only caused confusion and frustration.
We can easily fix that by simply not reporting them to GitHub and enforcing that with a presubmit, but I'm not sure if maybe there might be some valid reasons behind our currently allowing them to be reported on GitHub instead of just gubernator / deck / testgrid.

@stevekuznetsov
Copy link
Contributor

Yeah, GitHub has this first-class concept of status for commits which is a natural place for tests to report but it is 100% conflated with "what needs to happen for merge". The status for the submit-queue (or tide, or whatever) is a LOT more complex (have these labels, don't have those, have this milestone, etc) so I agree that using the "green checkmarks" to message to developers things that they need to merge is more appropriate and less confusing than the current approach, as long as they still have somewhere obvious to look at test results.

@BenTheElder
Copy link
Member Author

Discussed today at sig-testing meeting, we intend to make this reality. I've had to fix a few other things on the way but I'll have a PR to make this happen shortly and then we should notify the broader community.

Also @stevekuznetsov brought up some more good points about improving the test result bot comments which we should follow up with at some point.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 18, 2018
@BenTheElder
Copy link
Member Author

we cleaned this up a bit but we're still not forcing it, this won't make as much sense with tide anyhow.

/close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/jobs area/prow Issues or PRs related to prow lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

6 participants