Skip to content

Commit c092fe6

Browse files
melinathNandiniAgrawal15
authored andcommitted
Added check for multiple new resources in one PR (GoogleCloudPlatform#13937)
1 parent faeaef8 commit c092fe6

File tree

3 files changed

+49
-1
lines changed

3 files changed

+49
-1
lines changed

.ci/magician/cmd/generate_comment.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"os"
2222
"path/filepath"
2323
"regexp"
24+
"slices"
2425
"sort"
2526
"strconv"
2627
"strings"
@@ -84,6 +85,7 @@ type diffCommentData struct {
8485
MissingServiceLabels []string
8586
MissingTests map[string]*MissingTestInfo
8687
MissingDocs *MissingDocsSummary
88+
AddedResources []string
8789
Errors []Errors
8890
}
8991

@@ -93,6 +95,7 @@ type simpleSchemaDiff struct {
9395

9496
const allowBreakingChangesLabel = "override-breaking-change"
9597
const allowMissingServiceLabelsLabel = "override-missing-service-labels"
98+
const allowMultipleResourcesLabel = "override-multiple-resources"
9699

97100
var gcEnvironmentVariables = [...]string{
98101
"BUILD_ID",
@@ -363,6 +366,25 @@ func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep,
363366
})
364367
data.BreakingChanges = breakingChangesSlice
365368

369+
// Check if multiple resources were added.
370+
multipleResourcesState := "success"
371+
if len(uniqueAddedResources) > 1 {
372+
multipleResourcesState = "failure"
373+
for _, label := range pullRequest.Labels {
374+
if label.Name == allowMultipleResourcesLabel {
375+
multipleResourcesState = "success"
376+
break
377+
}
378+
}
379+
}
380+
targetURL := fmt.Sprintf("https://console.cloud.google.com/cloud-build/builds;region=global/%s;step=%s?project=%s", buildId, buildStep, projectId)
381+
if err = gh.PostBuildStatus(strconv.Itoa(prNumber), "terraform-provider-multiple-resources", multipleResourcesState, targetURL, commitSha); err != nil {
382+
fmt.Printf("Error posting terraform-provider-multiple-resources build status for pr %d commit %s: %v\n", prNumber, commitSha, err)
383+
errors["Other"] = append(errors["Other"], "Failed to update missing-service-labels status check with state: "+multipleResourcesState)
384+
}
385+
data.AddedResources = maps.Keys(uniqueAddedResources)
386+
slices.Sort(data.AddedResources)
387+
366388
// Compute affected resources based on changed files
367389
changedFilesAffectedResources := map[string]struct{}{}
368390
for _, repo := range []source.Repo{tpgRepo, tpgbRepo} {
@@ -427,7 +449,6 @@ func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep,
427449
}
428450
}
429451
}
430-
targetURL := fmt.Sprintf("https://console.cloud.google.com/cloud-build/builds;region=global/%s;step=%s?project=%s", buildId, buildStep, projectId)
431452
if err = gh.PostBuildStatus(strconv.Itoa(prNumber), "terraform-provider-breaking-change-test", breakingState, targetURL, commitSha); err != nil {
432453
fmt.Printf("Error posting terraform-provider-breaking-change-test build status for pr %d commit %s: %v\n", prNumber, commitSha, err)
433454
errors["Other"] = append(errors["Other"], "Failed to update breaking-change status check with state: "+breakingState)

.ci/magician/cmd/generate_comment_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ func TestExecGenerateComment(t *testing.T) {
129129

130130
for method, expectedCalls := range map[string][][]any{
131131
"PostBuildStatus": {
132+
{"123456", "terraform-provider-multiple-resources", "success", "https://console.cloud.google.com/cloud-build/builds;region=global/build1;step=17?project=project1", "sha1"},
132133
{"123456", "terraform-provider-breaking-change-test", "success", "https://console.cloud.google.com/cloud-build/builds;region=global/build1;step=17?project=project1", "sha1"},
133134
{"123456", "terraform-provider-missing-service-labels", "success", "https://console.cloud.google.com/cloud-build/builds;region=global/build1;step=17?project=project1", "sha1"},
134135
},
@@ -242,6 +243,25 @@ func TestFormatDiffComment(t *testing.T) {
242243
"## Missing test report",
243244
},
244245
},
246+
"multiple resources are displayed": {
247+
data: diffCommentData{
248+
AddedResources: []string{"google_redis_instance", "google_alloydb_cluster"},
249+
},
250+
expectedStrings: []string{
251+
"## Diff report",
252+
"## Multiple resources added",
253+
"`override-multiple-resources`",
254+
"split it into multiple PRs",
255+
"`google_redis_instance`, `google_alloydb_cluster`.",
256+
},
257+
notExpectedStrings: []string{
258+
"generated some diffs",
259+
"## Errors",
260+
"## Missing test report",
261+
"## Missing doc report",
262+
"## Breaking Change(s) Detected",
263+
},
264+
},
245265
"missing tests are displayed": {
246266
data: diffCommentData{
247267
MissingTests: map[string]*MissingTestInfo{

.ci/magician/cmd/templates/DIFF_COMMENT.md.tmpl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ If you believe this detection to be incorrect please raise the concern with your
5151
An `override-missing-service-label` label can be added to allow merging.
5252
{{end}}
5353

54+
{{- if gt (len .AddedResources) 1 }}
55+
## Multiple resources added
56+
57+
This PR adds multiple new resources: {{range $i, $resource := .AddedResources}}{{ if gt $i 0}}, {{end}}`{{$resource}}`{{end}}. This makes review significantly more difficult. Please split it into multiple PRs, one per resource.
58+
An `override-multiple-resources` label can be added to allow merging.
59+
{{end}}
60+
5461
{{- if and (.MissingDocs) (or .MissingDocs.Resource .MissingDocs.DataSource) }}
5562
## Missing doc report (experimental)
5663

0 commit comments

Comments
 (0)