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

test: prevent auto comments on test failure issues with X-noreuse label #87708

Merged
merged 1 commit into from
Sep 28, 2022

Conversation

msbutler
Copy link
Collaborator

@msbutler msbutler commented Sep 9, 2022

Currently, when our nightly test suite detects a test failure, it either opens a new github issue or comments on an existing one with a title that roughly matches the failed test's name, using github search. Sometimes it's nice that fresh failures get posted to the existing issue, but in other situations, an engineering team may use the original test failure issue to track a specific bug. So, if the nightly CI build continues to comment on the original issue, perhaps due to a different failure mode, it becomes harder for the eng team to track different failure modes for a given test. Further, even if the eng team renamed part of the issue name to prevent automatic commenting, github search may still match and comment on the renamed issue.

This patch prevents test failures from getting automatically posted to existing github issues with the new X-noreuse label.

Release note: None

@msbutler msbutler added the T-testeng TestEng Team label Sep 9, 2022
@msbutler msbutler requested a review from tbg September 9, 2022 15:10
@msbutler msbutler requested a review from a team as a code owner September 9, 2022 15:10
@msbutler msbutler self-assigned this Sep 9, 2022
@msbutler msbutler requested review from herkolategan and removed request for a team September 9, 2022 15:10
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg
Copy link
Member

tbg commented Sep 9, 2022

LGTM

Just wanted to point out an alternative workflow: don't use test failure threads to track bugs. Instead, file a separate issue for the bug and mention the comment with the test failure in it. I think I personally tend to follow that strategy, since roachtest issues are never really that clean a source. For example, the interesting bug might be the fifth test failure in an existing issue.

I'm fine with this though, after all nobody's forced to do it that way.

Copy link
Collaborator

@herkolategan herkolategan left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @msbutler)

Currently, when our nightly test suite detects a test failure, it either opens
a new github issue or comments on an existing one with a title that roughly
matches the failed test's name, using github search. Sometimes it's nice that
fresh failures get posted to the existing issue, but in other situations, an
engineering team may use the original test failure issue to track a specific
bug. So, if the nightly CI build continues to comment on the original issue,
perhaps due to a different failure mode, it becomes harder for the eng team to
track different failure modes for a given test. Further, even if the eng team
renamed part of the issue name to prevent automatic commenting, github search
may still match and comment on the renamed issue.

This patch prevents test failures from getting automatically posted to existing
github issues with the new X-noreuse label.

Release note: None
@msbutler msbutler force-pushed the butler-roachtest-reuse branch from a1bb0fb to fa899d3 Compare September 28, 2022 14:04
@msbutler
Copy link
Collaborator Author

TFTRs!!

bors r= tbg, herkolategan

@craig
Copy link
Contributor

craig bot commented Sep 28, 2022

Build succeeded:

@craig craig bot merged commit 6d5dae1 into cockroachdb:master Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-testeng TestEng Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants