From 4f6d49e14290bba23856cdca8e2d240da8c708d2 Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Thu, 22 Aug 2024 13:46:35 +0100 Subject: [PATCH] Don't try to report problems on unmodified files --- cmd/pint/tests/0032_ci_github.txt | 1 + cmd/pint/tests/0033_ci_github_multi.txt | 1 + cmd/pint/tests/0083_github_action.txt | 1 + .../tests/0084_github_action_override.txt | 1 + cmd/pint/tests/0085_github_no_envs.txt | 1 + cmd/pint/tests/0098_rule_file_symlink_gh.txt | 1 + cmd/pint/tests/0118_ci_dir_move.txt | 1 + cmd/pint/tests/0119_ci_fail_on_warning.txt | 1 + docs/changelog.md | 1 + internal/reporter/github.go | 38 +++++++++++++++++-- internal/reporter/github_test.go | 2 +- 11 files changed, 45 insertions(+), 4 deletions(-) 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 39cb2a67..9ad2ff45 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 4232dd1c..2cb31706 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 2e45ba74..c7ea845a 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 71f4398d..30747427 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 1710c9da..44cbfedb 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -15,6 +15,7 @@ - If there is a pint config file present then pint will now always add it to the `parser` block `exclude` list. This is to avoid trying to parse it as a rule file if it's included in the same folder as rules. +- Don't try to report problem on unmodified files when using GitHub reporter. ## v0.64.1 diff --git a/internal/reporter/github.go b/internal/reporter/github.go index 7762dcb1..79d900dc 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" @@ -261,6 +277,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..5f9f9aca 100644 --- a/internal/reporter/github_test.go +++ b/internal/reporter/github_test.go @@ -58,7 +58,7 @@ func TestGithubReporter(t *testing.T) { }), timeout: 100 * time.Millisecond, error: func(_ string) string { - return "failed to list pull request reviews: context deadline exceeded" + return "failed to list pull request files: context deadline exceeded" }, reports: []reporter.Report{ {