Skip to content

Commit

Permalink
Implement new reporter: github-check
Browse files Browse the repository at this point in the history 
Fix #332
  • Loading branch information
@haya14busa
haya14busa committed Sep 29, 2019
1 parent 81f21e9 commit 2d7e23e
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 21 deletions.
4 changes: 2 additions & 2 deletions cmd/reviewdog/doghouse.go
View file
Expand Up @@ -20,12 +20,12 @@ import (
"golang.org/x/sync/errgroup"
)

func runDoghouse(ctx context.Context, r io.Reader, w io.Writer, opt *option, isProject bool) error {
func runDoghouse(ctx context.Context, r io.Reader, w io.Writer, opt *option, isProject bool, allowNonPR bool) error {
ghInfo, isPr, err := cienv.GetBuildInfo()
if err != nil {
return err
}
if !isPr {
if !isPr && !allowNonPR {
fmt.Fprintln(os.Stderr, "reviewdog: this is not PullRequest build.")
return nil
}
Expand Down
17 changes: 13 additions & 4 deletions cmd/reviewdog/main.go
View file
Expand Up @@ -67,12 +67,15 @@ const (
runnersDoc = `comma separated runners name to run in config file. default: run all runners`
levelDoc = `report level currently used for github-pr-check reporter ("info","warning","error").`
guessPullRequestDoc = `guess Pull Request ID by branch name and commit SHA`
reporterDoc = `reporter of reviewdog results. (local, github-pr-check, github-pr-review, gitlab-mr-discussion, gitlab-mr-commit)
reporterDoc = `reporter of reviewdog results. (local, github-check, github-pr-check, github-pr-review, gitlab-mr-discussion, gitlab-mr-commit)
"local" (default)
Report results to stdout.
"github-pr-check" (experimental)
Report results to GitHub PullRequest Check tab.
"github-check" (experimental)
Report results to GitHub Check. It works both for Pull Requests and commits
and this reporter reports all results regardless of it's new result or not.
You can see report results in GitHub PullRequest Check tab for Pull Request.
There are two options to use this reporter.
Option 1) Run reviewdog from GitHub Actions w/ secrets.GITHUB_TOKEN
Expand All @@ -91,6 +94,10 @@ const (
Note: Token is not required if you run reviewdog in Travis CI.
"github-pr-check" (experimental)
Same as github-check reporter but it only supports Pull Requests and
reports only new results which is in diff.
"github-pr-review"
Report results to GitHub review comments.
Expand Down Expand Up @@ -198,8 +205,10 @@ See -reporter flag for migration and set -reporter="github-pr-review" or -report
switch opt.reporter {
default:
return fmt.Errorf("unknown -reporter: %s", opt.reporter)
case "github-check":
return runDoghouse(ctx, r, w, opt, isProject, true)
case "github-pr-check":
return runDoghouse(ctx, r, w, opt, isProject)
return runDoghouse(ctx, r, w, opt, isProject, false)
case "github-pr-review":
if os.Getenv("REVIEWDOG_GITHUB_API_TOKEN") == "" {
fmt.Fprintln(os.Stderr, "REVIEWDOG_GITHUB_API_TOKEN is not set")
Expand Down
27 changes: 17 additions & 10 deletions doghouse/server/doghouse.go
View file
Expand Up @@ -38,9 +38,16 @@ func NewChecker(req *doghouse.CheckRequest, gh *github.Client) *Checker {
}

func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {
filediffs, err := ch.diff(ctx)
if err != nil {
return nil, fmt.Errorf("fail to parse diff: %v", err)

filterByDiff := ch.req.PullRequest != 0

var filediffs []*diff.FileDiff
if ch.req.PullRequest != 0 {
var err error
filediffs, err = ch.pullRequestDiff(ctx, ch.req.PullRequest)
if err != nil {
return nil, fmt.Errorf("fail to parse diff: %v", err)
}
}

results := annotationsToCheckResults(ch.req.Annotations)
Expand All @@ -60,7 +67,7 @@ func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {
return nil, fmt.Errorf("failed to create check: %v", err)
}

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

func (ch *Checker) postCheck(ctx context.Context, checkID int64, checks []*reviewdog.FilteredCheck) (*github.CheckRun, error) {
func (ch *Checker) postCheck(ctx context.Context, checkID int64, checks []*reviewdog.FilteredCheck, filterByDiff bool) (*github.CheckRun, error) {
var annotations []*github.CheckRunAnnotation
for _, c := range checks {
if !c.InDiff {
if !c.InDiff && filterByDiff {
continue
}
annotations = append(annotations, ch.toCheckRunAnnotation(c))
Expand Down Expand Up @@ -215,8 +222,8 @@ func (ch *Checker) toCheckRunAnnotation(c *reviewdog.FilteredCheck) *github.Chec
return a
}

func (ch *Checker) diff(ctx context.Context) ([]*diff.FileDiff, error) {
d, err := ch.rawDiff(ctx)
func (ch *Checker) pullRequestDiff(ctx context.Context, pr int) ([]*diff.FileDiff, error) {
d, err := ch.rawPullRequestDiff(ctx, pr)
if err != nil {
return nil, err
}
Expand All @@ -227,8 +234,8 @@ func (ch *Checker) diff(ctx context.Context) ([]*diff.FileDiff, error) {
return filediffs, nil
}

func (ch *Checker) rawDiff(ctx context.Context) ([]byte, error) {
d, err := ch.gh.GetPullRequestDiff(ctx, ch.req.Owner, ch.req.Repo, ch.req.PullRequest)
func (ch *Checker) rawPullRequestDiff(ctx context.Context, pr int) ([]byte, error) {
d, err := ch.gh.GetPullRequestDiff(ctx, ch.req.Owner, ch.req.Repo, pr)
if err != nil {
return nil, err
}
Expand Down
97 changes: 93 additions & 4 deletions doghouse/server/doghouse_test.go
View file
Expand Up @@ -204,8 +204,97 @@ func TestCheck_OK_multiple_update_runs(t *testing.T) {
}
}

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

req := &doghouse.CheckRequest{
// Do not set PullRequest
Name: name,
Owner: owner,
Repo: repo,
SHA: sha,
Annotations: []*doghouse.Annotation{
{
Path: "sample.new.txt",
Line: 2,
Message: "test message",
RawMessage: "raw test message",
},
{
Path: "sample.new.txt",
Line: 14,
Message: "test message2",
RawMessage: "raw test message2",
},
},
Level: "warning",
}

cli := &fakeCheckerGitHubCli{}
cli.FakeGetPullRequestDiff = func(ctx context.Context, owner, repo string, number int) ([]byte, error) {
t.Errorf("GetPullRequestDiff should not be called")
return nil, nil
}
cli.FakeCreateCheckRun = func(ctx context.Context, owner, repo string, opt github.CreateCheckRunOptions) (*github.CheckRun, error) {
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
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"),
},
{
Path: github.String("sample.new.txt"),
StartLine: github.Int(14),
EndLine: github.Int(14),
AnnotationLevel: github.String("warning"),
Message: github.String("test message2"),
Title: github.String("[haya14busa-linter] sample.new.txt#L14"),
RawDetails: github.String("raw test message2"),
},
}
if d := cmp.Diff(annotations, wantAnnotaions); d != "" {
t.Errorf("Annotation diff found:\n%s", d)
}
}
return &github.CheckRun{HTMLURL: github.String(reportURL)}, nil
}
checker := &Checker{req: req, gh: cli}
res, err := checker.Check(context.Background())
if err != nil {
t.Fatal(err)
}

if res.ReportURL != reportURL {
t.Errorf("res.reportURL = %q, want %q", res.ReportURL, reportURL)
}
}

func TestCheck_fail_diff(t *testing.T) {
req := &doghouse.CheckRequest{}
req := &doghouse.CheckRequest{PullRequest: 1}
cli := &fakeCheckerGitHubCli{}
cli.FakeGetPullRequestDiff = func(ctx context.Context, owner, repo string, number int) ([]byte, error) {
return nil, errors.New("test diff failure")
Expand All @@ -224,7 +313,7 @@ func TestCheck_fail_diff(t *testing.T) {

func TestCheck_fail_invalid_diff(t *testing.T) {
t.Skip("Parse invalid diff function somehow doesn't return error")
req := &doghouse.CheckRequest{}
req := &doghouse.CheckRequest{PullRequest: 1}
cli := &fakeCheckerGitHubCli{}
cli.FakeGetPullRequestDiff = func(ctx context.Context, owner, repo string, number int) ([]byte, error) {
return []byte("invalid diff"), nil
Expand All @@ -242,7 +331,7 @@ func TestCheck_fail_invalid_diff(t *testing.T) {
}

func TestCheck_fail_check(t *testing.T) {
req := &doghouse.CheckRequest{}
req := &doghouse.CheckRequest{PullRequest: 1}
cli := &fakeCheckerGitHubCli{}
cli.FakeGetPullRequestDiff = func(ctx context.Context, owner, repo string, number int) ([]byte, error) {
return []byte(sampleDiff), nil
Expand All @@ -260,7 +349,7 @@ func TestCheck_fail_check(t *testing.T) {
}

func TestCheck_fail_check_with_403(t *testing.T) {
req := &doghouse.CheckRequest{}
req := &doghouse.CheckRequest{PullRequest: 1}
cli := &fakeCheckerGitHubCli{}
cli.FakeGetPullRequestDiff = func(ctx context.Context, owner, repo string, number int) ([]byte, error) {
return []byte(sampleDiff), nil
Expand Down
2 changes: 1 addition & 1 deletion doghouse/service.go
View file
Expand Up @@ -8,7 +8,7 @@ type CheckRequest struct {
// Required.
SHA string `json:"sha,omitempty"`
// PullRequest number.
// Required.
// Optional.
PullRequest int `json:"pull_request,omitempty"`
// Owner of the repository.
// Required.
Expand Down

0 comments on commit 2d7e23e

Please sign in to comment.