Dedup report details

cloudflare · Mar 25, 2024 · 0ef4580 · 0ef4580
1 parent 54ccdd8
commit 0ef4580
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 2 deletions.
diff --git a/internal/reporter/bitbucket_api.go b/internal/reporter/bitbucket_api.go
@@ -267,6 +267,7 @@ type bitBucketAPI struct {
 	project     string
 	repo        string
 	timeout     time.Duration
+	maxComments int
 }
 
 func (bb bitBucketAPI) request(method, path string, body io.Reader) ([]byte, error) {
@@ -586,13 +587,16 @@ func (bb bitBucketAPI) getPullRequestComments(pr *bitBucketPR) ([]bitBucketComme
 }
 
 func (bb bitBucketAPI) makeComments(summary Summary, changes *bitBucketPRChanges) []BitBucketPendingComment {
+	var buf strings.Builder
 	comments := []BitBucketPendingComment{}
 	for _, reports := range dedupReports(summary.reports) {
 		if _, ok := changes.pathModifiedLines[reports[0].ReportedPath]; !ok {
 			continue
 		}
 
-		var buf strings.Builder
+		mergeDetails := identicalDetails(reports)
+
+		buf.Reset()
 
 		buf.WriteString(problemIcon(reports[0].Problem.Severity))
 		buf.WriteString(" **")
@@ -604,7 +608,7 @@ func (bb bitBucketAPI) makeComments(summary Summary, changes *bitBucketPRChanges
 			buf.WriteString("------\n\n")
 			buf.WriteString(report.Problem.Text)
 			buf.WriteString("\n\n")
-			if report.Problem.Details != "" {
+			if !mergeDetails && report.Problem.Details != "" {
 				buf.WriteString(report.Problem.Details)
 				buf.WriteString("\n\n")
 			}
@@ -615,6 +619,11 @@ func (bb bitBucketAPI) makeComments(summary Summary, changes *bitBucketPRChanges
 				buf.WriteString("`.\n\n")
 			}
 		}
+		if mergeDetails && reports[0].Problem.Details != "" {
+			buf.WriteString("------\n\n")
+			buf.WriteString(reports[0].Problem.Details)
+			buf.WriteString("\n\n")
+		}
 		buf.WriteString("------\n\n")
 		buf.WriteString(":information_source: To see documentation covering this check and instructions on how to resolve it [click here](https://cloudflare.github.io/pint/checks/")
 		buf.WriteString(reports[0].Problem.Reporter)
@@ -878,6 +887,23 @@ func dedupReports(src []Report) (dst [][]Report) {
 	return dst
 }
 
+func identicalDetails(src []Report) bool {
+	if len(src) <= 1 {
+		return false
+	}
+	var details string
+	for _, report := range src {
+		if details == "" {
+			details = report.Problem.Details
+			continue
+		}
+		if details != report.Problem.Details {
+			return false
+		}
+	}
+	return true
+}
+
 func problemIcon(s checks.Severity) string {
 	// nolint:exhaustive
 	switch s {

diff --git a/internal/reporter/bitbucket_comments_test.go b/internal/reporter/bitbucket_comments_test.go
@@ -323,6 +323,62 @@ func TestBitBucketMakeComments(t *testing.T) {
 				},
 			},
 		},
+		{
+			description: "dedup details",
+			summary: Summary{reports: []Report{
+				{
+					ReportedPath:  "rule.yaml",
+					SourcePath:    "rule.yaml",
+					ModifiedLines: []int{2, 3},
+					Problem: checks.Problem{
+						Severity: checks.Bug,
+						Lines: parser.LineRange{
+							First: 2,
+							Last:  2,
+						},
+						Text:     "first error",
+						Details:  "shared details",
+						Reporter: "r1",
+					},
+				},
+				{
+					ReportedPath:  "rule.yaml",
+					SourcePath:    "rule.yaml",
+					ModifiedLines: []int{2, 3},
+					Problem: checks.Problem{
+						Severity: checks.Bug,
+						Lines: parser.LineRange{
+							First: 2,
+							Last:  2,
+						},
+						Text:     "second error",
+						Details:  "shared details",
+						Reporter: "r1",
+					},
+				},
+			}},
+			changes: &bitBucketPRChanges{
+				pathModifiedLines: map[string][]int{
+					"rule.yaml": {2, 3},
+				},
+				pathLineMapping: map[string]map[int]int{
+					"rule.yaml": {2: 2, 3: 3},
+				},
+			},
+			comments: []BitBucketPendingComment{
+				{
+					Text:     commentBody("stop_sign", "Bug", "r1", "first error\n\n------\n\nsecond error\n\n------\n\nshared details"),
+					Severity: "BLOCKER",
+					Anchor: BitBucketPendingCommentAnchor{
+						Path:     "rule.yaml",
+						Line:     2,
+						LineType: "ADDED",
+						FileType: "TO",
+						DiffType: "EFFECTIVE",
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {