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

Only use a single comment for the flakiness report on PRs #46785

Merged

Conversation

kevin940726
Copy link
Member

What?

Continued from #45798, instead of posting a comment on each commit, we only post a single comment on PRs.

Why?

  1. We currently have too many flaky tests and some of them are way too flaky than expected. This is causing the job to post comments on nearly every commit, which is super noisy.
  2. Comments on commits will send notifications separately, this is also annoying and too verbose.

How?

This PR updates the system so that it will stop posting comments on commits if it's triggered by PRs. Instead, it will post a single comment on the PR and update it.

One downside of this is that only the latest commit on the PR will be shown in the comment, and other commits will be hidden in the edit history. I think this is acceptable as oftentimes only the latest run is all we care about.

Testing Instructions

This can be tested with a fork on your own repo.

Or you can view an example PR here: kevin940726#43 (comment)

Screenshots or screencast

image

@kevin940726 kevin940726 added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Dec 26, 2022
@alexstine
Copy link
Contributor

I do not know enough about GitHub SDK to approve this, but this looks much less noisy. 👍 from me for the concept.

Copy link
Member

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm leaving my approval on this because I think it's much better than what's happening now, but I still think the better option is reverting #45798. It still feels to me like this approach is addressing the wrong problem and we're raising alarm to people who won't be in a good position to fix things while continuing to not alert the people who'll be.

I'll leave more comment in the original PR in response to what you wrote there, but again, you have my approval because any reduction in the amount of distraction this brings me will be a relief.

@kevin940726 kevin940726 merged commit 7de7a42 into trunk Jan 5, 2023
@kevin940726 kevin940726 deleted the update/use-single-comment-for-flakiness-report-on-pr branch January 5, 2023 15:24
@github-actions github-actions bot added this to the Gutenberg 15.0 milestone Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants