From 59d91f3e530f62e1789e49f58ec2214818ad5f1f Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Wed, 13 Dec 2023 11:39:29 +0000 Subject: [PATCH] Merge comments on symlinks --- docs/changelog.md | 6 + internal/reporter/bitbucket_api.go | 3 - internal/reporter/bitbucket_comments_test.go | 265 +++++++++++++++++++ internal/reporter/bitbucket_test.go | 15 +- 4 files changed, 274 insertions(+), 15 deletions(-) create mode 100644 internal/reporter/bitbucket_comments_test.go diff --git a/docs/changelog.md b/docs/changelog.md index 16f7ad99..9d6161a7 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,11 @@ # Changelog +## v0.52.0 + +### Changed + +- More reports will now be merged into a single comments when using BitBucket. + ## v0.51.1 ### Fixed diff --git a/internal/reporter/bitbucket_api.go b/internal/reporter/bitbucket_api.go index 13125788..23048fd6 100644 --- a/internal/reporter/bitbucket_api.go +++ b/internal/reporter/bitbucket_api.go @@ -853,9 +853,6 @@ func dedupReports(src []Report) (dst [][]Report) { if d[0].ReportedPath != report.ReportedPath { continue } - if d[0].SourcePath != report.SourcePath { - continue - } if d[0].Problem.Lines[0] != report.Problem.Lines[0] { continue } diff --git a/internal/reporter/bitbucket_comments_test.go b/internal/reporter/bitbucket_comments_test.go new file mode 100644 index 00000000..a146d916 --- /dev/null +++ b/internal/reporter/bitbucket_comments_test.go @@ -0,0 +1,265 @@ +package reporter + +import ( + "fmt" + "log/slog" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/neilotoole/slogt" + + "github.com/cloudflare/pint/internal/checks" +) + +func TestBitBucketMakeComments(t *testing.T) { + type testCaseT struct { + description string + summary Summary + changes *bitBucketPRChanges + comments []BitBucketPendingComment + } + + commentBody := func(icon, severity, reporter, text string) string { + return fmt.Sprintf( + ":%s: **%s** reported by [pint](https://cloudflare.github.io/pint/) **%s** check.\n\n------\n\n%s\n\n------\n\n:information_source: To see documentation covering this check and instructions on how to resolve it [click here](https://cloudflare.github.io/pint/checks/%s.html).\n", + icon, severity, reporter, text, reporter, + ) + } + + testCases := []testCaseT{ + { + description: "empty summary", + comments: []BitBucketPendingComment{}, + }, + { + description: "report not included in changes", + summary: Summary{reports: []Report{ + { + ReportedPath: "rule.yaml", + SourcePath: "rule.yaml", + ModifiedLines: []int{2, 3}, + Problem: checks.Problem{ + Severity: checks.Bug, + }, + }, + }}, + changes: &bitBucketPRChanges{}, + comments: []BitBucketPendingComment{}, + }, + { + description: "reports included in changes", + summary: Summary{reports: []Report{ + { + ReportedPath: "rule.yaml", + SourcePath: "rule.yaml", + ModifiedLines: []int{2, 3}, + Problem: checks.Problem{ + Severity: checks.Bug, + Lines: []int{2}, + Text: "first error", + Details: "first details", + Reporter: "r1", + }, + }, + { + ReportedPath: "rule.yaml", + SourcePath: "rule.yaml", + ModifiedLines: []int{2, 3}, + Problem: checks.Problem{ + Severity: checks.Warning, + Lines: []int{3}, + Text: "second error", + Details: "second details", + Reporter: "r1", + }, + }, + { + ReportedPath: "rule.yaml", + SourcePath: "rule.yaml", + ModifiedLines: []int{2, 3}, + Problem: checks.Problem{ + Severity: checks.Bug, + Lines: []int{2}, + Text: "third error", + Details: "third details", + Reporter: "r2", + }, + }, + { + ReportedPath: "rule.yaml", + SourcePath: "symlink.yaml", + ModifiedLines: []int{2, 3}, + Problem: checks.Problem{ + Severity: checks.Bug, + Lines: []int{2}, + Text: "fourth error", + Details: "fourth details", + Reporter: "r2", + }, + }, + { + ReportedPath: "second.yaml", + SourcePath: "second.yaml", + ModifiedLines: []int{1, 2, 3}, + Problem: checks.Problem{ + Anchor: checks.AnchorBefore, + Severity: checks.Bug, + Lines: []int{2}, + Text: "fifth error", + Details: "fifth details", + Reporter: "r2", + }, + }, + { + ReportedPath: "second.yaml", + SourcePath: "second.yaml", + ModifiedLines: []int{1, 2, 3}, + Problem: checks.Problem{ + Severity: checks.Bug, + Lines: []int{2}, + Text: "sixth error", + Details: "sixth details", + Reporter: "r2", + }, + }, + }}, + changes: &bitBucketPRChanges{ + pathModifiedLines: map[string][]int{ + "rule.yaml": {2, 3}, + "second.yaml": {1, 2, 3}, + }, + pathLineMapping: map[string]map[int]int{ + "rule.yaml": {2: 2, 3: 3}, + "second.yaml": {1: 5, 2: 6, 3: 7}, + }, + }, + comments: []BitBucketPendingComment{ + { + Text: commentBody("stop_sign", "Bug", "r1", "first error\n\nfirst details"), + Severity: "BLOCKER", + Anchor: BitBucketPendingCommentAnchor{ + Path: "rule.yaml", + Line: 2, + LineType: "ADDED", + FileType: "TO", + DiffType: "EFFECTIVE", + }, + }, + { + Text: commentBody("warning", "Warning", "r1", "second error\n\nsecond details"), + Severity: "NORMAL", + Anchor: BitBucketPendingCommentAnchor{ + Path: "rule.yaml", + Line: 3, + LineType: "ADDED", + FileType: "TO", + DiffType: "EFFECTIVE", + }, + }, + { + Text: commentBody("stop_sign", "Bug", "r2", "third error\n\nthird details\n\n------\n\nfourth error\n\nfourth details\n\n:leftwards_arrow_with_hook: This problem was detected on a symlinked file `symlink.yaml`."), + Severity: "BLOCKER", + Anchor: BitBucketPendingCommentAnchor{ + Path: "rule.yaml", + Line: 2, + LineType: "ADDED", + FileType: "TO", + DiffType: "EFFECTIVE", + }, + }, + { + Text: commentBody("stop_sign", "Bug", "r2", "fifth error\n\nfifth details"), + Severity: "BLOCKER", + Anchor: BitBucketPendingCommentAnchor{ + Path: "second.yaml", + Line: 2, + LineType: "REMOVED", + FileType: "FROM", + DiffType: "EFFECTIVE", + }, + }, + { + Text: commentBody("stop_sign", "Bug", "r2", "sixth error\n\nsixth details"), + Severity: "BLOCKER", + Anchor: BitBucketPendingCommentAnchor{ + Path: "second.yaml", + Line: 2, + LineType: "ADDED", + FileType: "TO", + DiffType: "EFFECTIVE", + }, + }, + }, + }, + { + description: "dedup reporter", + summary: Summary{reports: []Report{ + { + ReportedPath: "rule.yaml", + SourcePath: "rule.yaml", + ModifiedLines: []int{2, 3}, + Problem: checks.Problem{ + Severity: checks.Bug, + Lines: []int{2}, + Text: "first error", + Details: "first details", + Reporter: "r1", + }, + }, + { + ReportedPath: "rule.yaml", + SourcePath: "rule.yaml", + ModifiedLines: []int{2, 3}, + Problem: checks.Problem{ + Severity: checks.Bug, + Lines: []int{2}, + Text: "second error", + Details: "second 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\nfirst details\n\n------\n\nsecond error\n\nsecond details"), + Severity: "BLOCKER", + Anchor: BitBucketPendingCommentAnchor{ + Path: "rule.yaml", + Line: 2, + LineType: "ADDED", + FileType: "TO", + DiffType: "EFFECTIVE", + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + slog.SetDefault(slogt.New(t)) + r := NewBitBucketReporter( + "v0.0.0", + "http://bitbucket.example.com", + time.Second, + "token", + "proj", + "repo", + nil) + comments := r.api.makeComments(tc.summary, tc.changes) + if diff := cmp.Diff(tc.comments, comments); diff != "" { + t.Errorf("api.makeComments() returned wrong output (-want +got):\n%s", diff) + return + } + }) + } +} diff --git a/internal/reporter/bitbucket_test.go b/internal/reporter/bitbucket_test.go index 80791bf6..5a04f200 100644 --- a/internal/reporter/bitbucket_test.go +++ b/internal/reporter/bitbucket_test.go @@ -1616,18 +1616,7 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - Text: ":warning: **Warning** reported by [pint](https://cloudflare.github.io/pint/) **mock** check.\n\n------\n\nbad name\n\nbad name details\n\n------\n\nmock text 1\n\nmock details\n\n------\n\n:information_source: To see documentation covering this check and instructions on how to resolve it [click here](https://cloudflare.github.io/pint/checks/mock.html).\n", - Severity: "NORMAL", - Anchor: reporter.BitBucketPendingCommentAnchor{ - Path: "foo.txt", - Line: 2, - LineType: "ADDED", - FileType: "TO", - DiffType: "EFFECTIVE", - }, - }, - { - Text: ":warning: **Warning** reported by [pint](https://cloudflare.github.io/pint/) **mock** check.\n\n------\n\nmock text 2\n\nmock details\n\n:leftwards_arrow_with_hook: This problem was detected on a symlinked file `symlink.txt`.\n\n------\n\n:information_source: To see documentation covering this check and instructions on how to resolve it [click here](https://cloudflare.github.io/pint/checks/mock.html).\n", + Text: ":warning: **Warning** reported by [pint](https://cloudflare.github.io/pint/) **mock** check.\n\n------\n\nbad name\n\nbad name details\n\n------\n\nmock text 1\n\nmock details\n\n------\n\nmock text 2\n\nmock details\n\n:leftwards_arrow_with_hook: This problem was detected on a symlinked file `symlink.txt`.\n\n------\n\n:information_source: To see documentation covering this check and instructions on how to resolve it [click here](https://cloudflare.github.io/pint/checks/mock.html).\n", Severity: "NORMAL", Anchor: reporter.BitBucketPendingCommentAnchor{ Path: "foo.txt", @@ -1916,6 +1905,8 @@ func TestBitBucketReporter(t *testing.T) { t.Errorf("error check failure: %s", e) return } + + require.Len(t, tc.pullRequestComments, commentIndex) }) } }