Skip to content

Commit

Permalink
Support more than 50 annotations
Browse files Browse the repository at this point in the history 
Fix #147
  • Loading branch information
@haya14busa
haya14busa committed Sep 28, 2019
1 parent 9759395 commit 4e78a95
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 34 deletions.
72 changes: 60 additions & 12 deletions doghouse/server/doghouse.go
View file
Expand Up @@ -22,6 +22,11 @@ import (
// Only 65535 characters are allowed; 250684 were supplied. []
const maxFilteredFinding = 150

// > The Checks API limits the number of annotations to a maximum of 50 per API
// > request.
// https://developer.github.com/v3/checks/runs/#output-object
const maxAnnotationsPerRequest = 50

type Checker struct {
req *doghouse.CheckRequest
gh checkerGitHubClientInterface
Expand All @@ -39,7 +44,8 @@ func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {

results := annotationsToCheckResults(ch.req.Annotations)
filtered := reviewdog.FilterCheck(results, filediffs, 1, "")
checkRun, err := ch.postCheck(ctx, filtered)

check, err := ch.createCheck(ctx)
if err != nil {
// If this error is StatusForbidden (403) here, it means reviewdog is
// running on GitHub Actions and has only read permission (because it's
Expand All @@ -50,6 +56,11 @@ func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {
if err, ok := err.(*github.ErrorResponse); ok && err.Response.StatusCode == http.StatusForbidden {
return &doghouse.CheckResponse{CheckedResults: filtered}, nil
}
return nil, fmt.Errorf("failed to create check: %v", err)
}

checkRun, err := ch.postCheck(ctx, check.GetID(), filtered)
if err != nil {
return nil, fmt.Errorf("failed to post result: %v", err)
}
res := &doghouse.CheckResponse{
Expand All @@ -58,40 +69,70 @@ func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {
return res, nil
}

func (ch *Checker) postCheck(ctx context.Context, checks []*reviewdog.FilteredCheck) (*github.CheckRun, error) {
func (ch *Checker) postCheck(ctx context.Context, checkID int64, checks []*reviewdog.FilteredCheck) (*github.CheckRun, error) {
var annotations []*github.CheckRunAnnotation
for _, c := range checks {
if !c.InDiff {
continue
}
annotations = append(annotations, ch.toCheckRunAnnotation(c))
}
if err := ch.postAnnotations(ctx, checkID, annotations); err != nil {
return nil, fmt.Errorf("failed to post annotations: %v", err)
}

conclusion := "success"
if len(annotations) > 0 {
conclusion = ch.conclusion()
}
name := "reviewdog"
name := ch.checkName()
title := "reviewdog report"
if ch.req.Name != "" {
name = ch.req.Name
if name != "reviewdog" {
title = fmt.Sprintf("reviewdog [%s] report", name)
}
opt := github.CreateCheckRunOptions{
Name: name,
ExternalID: github.String(name),
HeadSHA: ch.req.SHA,
opt := github.UpdateCheckRunOptions{
Status: github.String("completed"),
Conclusion: github.String(conclusion),
CompletedAt: &github.Timestamp{Time: time.Now()},
Output: &github.CheckRunOutput{
Title: github.String(title),
Summary: github.String(ch.summary(checks)),
Annotations: annotations,
Title: github.String(title),
Summary: github.String(ch.summary(checks)),
},
}
return ch.gh.UpdateCheckRun(ctx, ch.req.Owner, ch.req.Repo, checkID, opt)
}

func (ch *Checker) createCheck(ctx context.Context) (*github.CheckRun, error) {
opt := github.CreateCheckRunOptions{
Name: ch.checkName(),
HeadSHA: ch.req.SHA,
Status: github.String("in_progress"),
}
return ch.gh.CreateCheckRun(ctx, ch.req.Owner, ch.req.Repo, opt)
}

func (ch *Checker) postAnnotations(ctx context.Context, checkID int64, annotations []*github.CheckRunAnnotation) error {
opt := github.UpdateCheckRunOptions{
Output: &github.CheckRunOutput{
Annotations: annotations[:min(maxAnnotationsPerRequest, len(annotations))],
},
}
if _, err := ch.gh.UpdateCheckRun(ctx, ch.req.Owner, ch.req.Repo, checkID, opt); err != nil {
return err
}
if len(annotations) > maxAnnotationsPerRequest {
return ch.postAnnotations(ctx, checkID, annotations[maxAnnotationsPerRequest:])
}
return nil
}

func (ch *Checker) checkName() string {
if ch.req.Name != "" {
return ch.req.Name
}
return "reviewdog"
}

// https://developer.github.com/v3/checks/runs/#parameters-1
func (ch *Checker) conclusion() string {
switch strings.ToLower(ch.req.Level) {
Expand Down Expand Up @@ -218,3 +259,10 @@ func annotationsToCheckResults(as []*doghouse.Annotation) []*reviewdog.CheckResu
}
return cs
}

func min(x, y int) int {
if x > y {
return y
}
return x
}
123 changes: 101 additions & 22 deletions doghouse/server/doghouse_test.go
View file
Expand Up @@ -15,6 +15,7 @@ type fakeCheckerGitHubCli struct {
checkerGitHubClientInterface
FakeGetPullRequestDiff func(ctx context.Context, owner, repo string, number int) ([]byte, error)
FakeCreateCheckRun func(ctx context.Context, owner, repo string, opt github.CreateCheckRunOptions) (*github.CheckRun, error)
FakeUpdateCheckRun func(ctx context.Context, owner, repo string, checkID int64, opt github.UpdateCheckRunOptions) (*github.CheckRun, error)
}

func (f *fakeCheckerGitHubCli) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) ([]byte, error) {
Expand All @@ -25,6 +26,10 @@ func (f *fakeCheckerGitHubCli) CreateCheckRun(ctx context.Context, owner, repo s
return f.FakeCreateCheckRun(ctx, owner, repo, opt)
}

func (f *fakeCheckerGitHubCli) UpdateCheckRun(ctx context.Context, owner, repo string, checkID int64, opt github.UpdateCheckRunOptions) (*github.CheckRun, error) {
return f.FakeUpdateCheckRun(ctx, owner, repo, checkID, opt)
}

const sampleDiff = `--- sample.old.txt 2016-10-13 05:09:35.820791185 +0900
+++ sample.new.txt 2016-10-13 05:15:26.839245048 +0900
@@ -1,3 +1,4 @@
Expand All @@ -48,13 +53,14 @@ const sampleDiff = `--- sample.old.txt 2016-10-13 05:09:35.820791185 +0900

func TestCheck_OK(t *testing.T) {
const (
name = "haya14busa-linter"
owner = "haya14busa"
repo = "reviewdog"
prNum = 14
sha = "1414"
reportURL = "http://example.com/report_url"
conclusion = "neutral"
name = "haya14busa-linter"
owner = "haya14busa"
repo = "reviewdog"
prNum = 14
sha = "1414"
reportURL = "http://example.com/report_url"
conclusion = "neutral"
wantCheckID = 1414
)

req := &doghouse.CheckRequest{
Expand Down Expand Up @@ -91,23 +97,32 @@ func TestCheck_OK(t *testing.T) {
if opt.HeadSHA != sha {
t.Errorf("CreateCheckRunOptions.HeadSHA = %q, want %q", opt.HeadSHA, sha)
}
if *opt.Conclusion != conclusion {
t.Errorf("CreateCheckRunOptions.Conclusion = %q, want %q", *opt.Conclusion, conclusion)
return &github.CheckRun{ID: github.Int64(wantCheckID)}, nil
}
cli.FakeUpdateCheckRun = func(ctx context.Context, owner, repo string, checkID int64, opt github.UpdateCheckRunOptions) (*github.CheckRun, error) {
if checkID != wantCheckID {
t.Errorf("UpdateCheckRun: checkID = %d, want %d", checkID, wantCheckID)
}
annotations := opt.Output.Annotations
wantAnnotaions := []*github.CheckRunAnnotation{
{
Path: github.String("sample.new.txt"),
StartLine: github.Int(2),
EndLine: github.Int(2),
AnnotationLevel: github.String("warning"),
Message: github.String("test message"),
Title: github.String("[haya14busa-linter] sample.new.txt#L2"),
RawDetails: github.String("raw test message"),
},
}
if d := cmp.Diff(annotations, wantAnnotaions); d != "" {
t.Errorf("Annotation diff found:\n%s", d)
if len(annotations) == 0 {
if *opt.Conclusion != conclusion {
t.Errorf("UpdateCheckRunOptions.Conclusion = %q, want %q", *opt.Conclusion, conclusion)
}
} else {
wantAnnotaions := []*github.CheckRunAnnotation{
{
Path: github.String("sample.new.txt"),
StartLine: github.Int(2),
EndLine: github.Int(2),
AnnotationLevel: github.String("warning"),
Message: github.String("test message"),
Title: github.String("[haya14busa-linter] sample.new.txt#L2"),
RawDetails: github.String("raw test message"),
},
}
if d := cmp.Diff(annotations, wantAnnotaions); d != "" {
t.Errorf("Annotation diff found:\n%s", d)
}
}
return &github.CheckRun{HTMLURL: github.String(reportURL)}, nil
}
Expand All @@ -122,6 +137,70 @@ func TestCheck_OK(t *testing.T) {
}
}

func TestCheck_OK_multiple_update_runs(t *testing.T) {
const (
name = "haya14busa-linter"
owner = "haya14busa"
repo = "reviewdog"
prNum = 14
sha = "1414"
reportURL = "http://example.com/report_url"
conclusion = "neutral"
wantCheckID = 1414
)

req := &doghouse.CheckRequest{
Name: name,
Owner: owner,
Repo: repo,
PullRequest: prNum,
SHA: sha,
Level: "warning",
}
for i := 0; i < 101; i++ {
req.Annotations = append(req.Annotations, &doghouse.Annotation{
Path: "sample.new.txt",
Line: 2,
Message: "test message",
RawMessage: "raw test message",
})
}

cli := &fakeCheckerGitHubCli{}
cli.FakeGetPullRequestDiff = func(ctx context.Context, owner, repo string, number int) ([]byte, error) {
return []byte(sampleDiff), nil
}
cli.FakeCreateCheckRun = func(ctx context.Context, owner, repo string, opt github.CreateCheckRunOptions) (*github.CheckRun, error) {
if opt.Name != name {
t.Errorf("CreateCheckRunOptions.Name = %q, want %q", opt.Name, name)
}
if opt.HeadSHA != sha {
t.Errorf("CreateCheckRunOptions.HeadSHA = %q, want %q", opt.HeadSHA, sha)
}
return &github.CheckRun{ID: github.Int64(wantCheckID)}, nil
}
cli.FakeUpdateCheckRun = func(ctx context.Context, owner, repo string, checkID int64, opt github.UpdateCheckRunOptions) (*github.CheckRun, error) {
if checkID != wantCheckID {
t.Errorf("UpdateCheckRun: checkID = %d, want %d", checkID, wantCheckID)
}
annotations := opt.Output.Annotations
switch len(annotations) {
case 0:
if *opt.Conclusion != conclusion {
t.Errorf("UpdateCheckRunOptions.Conclusion = %q, want %q", *opt.Conclusion, conclusion)
}
case maxAnnotationsPerRequest, 1: // Expected
default:
t.Errorf("UpdateCheckRun: len(annotations) = %d, but it's unexpected", len(annotations))
}
return &github.CheckRun{HTMLURL: github.String(reportURL)}, nil
}
checker := &Checker{req: req, gh: cli}
if _, err := checker.Check(context.Background()); err != nil {
t.Fatal(err)
}
}

func TestCheck_fail_diff(t *testing.T) {
req := &doghouse.CheckRequest{}
cli := &fakeCheckerGitHubCli{}
Expand Down
6 changes: 6 additions & 0 deletions doghouse/server/github_checker.go
View file
Expand Up @@ -9,6 +9,7 @@ import (
type checkerGitHubClientInterface interface {
GetPullRequestDiff(ctx context.Context, owner, repo string, number int) ([]byte, error)
CreateCheckRun(ctx context.Context, owner, repo string, opt github.CreateCheckRunOptions) (*github.CheckRun, error)
UpdateCheckRun(ctx context.Context, owner, repo string, checkID int64, opt github.UpdateCheckRunOptions) (*github.CheckRun, error)
}

type checkerGitHubClient struct {
Expand All @@ -25,3 +26,8 @@ func (c *checkerGitHubClient) CreateCheckRun(ctx context.Context, owner, repo st
checkRun, _, err := c.Checks.CreateCheckRun(ctx, owner, repo, opt)
return checkRun, err
}

func (c *checkerGitHubClient) UpdateCheckRun(ctx context.Context, owner, repo string, checkID int64, opt github.UpdateCheckRunOptions) (*github.CheckRun, error) {
checkRun, _, err := c.Checks.UpdateCheckRun(ctx, owner, repo, checkID, opt)
return checkRun, err
}

0 comments on commit 4e78a95

Please sign in to comment.