Refactor github reporer to use common comments code

cmd/pint/ci.go

@@ -171,7 +171,7 @@ func actionCI(c *cli.Context) error {
  ); err != nil {
  return err
  }
- reps = append(reps, gl)
+ reps = append(reps, reporter.NewCommentReporter(gl))
  }
 
  meta.cfg.Repository = detectRepository(meta.cfg.Repository)
@@ -191,6 +191,12 @@ func actionCI(c *cli.Context) error {
  return fmt.Errorf("got not a valid number via GITHUB_PULL_REQUEST_NUMBER: %w", err)
  }
 
+ var headCommit string
+ headCommit, err = git.HeadCommit(git.RunGit)
+ if err != nil {
+ return errors.New("failed to get the HEAD commit")
+ }
+
  timeout, _ := time.ParseDuration(meta.cfg.Repository.GitHub.Timeout)
  var gr reporter.GithubReporter
  if gr, err = reporter.NewGithubReporter(
@@ -203,11 +209,11 @@ func actionCI(c *cli.Context) error {
  meta.cfg.Repository.GitHub.Repo,
  prNum,
  meta.cfg.Repository.GitHub.MaxComments,
- git.RunGit,
+ headCommit,
  ); err != nil {
  return err
  }
- reps = append(reps, gr)
+ reps = append(reps, reporter.NewCommentReporter(gr))
  }
 
  minSeverity, err := checks.ParseSeverity(c.String(failOnFlag))

internal/reporter/comments.go

@@ -2,10 +2,12 @@ package reporter
 
 import (
  "context"
+ "log/slog"
  "slices"
  "strings"
 
  "github.com/cloudflare/pint/internal/checks"
+ "github.com/cloudflare/pint/internal/output"
 )
 
 type PendingCommentV2 struct {
@@ -30,9 +32,22 @@ type Commenter interface {
  Create(context.Context, any, PendingCommentV2) error
  Delete(context.Context, any, ExistingCommentV2) error
  CanCreate(int) (bool, error)
+ CanDelete(ExistingCommentV2) bool
  IsEqual(ExistingCommentV2, PendingCommentV2) bool
 }
 
+func NewCommentReporter(c Commenter) CommentReporter {
+ return CommentReporter{c: c}
+}
+
+type CommentReporter struct {
+ c Commenter
+}
+
+func (cr CommentReporter) Submit(summary Summary) (err error) {
+ return Submit(context.Background(), summary, cr.c)
+}
+
 func makeComments(summary Summary) (comments []PendingCommentV2) {
  var buf strings.Builder
  for _, reports := range dedupReports(summary.reports) {
@@ -155,3 +170,122 @@ func problemIcon(s checks.Severity) string {
  return ":stop_sign:"
  }
 }
+
+func errsToComment(errs []error) string {
+ var buf strings.Builder
+ buf.WriteString("There were some errors when pint was trying to create a report.\n")
+ buf.WriteString("Some review comments might be outdated or missing.\n")
+ buf.WriteString("List of all errors:\n\n")
+ for _, err := range errs {
+ buf.WriteString("- `")
+ buf.WriteString(err.Error())
+ buf.WriteString("`\n")
+ }
+ return buf.String()
+}
+
+func Submit(ctx context.Context, s Summary, c Commenter) error {
+ dsts, err := c.Destinations(ctx)
+ if err != nil {
+ return err
+ }
+
+ for _, dst := range dsts {
+ if err = updateDestination(ctx, s, c, dst); err != nil {
+ return err
+ }
+ }
+
+ return nil
+}
+
+func updateDestination(ctx context.Context, s Summary, c Commenter, dst any) (err error) {
+ slog.Info("Listing existing comments", slog.String("reporter", c.Describe()))
+ existingComments, err := c.List(ctx, dst)
+ if err != nil {
+ return err
+ }
+
+ var created int
+ var ok bool
+ var errs []error
+ pendingComments := makeComments(s)
+ for _, pending := range pendingComments {
+ for _, existing := range existingComments {
+ if c.IsEqual(existing, pending) {
+ slog.Debug("Comment already exists",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", pending.path),
+ slog.Int("line", pending.line),
+ )
+ goto NEXTCreate
+ }
+ }
+ slog.Debug("Comment doesn't exist yet",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", pending.path),
+ slog.Int("line", pending.line),
+ )
+
+ ok, err = c.CanCreate(created)
+ if err != nil {
+ errs = append(errs, err)
+ }
+ if !ok {
+ slog.Debug("Cannot create new comment",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", pending.path),
+ slog.Int("line", pending.line),
+ )
+ goto NEXTCreate
+ }
+
+ if err := c.Create(ctx, dst, pending); err != nil {
+ slog.Error("Failed to create a new comment",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", pending.path),
+ slog.Int("line", pending.line),
+ slog.Any("err", err),
+ )
+ return err
+ }
+ created++
+ NEXTCreate:
+ }
+
+ for _, existing := range existingComments {
+ for _, pending := range pendingComments {
+ if c.IsEqual(existing, pending) {
+ goto NEXTDelete
+ }
+ }
+ if !c.CanDelete(existing) {
+ goto NEXTDelete
+ }
+ if err := c.Delete(ctx, dst, existing); err != nil {
+ slog.Error("Failed to delete a stale comment",
+ slog.String("reporter", c.Describe()),
+ slog.String("path", existing.path),
+ slog.Int("line", existing.line),
+ slog.Any("err", err),
+ )
+ errs = append(errs, err)
+ }
+ NEXTDelete:
+ }
+
+ slog.Info("Creating report summary",
+ slog.String("reporter", c.Describe()),
+ slog.Int("reports", len(s.reports)),
+ slog.Int("online", int(s.OnlineChecks)),
+ slog.Int("offline", int(s.OnlineChecks)),
+ slog.String("duration", output.HumanizeDuration(s.Duration)),
+ slog.Int("entries", s.TotalEntries),
+ slog.Int("checked", int(s.CheckedEntries)),
+ )
+ if err := c.Summary(ctx, dst, s, errs); err != nil {
+ return err
+ }
+
+ return nil
+}

internal/reporter/comments_test.go

@@ -5,7 +5,11 @@ import (
  "errors"
  "fmt"
  "log/slog"
+ "net/http"
+ "net/http/httptest"
+ "strings"
  "testing"
+ "time"
 
  "github.com/neilotoole/slogt"
  "github.com/stretchr/testify/require"
@@ -29,6 +33,7 @@ type testCommenter struct {
  list func(context.Context, any) ([]ExistingCommentV2, error)
  create func(context.Context, any, PendingCommentV2) error
  delete func(context.Context, any, ExistingCommentV2) error
+ canDelete func(ExistingCommentV2) (bool, error)
  canCreate func(int) (bool, error)
  isEqual func(ExistingCommentV2, PendingCommentV2) bool
 }
@@ -57,6 +62,10 @@ func (tc testCommenter) Delete(ctx context.Context, dst any, comment ExistingCom
  return tc.delete(ctx, dst, comment)
 }
 
+func (tc testCommenter) CanDelete(ExistingCommentV2) bool {
+ return true
+}
+
 func (tc testCommenter) CanCreate(n int) (bool, error) {
  return tc.canCreate(n)
 }
@@ -769,3 +778,171 @@ foo details
  })
  }
 }
+
+func TestCommentsCommonPaths(t *testing.T) {
+ type errorCheck func(err error) error
+
+ type testCaseT struct {
+ httpHandler http.Handler
+ errorHandler errorCheck
+
+ description string
+ branch string
+ token string
+
+ reports []Report
+ timeout time.Duration
+ project int
+ maxComments int
+ }
+
+ p := parser.NewParser(false)
+ mockRules, _ := p.Parse([]byte(`
+- record: target is down
+ expr: up == 0
+- record: sum errors
+ expr: sum(errors) by (job)
+`))
+
+ fooReport := Report{
+ Path: discovery.Path{
+ SymlinkTarget: "foo.txt",
+ Name: "foo.txt",
+ },
+ ModifiedLines: []int{2},
+ Rule: mockRules[0],
+ Problem: checks.Problem{
+ Reporter: "foo",
+ Text: "foo error",
+ Details: "foo details",
+ Lines: parser.LineRange{First: 1, Last: 3},
+ Severity: checks.Fatal,
+ Anchor: checks.AnchorAfter,
+ },
+ }
+
+ testCases := []testCaseT{
+ {
+ description: "returns an error on non-200 HTTP response",
+ branch: "fakeBranch",
+ token: "fakeToken",
+ timeout: time.Second,
+ project: 123,
+ maxComments: 50,
+ reports: []Report{fooReport},
+ httpHandler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ w.WriteHeader(400)
+ _, _ = w.Write([]byte("Bad Request"))
+ }),
+ errorHandler: func(err error) error {
+ if err != nil {
+ return nil
+ }
+ return fmt.Errorf("wrong error: %w", err)
+ },
+ },
+ {
+ description: "returns an error on HTTP response timeout",
+ branch: "fakeBranch",
+ token: "fakeToken",
+ timeout: time.Second,
+ project: 123,
+ maxComments: 50,
+ reports: []Report{fooReport},
+ httpHandler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ time.Sleep(time.Second * 2)
+ w.WriteHeader(400)
+ _, _ = w.Write([]byte("Bad Request"))
+ }),
+ errorHandler: func(err error) error {
+ if err != nil && strings.HasSuffix(err.Error(), "context deadline exceeded") {
+ return nil
+ }
+ return fmt.Errorf("wrong error: %w", err)
+ },
+ },
+ {
+ description: "returns an error on non-json body",
+ branch: "fakeBranch",
+ token: "fakeToken",
+ timeout: time.Second,
+ project: 123,
+ maxComments: 50,
+ reports: []Report{fooReport},
+ httpHandler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ w.WriteHeader(200)
+ _, _ = w.Write([]byte("OK"))
+ }),
+ errorHandler: func(err error) error {
+ if err != nil {
+ return nil
+ }
+ return fmt.Errorf("wrong error: %w", err)
+ },
+ },
+ {
+ description: "returns an error on empty JSON body",
+ branch: "fakeBranch",
+ token: "fakeToken",
+ timeout: time.Second,
+ project: 123,
+ maxComments: 50,
+ reports: []Report{fooReport},
+ httpHandler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ w.WriteHeader(200)
+ _, _ = w.Write([]byte("{}"))
+ }),
+ errorHandler: func(err error) error {
+ if err != nil {
+ return nil
+ }
+ return fmt.Errorf("wrong error: %w", err)
+ },
+ },
+ }
+
+ for _, tc := range testCases {
+ for _, c := range []func(string) Commenter{
+ func(uri string) Commenter {
+ r, err := NewGitLabReporter(
+ "v0.0.0",
+ tc.branch,
+ uri,
+ tc.timeout,
+ tc.token,
+ tc.project,
+ tc.maxComments,
+ )
+ require.NoError(t, err, "can't create gitlab reporter")
+ return r
+ },
+ func(uri string) Commenter {
+ r, err := NewGithubReporter(
+ "v0.0.0",
+ uri,
+ uri,
+ tc.timeout,
+ tc.token,
+ "owner",
+ "repo",
+ 123,
+ tc.maxComments,
+ "fake-commit-id",
+ )
+ require.NoError(t, err, "can't create gitlab reporter")
+ return r
+ },
+ } {
+ t.Run(tc.description, func(t *testing.T) {
+ slog.SetDefault(slogt.New(t))
+
+ srv := httptest.NewServer(tc.httpHandler)
+ defer srv.Close()
+
+ summary := NewSummary(tc.reports)
+ err := Submit(context.Background(), summary, c(srv.URL))
+ require.NoError(t, tc.errorHandler(err))
+ })
+ }
+ }
+}