From 95481834e73ac9e1ed396966ea4b69d6124b5744 Mon Sep 17 00:00:00 2001 From: wwcchh0123 <1170142389@qq.com> Date: Mon, 9 Dec 2024 16:49:21 +0800 Subject: [PATCH] feat: githubcheckrun status is marked as failure when the linter run failed. --- internal/lint/lint.go | 6 ++--- internal/lint/provider.go | 2 +- internal/lint/providergithub.go | 23 +++++++++++-------- internal/lint/providergitlab.go | 2 +- internal/linters/doc/note-check/note.go | 2 +- internal/linters/go/gofmt/gofmt.go | 2 +- internal/linters/go/gomodcheck/gomodcheck.go | 2 +- .../linters/shell/shellcheck/shellcheck.go | 2 +- 8 files changed, 23 insertions(+), 18 deletions(-) diff --git a/internal/lint/lint.go b/internal/lint/lint.go index 269b6ccc..d6015778 100644 --- a/internal/lint/lint.go +++ b/internal/lint/lint.go @@ -134,7 +134,7 @@ func GeneralHandler(ctx context.Context, log *xlog.Logger, a Agent, execRun func } } - return Report(ctx, a, lintResults) + return Report(ctx, a, lintResults, unexpected) } // ExecRun executes a command. @@ -178,7 +178,7 @@ func GeneralParse(log *xlog.Logger, output []byte) (map[string][]LinterOutput, [ // Report reports the lint results. // This function should be always called even in custom linter handler since it will filter out the lint errors that are not related to the PR. // and handle some special cases like auto-generated files. -func Report(ctx context.Context, a Agent, lintResults map[string][]LinterOutput) error { +func Report(ctx context.Context, a Agent, lintResults map[string][]LinterOutput, unexpected []string) error { log := util.FromContext(ctx) var ( num = a.Provider.GetCodeReviewInfo().Number @@ -200,7 +200,7 @@ func Report(ctx context.Context, a Agent, lintResults map[string][]LinterOutput) metric.IncIssueCounter(orgRepo, linterName, a.Provider.GetCodeReviewInfo().URL, a.Provider.GetCodeReviewInfo().HeadSHA, float64(countLinterErrors(lintResults))) } - return a.Provider.Report(ctx, a, lintResults) + return a.Provider.Report(ctx, a, lintResults, unexpected) } // LineParser is a function that parses a line of linter output. diff --git a/internal/lint/provider.go b/internal/lint/provider.go index db39251b..7fed5361 100644 --- a/internal/lint/provider.go +++ b/internal/lint/provider.go @@ -17,7 +17,7 @@ type Provider interface { // Base on the linter outputs, the provider will create or update or delete the comments for the PR/MR. HandleComments(ctx context.Context, outputs map[string][]LinterOutput) error // Report reports the lint results to the provider. - Report(ctx context.Context, agent Agent, lintResults map[string][]LinterOutput) error + Report(ctx context.Context, agent Agent, lintResults map[string][]LinterOutput, unexpected []string) error // GetFiles returns the files that match the given predicate in the PR/MR. // if predicate is nil, it returns all the files except removed files in the PR/MR. // NOTE(CarlJi): this is a simplified definition since only the file path is returned. diff --git a/internal/lint/providergithub.go b/internal/lint/providergithub.go index 3296c96c..609cadd0 100644 --- a/internal/lint/providergithub.go +++ b/internal/lint/providergithub.go @@ -224,7 +224,7 @@ func filterLinterOutputs(outputs map[string][]LinterOutput, comments []*github.P return toAdds, toDeletes } -const Reference = "If you have any questions about this comment, feel free to [raise an issue here](https://github.com/qiniu/reviewbot)." +const Reference = "If you have any questions about this comment, feel free to [raise an issue here](https://github.com/qiniu/reviewbot) or contact the administrator." func toGithubCheckRunAnnotations(linterOutputs map[string][]LinterOutput) []*github.CheckRunAnnotation { var annotations []*github.CheckRunAnnotation @@ -321,7 +321,7 @@ func (g *GithubProvider) HandleComments(ctx context.Context, outputs map[string] return nil } -func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[string][]LinterOutput) error { +func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[string][]LinterOutput, unexpectResults []string) error { log := util.FromContext(ctx) linterName := a.LinterConfig.Name org := a.Provider.GetCodeReviewInfo().Org @@ -331,7 +331,7 @@ func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[st switch a.LinterConfig.ReportType { case config.GitHubCheckRuns: - check := newBaseCheckRun(a, lintResults) + check := newBaseCheckRun(a, lintResults, unexpectResults) ch, err := g.CreateCheckRun(ctx, org, repo, check) if err != nil { if !errors.Is(err, context.Canceled) { @@ -362,7 +362,7 @@ func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[st metric.NotifyWebhookByText(ConstructGotchaMsg(linterName, a.Provider.GetCodeReviewInfo().URL, addedCmts[0].GetHTMLURL(), lintResults)) case config.GitHubMixType: // report all lint results as a check run summary, but not annotations - check := newMixCheckRun(a, lintResults) + check := newMixCheckRun(a, lintResults, unexpectResults) ch, err := g.CreateCheckRun(ctx, org, repo, check) if err != nil { if !errors.Is(err, context.Canceled) { @@ -763,7 +763,7 @@ func (g *GithubProvider) refreshToken() (string, error) { } // newBaseCheckRun creates the base check run options. -func newBaseCheckRun(a Agent, lintErrs map[string][]LinterOutput) github.CreateCheckRunOptions { +func newBaseCheckRun(a Agent, lintErrs map[string][]LinterOutput, unexpected []string) github.CreateCheckRunOptions { var ( headSha = a.Provider.GetCodeReviewInfo().HeadSHA startTime = a.Provider.GetCodeReviewInfo().UpdatedAt @@ -800,17 +800,22 @@ func newBaseCheckRun(a Agent, lintErrs map[string][]LinterOutput) github.CreateC } if len(annotations) > 0 { - check.Conclusion = github.String("failure") + check.Conclusion = github.String("neutral") } else { check.Conclusion = github.String("success") } + if len(unexpected) > 0 { + check.Conclusion = github.String("failure") + check.Output.Title = github.String(fmt.Sprintf("%s failed to execute, please check it", linterName)) + check.Output.Summary = github.String(fmt.Sprintf(":information_source: %s's execution scripts and output are in the [log](%s).\n :recycle: You can check to see if the script is working in your repository.\n :busts_in_silhouette: If it's missing something critical, contact reviewbot administrator to add it.", linterName, logURL)) + } return check } -func newMixCheckRun(a Agent, lintErrs map[string][]LinterOutput) github.CreateCheckRunOptions { - check := newBaseCheckRun(a, lintErrs) - if len(lintErrs) == 0 { +func newMixCheckRun(a Agent, lintErrs map[string][]LinterOutput, unexpected []string) github.CreateCheckRunOptions { + check := newBaseCheckRun(a, lintErrs, unexpected) + if len(lintErrs) == 0 && len(unexpected) != 0 { // if no lint errors, just return the base check run return check } diff --git a/internal/lint/providergitlab.go b/internal/lint/providergitlab.go index 3177feba..33f798c9 100644 --- a/internal/lint/providergitlab.go +++ b/internal/lint/providergitlab.go @@ -331,7 +331,7 @@ func linterNamePrefixGitLab(linterName string) string { return fmt.Sprintf("[**%s**]", linterName) } -func (g *GitlabProvider) Report(ctx context.Context, a Agent, lintResults map[string][]LinterOutput) error { +func (g *GitlabProvider) Report(ctx context.Context, a Agent, lintResults map[string][]LinterOutput, unexpected []string) error { linterName := a.LinterConfig.Name org := a.Provider.GetCodeReviewInfo().Org repo := a.Provider.GetCodeReviewInfo().Repo diff --git a/internal/linters/doc/note-check/note.go b/internal/linters/doc/note-check/note.go index 19e5c38b..4fa3917c 100644 --- a/internal/linters/doc/note-check/note.go +++ b/internal/linters/doc/note-check/note.go @@ -67,7 +67,7 @@ func noteCheckHandler(ctx context.Context, a lint.Agent) error { } } - return lint.Report(ctx, a, outputs) + return lint.Report(ctx, a, outputs, nil) } const NoteSuggestion = "A Note is recommended to use \"MARKER(uid): note body\" format." diff --git a/internal/linters/go/gofmt/gofmt.go b/internal/linters/go/gofmt/gofmt.go index 014e90db..c2536b42 100644 --- a/internal/linters/go/gofmt/gofmt.go +++ b/internal/linters/go/gofmt/gofmt.go @@ -66,7 +66,7 @@ func gofmtHandler(ctx context.Context, a lint.Agent) error { return err } - return lint.Report(ctx, a, parsedOutput) + return lint.Report(ctx, a, parsedOutput, nil) } type Gofmt struct { diff --git a/internal/linters/go/gomodcheck/gomodcheck.go b/internal/linters/go/gomodcheck/gomodcheck.go index 42305ec0..824b8c41 100644 --- a/internal/linters/go/gomodcheck/gomodcheck.go +++ b/internal/linters/go/gomodcheck/gomodcheck.go @@ -42,7 +42,7 @@ func goModCheckHandler(ctx context.Context, a lint.Agent) error { log.Errorf("gomodchecks parse output failed: %v", err) return err } - return lint.Report(ctx, a, parsedOutput) + return lint.Report(ctx, a, parsedOutput, nil) } func goModCheckOutput(log *xlog.Logger, a lint.Agent) (map[string][]lint.LinterOutput, error) { diff --git a/internal/linters/shell/shellcheck/shellcheck.go b/internal/linters/shell/shellcheck/shellcheck.go index 7ab82c30..ee5200ca 100644 --- a/internal/linters/shell/shellcheck/shellcheck.go +++ b/internal/linters/shell/shellcheck/shellcheck.go @@ -71,5 +71,5 @@ func shellcheck(ctx context.Context, a lint.Agent) error { // even if the lintResults is empty, we still need to report the result // since we need delete the existed comments related to the linter - return lint.Report(ctx, a, lintResults) + return lint.Report(ctx, a, lintResults, nil) }