diff --git a/cmd/pint/tests/0032_ci_github.txt b/cmd/pint/tests/0032_ci_github.txt index b57e05c8..decac70a 100644 --- a/cmd/pint/tests/0032_ci_github.txt +++ b/cmd/pint/tests/0032_ci_github.txt @@ -1,3 +1,4 @@ +http method github GET /api/v3/repos/cloudflare/pint/pulls/1/files 200 [{"filename":"rules.yml"}] http method github GET /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 [] http method github POST /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 {} http method github GET /api/v3/repos/cloudflare/pint/pulls/1/comments 200 [] diff --git a/cmd/pint/tests/0033_ci_github_multi.txt b/cmd/pint/tests/0033_ci_github_multi.txt index 92d0313d..997c1e35 100644 --- a/cmd/pint/tests/0033_ci_github_multi.txt +++ b/cmd/pint/tests/0033_ci_github_multi.txt @@ -1,3 +1,4 @@ +http method github GET /api/v3/repos/cloudflare/pint/pulls/1/files 200 [{"filename":"rules.yml"}] http method github GET /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 [] http method github POST /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 {} http method github GET /api/v3/repos/cloudflare/pint/pulls/1/comments 200 [] diff --git a/cmd/pint/tests/0083_github_action.txt b/cmd/pint/tests/0083_github_action.txt index 3b7be0c3..a239a956 100644 --- a/cmd/pint/tests/0083_github_action.txt +++ b/cmd/pint/tests/0083_github_action.txt @@ -1,3 +1,4 @@ +http method github GET /api/v3/repos/foo/bar/pulls/123/files 200 [{"filename":"rules.yml"}] http method github GET /api/v3/repos/foo/bar/pulls/123/reviews 200 [] http method github POST /api/v3/repos/foo/bar/pulls/123/reviews 200 {} http method github GET /api/v3/repos/foo/bar/pulls/123/comments 200 [] diff --git a/cmd/pint/tests/0084_github_action_override.txt b/cmd/pint/tests/0084_github_action_override.txt index cb815ecc..2a92e090 100644 --- a/cmd/pint/tests/0084_github_action_override.txt +++ b/cmd/pint/tests/0084_github_action_override.txt @@ -1,3 +1,4 @@ +http method github GET /api/v3/repos/cloudflare/pint/pulls/123/files 200 [{"filename":"rules.yml"}] http method github GET /api/v3/repos/cloudflare/pint/pulls/123/reviews 200 [] http method github POST /api/v3/repos/cloudflare/pint/pulls/123/reviews 200 {} http method github GET /api/v3/repos/cloudflare/pint/pulls/123/comments 200 [] diff --git a/cmd/pint/tests/0085_github_no_envs.txt b/cmd/pint/tests/0085_github_no_envs.txt index cb08aa4a..fcb1f66e 100644 --- a/cmd/pint/tests/0085_github_no_envs.txt +++ b/cmd/pint/tests/0085_github_no_envs.txt @@ -1,3 +1,4 @@ +http method github GET /api/v3/repos/cloudflare/pint/pulls/123/files 200 [{"filename":"rules.yml"}] http method github GET /api/v3/repos/cloudflare/pint/pulls/123/reviews 200 [] http method github POST /api/v3/repos/cloudflare/pint/pulls/123/reviews 200 {} http method github GET /api/v3/repos/cloudflare/pint/pulls/123/comments 200 [] diff --git a/cmd/pint/tests/0098_rule_file_symlink_gh.txt b/cmd/pint/tests/0098_rule_file_symlink_gh.txt index 1827196a..e17d9c8e 100644 --- a/cmd/pint/tests/0098_rule_file_symlink_gh.txt +++ b/cmd/pint/tests/0098_rule_file_symlink_gh.txt @@ -1,3 +1,4 @@ +http method github GET /api/v3/repos/cloudflare/pint/pulls/1/files 200 [{"filename":"rules.yml"}] http method github GET /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 [] http method github POST /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 {} http method github GET /api/v3/repos/cloudflare/pint/pulls/1/comments 200 [] diff --git a/cmd/pint/tests/0118_ci_dir_move.txt b/cmd/pint/tests/0118_ci_dir_move.txt index aadc9d04..0c7638f1 100644 --- a/cmd/pint/tests/0118_ci_dir_move.txt +++ b/cmd/pint/tests/0118_ci_dir_move.txt @@ -1,3 +1,4 @@ +http method github GET /api/v3/repos/cloudflare/pint/pulls/1/files 200 [{"filename":"rules.yml"}] http method github GET /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 [] http method github POST /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 {} http method github GET /api/v3/repos/cloudflare/pint/pulls/1/comments 200 [] diff --git a/cmd/pint/tests/0119_ci_fail_on_warning.txt b/cmd/pint/tests/0119_ci_fail_on_warning.txt index e029c8d6..7de0dd98 100644 --- a/cmd/pint/tests/0119_ci_fail_on_warning.txt +++ b/cmd/pint/tests/0119_ci_fail_on_warning.txt @@ -1,3 +1,4 @@ +http method github GET /api/v3/repos/cloudflare/pint/pulls/1/files 200 [{"filename":"rules.yml"}] http method github GET /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 [] http method github POST /api/v3/repos/cloudflare/pint/pulls/1/reviews 200 {} http method github GET /api/v3/repos/cloudflare/pint/pulls/1/comments 200 [] diff --git a/docs/changelog.md b/docs/changelog.md index 62ade3fb..29f495d9 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -9,6 +9,10 @@ - [promql/range_query](checks/promql/range_query.md) now allows to configure a custom maximum duration for range queries - #1064. +### Fixed + +- Don't try to report problem on unmodified files when using GitHub reporter. + ## v0.64.1 ### Fixed diff --git a/internal/reporter/github.go b/internal/reporter/github.go index fde3dbdb..b80545cd 100644 --- a/internal/reporter/github.go +++ b/internal/reporter/github.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "log/slog" + "slices" "strconv" "strings" "time" @@ -37,6 +38,10 @@ type ghCommentMeta struct { id int64 } +type ghPR struct { + paths []string +} + // NewGithubReporter creates a new GitHub reporter that reports // problems via comments on a given pull request number (integer). func NewGithubReporter(version, baseURL, uploadURL string, timeout time.Duration, token, owner, repo string, prNum, maxComments int, headCommit string) (_ GithubReporter, err error) { @@ -84,8 +89,10 @@ func (gr GithubReporter) Describe() string { return "GitHub" } -func (gr GithubReporter) Destinations(context.Context) ([]any, error) { - return []any{gr.prNum}, nil +func (gr GithubReporter) Destinations(ctx context.Context) (_ []any, err error) { + pr := ghPR{} + pr.paths, err = gr.listPRFiles(ctx) + return []any{pr}, err } func (gr GithubReporter) Summary(ctx context.Context, _ any, s Summary, errs []error) error { @@ -144,7 +151,16 @@ func (gr GithubReporter) List(ctx context.Context, _ any) ([]ExistingComment, er return comments, nil } -func (gr GithubReporter) Create(ctx context.Context, _ any, p PendingComment) error { +func (gr GithubReporter) Create(ctx context.Context, dst any, p PendingComment) error { + pr := dst.(ghPR) + + if !slices.Contains(pr.paths, p.path) { + slog.Debug("Skipping report for path with no changes", + slog.String("path", p.path), + ) + return nil + } + var side string if p.anchor == checks.AnchorBefore { side = "LEFT" @@ -250,6 +266,22 @@ func (gr GithubReporter) createReview(ctx context.Context, summary Summary) erro return nil } +func (gr GithubReporter) listPRFiles(ctx context.Context) ([]string, error) { + reqCtx, cancel := gr.reqContext(ctx) + defer cancel() + + slog.Debug("Getting the list of modified files", slog.Int("pr", gr.prNum)) + files, _, err := gr.client.PullRequests.ListFiles(reqCtx, gr.owner, gr.repo, gr.prNum, nil) + if err != nil { + return nil, fmt.Errorf("failed to list pull request files: %w", err) + } + paths := []string{} + for _, file := range files { + paths = append(paths, file.GetFilename()) + } + return paths, nil +} + func formatGHReviewBody(version string, summary Summary) string { var b strings.Builder diff --git a/internal/reporter/github_test.go b/internal/reporter/github_test.go index 4c15ae3f..6be1590d 100644 --- a/internal/reporter/github_test.go +++ b/internal/reporter/github_test.go @@ -46,13 +46,59 @@ func TestGithubReporter(t *testing.T) { for _, tc := range []testCaseT{ { - description: "list pull requests timeout", + description: "list files error", owner: "foo", repo: "bar", token: "something", prNum: 123, maxComments: 50, - httpHandler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + timeout: time.Second, + httpHandler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/api/v3/repos/foo/bar/pulls/123/files" { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte("Error")) + return + } + _, _ = w.Write([]byte("")) + }), + error: func(uri string) string { + return fmt.Sprintf("failed to list pull request files: GET %s/api/v3/repos/foo/bar/pulls/123/files: 400 []", uri) + }, + reports: []reporter.Report{ + { + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + + ModifiedLines: []int{2}, + Rule: mockRules[1], + Problem: checks.Problem{ + Lines: parser.LineRange{ + First: 2, + Last: 2, + }, + Reporter: "mock", + Text: "syntax error", + Details: "syntax details", + Severity: checks.Fatal, + }, + }, + }, + }, + { + description: "list pull reviews timeout", + owner: "foo", + repo: "bar", + token: "something", + prNum: 123, + maxComments: 50, + httpHandler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/api/v3/repos/foo/bar/pulls/123/files" { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("[]")) + return + } time.Sleep(1 * time.Second) _, _ = w.Write([]byte("OK")) }), @@ -514,6 +560,11 @@ func TestGithubReporter(t *testing.T) { maxComments: 2, timeout: time.Second, httpHandler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/api/v3/repos/foo/bar/pulls/123/files" { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`[{"filename":"foo.txt"}]`)) + return + } if r.Method == http.MethodGet && r.URL.Path == "/api/v3/repos/foo/bar/pulls/123/reviews" { _, _ = w.Write([]byte(`[{"id":1,"body":"### This pull request was validated by [pint](https://github.com/cloudflare/pint).\nxxxx"}]`)) return