From 3d88fefff0adf4287f2590a6d7210afe305829ac Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Tue, 3 Dec 2024 12:14:18 +0000 Subject: [PATCH 1/4] githubpost: remove unused formatters from test --- pkg/cmd/bazci/githubpost/githubpost_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/cmd/bazci/githubpost/githubpost_test.go b/pkg/cmd/bazci/githubpost/githubpost_test.go index 683848fd6f98..dc6edd47194f 100644 --- a/pkg/cmd/bazci/githubpost/githubpost_test.go +++ b/pkg/cmd/bazci/githubpost/githubpost_test.go @@ -414,7 +414,6 @@ func TestListFailuresFromTestXML(t *testing.T) { fileName string expPkg string expIssues []issue - formatter Formatter }{ { fileName: "basic.xml", @@ -429,7 +428,6 @@ func TestListFailuresFromTestXML(t *testing.T) { --- FAIL: TestJSONErrors/frues (0.00s)`, mention: []string{"@cockroachdb/unowned"}, }}, - formatter: DefaultFormatter, }, } @@ -481,7 +479,6 @@ func TestPostGeneralFailure(t *testing.T) { testCases := []struct { fileName string expIssues []issue - formatter Formatter }{ { fileName: "failed-build-output.txt", @@ -490,7 +487,6 @@ func TestPostGeneralFailure(t *testing.T) { mention: []string{"@cockroachdb/unowned"}, labels: []string{"C-test-failure", "release-blocker", "T-testeng"}, }}, - formatter: DefaultFormatter, }, } From 10eab82e8ffebf3fcf60c00ecaf8925462716357 Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Tue, 3 Dec 2024 12:14:53 +0000 Subject: [PATCH 2/4] team: add `unittest` alias purpose --- pkg/internal/team/team.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/internal/team/team.go b/pkg/internal/team/team.go index 6f9bb9c4ee2c..4b538e485bf0 100644 --- a/pkg/internal/team/team.go +++ b/pkg/internal/team/team.go @@ -97,11 +97,13 @@ const ( // PurposeRoachtest indicates that the team handles that should be mentioned // in roachtest issues should be returned. PurposeRoachtest = Purpose("roachtest") + PurposeUnittest = Purpose("unittest") ) var validPurposes = map[Purpose]struct{}{ PurposeOther: {}, PurposeRoachtest: {}, // mention in roachtest issues + PurposeUnittest: {}, // mention in unit test issues } // LoadTeams loads the teams from an io input. From b25f499aa6ecb53940aadccf94c8eca568edb528 Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Tue, 3 Dec 2024 12:16:12 +0000 Subject: [PATCH 3/4] githubpost: prefer 'unittest' team alias(es) if available If a team defines one or multiple aliases for "unittest" purposes, CI/nightly issue posters now mention this one instead of the team's main alias. --- pkg/cmd/bazci/githubpost/githubpost.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/bazci/githubpost/githubpost.go b/pkg/cmd/bazci/githubpost/githubpost.go index b9ae6073f600..9f78be824fe9 100644 --- a/pkg/cmd/bazci/githubpost/githubpost.go +++ b/pkg/cmd/bazci/githubpost/githubpost.go @@ -68,12 +68,21 @@ func DefaultFormatter(ctx context.Context, f Failure) (issues.IssueFormatter, is } if len(teams) > 0 { projColID = teams[0].TriageColumnID - for _, team := range teams { - if !team.SilenceMentions { - mentions = append(mentions, "@"+string(team.Name())) + for _, tm := range teams { + if !tm.SilenceMentions { + var hasAliases bool + for al, purp := range tm.Aliases { + if purp == team.PurposeUnittest { + hasAliases = true + mentions = append(mentions, "@"+string(al)) + } + } + if !hasAliases { + mentions = append(mentions, "@"+string(tm.Name())) + } } - if team.Label != "" { - labels = append(labels, team.Label) + if tm.Label != "" { + labels = append(labels, tm.Label) } } } From 6867acb5580c7e6c4c4997038122e3b9c1825970 Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Tue, 3 Dec 2024 12:33:17 +0000 Subject: [PATCH 4/4] TEAMS: ping kv-triage for unittest failures Our triage process is robust without these mentions. They can go to (at)cockroachdb/kv-notifications for those inclined to consume a firehose. This change frees up our main alias (at)cockroachdb/kv for more meaningful notifications. --- TEAMS.yaml | 4 ++++ pkg/cmd/bazci/githubpost/githubpost.go | 2 +- pkg/cmd/bazci/githubpost/githubpost_test.go | 12 ++++++------ pkg/internal/team/TEAMS.yaml | 4 ++++ 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/TEAMS.yaml b/TEAMS.yaml index c24281ad4826..5061dd7bdbc2 100644 --- a/TEAMS.yaml +++ b/TEAMS.yaml @@ -41,6 +41,10 @@ cockroachdb/sql-queries: cockroachdb/kv: aliases: cockroachdb/kv-triage: roachtest + # This is a hack - we can't have the same key twice, + # but it also doesn't seem worth rewriting the aliases + # into slice form for just this. + 'cockroachdb/kv-triage ': unittest cockroachdb/kv-prs: other triage_column_id: 14242655 label: T-kv diff --git a/pkg/cmd/bazci/githubpost/githubpost.go b/pkg/cmd/bazci/githubpost/githubpost.go index 9f78be824fe9..09cdfbe7ce9c 100644 --- a/pkg/cmd/bazci/githubpost/githubpost.go +++ b/pkg/cmd/bazci/githubpost/githubpost.go @@ -74,7 +74,7 @@ func DefaultFormatter(ctx context.Context, f Failure) (issues.IssueFormatter, is for al, purp := range tm.Aliases { if purp == team.PurposeUnittest { hasAliases = true - mentions = append(mentions, "@"+string(al)) + mentions = append(mentions, "@"+strings.TrimSpace(string(al))) } } if !hasAliases { diff --git a/pkg/cmd/bazci/githubpost/githubpost_test.go b/pkg/cmd/bazci/githubpost/githubpost_test.go index dc6edd47194f..da74bcd969bd 100644 --- a/pkg/cmd/bazci/githubpost/githubpost_test.go +++ b/pkg/cmd/bazci/githubpost/githubpost_test.go @@ -101,7 +101,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestStopperWithCancelConcurrent", title: "util/stop: TestStopperWithCancelConcurrent failed", message: "this is just a testing issue", - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }}, @@ -117,7 +117,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestStopperWithCancelConcurrent", title: "util/stop: TestStopperWithCancelConcurrent failed", message: "this is just a testing issue", - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"T-kv"}, hasProject: true, }}, @@ -131,7 +131,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestReplicateQueueRebalance", title: "kv/kvserver: TestReplicateQueueRebalance failed", message: "replicate_queue_test.go:88: condition failed to evaluate within 45s: not balanced: [10 1 10 1 8]", - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }}, @@ -145,7 +145,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestGossipHandlesReplacedNode", title: "kv/kvserver: TestGossipHandlesReplacedNode failed", message: "F180711 20:13:15.826193 83 storage/replica.go:1877 [n?,s1,r1/1:/M{in-ax}] on-disk and in-memory state diverged:", - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }}, @@ -192,7 +192,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestTxnCoordSenderPipelining", title: "kv/kvclient/kvcoord: TestTxnCoordSenderPipelining failed", message: `injected failure`, - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }, @@ -206,7 +206,7 @@ TestTxnCoordSenderPipelining - 1.00s Slow passing tests: TestAnchorKey - 1.01s `, - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }, diff --git a/pkg/internal/team/TEAMS.yaml b/pkg/internal/team/TEAMS.yaml index c24281ad4826..5061dd7bdbc2 100644 --- a/pkg/internal/team/TEAMS.yaml +++ b/pkg/internal/team/TEAMS.yaml @@ -41,6 +41,10 @@ cockroachdb/sql-queries: cockroachdb/kv: aliases: cockroachdb/kv-triage: roachtest + # This is a hack - we can't have the same key twice, + # but it also doesn't seem worth rewriting the aliases + # into slice form for just this. + 'cockroachdb/kv-triage ': unittest cockroachdb/kv-prs: other triage_column_id: 14242655 label: T-kv