Skip to content

Commit

Permalink
Merge 88976cf into cc1a3c8
Browse files Browse the repository at this point in the history
  • Loading branch information
haya14busa committed Dec 16, 2016
2 parents cc1a3c8 + 88976cf commit 4849d09
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .drone.sec
@@ -1 +1 @@
eyJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkExMjhHQ00ifQ.jNZPGtoP-mdDdVeX3QoNjHkzW3hgBRL_HQKQRkC5dksDXBu9xOCH2dnrFVtPMMXlxdjBNAJw6bc3q27FZxgvBLqiIwj-x_gs5y_X7C5t11d3lWP2-bS-JbHpWiZ5t-Mh_lT-w0Tz4fPH9FtxdOERg_cYfFzXOLUNmq4G3lDFQbLy8Kx3MJNqnDgd3psTXAaJUOToII-wtBgtze-TbDb34kJtCrlEJ5-guepsRXXWl3uu8u7t8jvEgsrwp1rs09rdwGucb2dWf_vo5258pjud2-oinL2nzCKIdzXR4de5B-CFStm9D8QmLqoNOreWe05A2bS6tvsXLJPSOduFB_uH2A.IuaZb-A6fGgzcyff.3uGTMjhUXBGz7KjJGEDsoFar_EzpBb4U3geXkekhCoQoxsXbcWDWcCJLiuX3lXM8aeA6wZHgLpr_6dNGjO10UpwyRozzenzA1dJ3YIqT_PjIWn4sVlOUBT2oIOn7EyCh-2JJWghl9Jnsf7K1_a-fxfLzqB4NrkUHdJNaev5gvh2m85SrfIprKIjI6viT8lLMFtpTsPbmK56-J9Zsgxg-.toQoDcioYYq9TZ6yNiAtug
eyJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkExMjhHQ00ifQ.Zv7ykMwj9ktwhBSlXILbLYz5ZdKyVtEe0zdq5u9HIQ7lDceSYXl3oSspaZtz9TT7FbTj0Qf6p28HtutdE7QCWHuCSGXRXXi3AGS4UEVJq3MvnQGLnnTx8qXjCGhB9KSy3sDo7XqUw7GCCmr3RlZmBBgaIHE9QENnUDp0pXPjzkbNbOR8RQr9E8kja6FTBqCjqEI1xoWDuo3RoH82kRK68Yn3efW-7QFQwP0KufJWlFGZ1Nsr4RNg-Vz8wXzcM12FdUY054MkCHO1b_7A4_lwho52uLojWBnf8P873laDaxDy6au8pSXGoMcGljFzVyPg9FvWPOAu_3tX0JjyES3zRg.6H5T4VhMkZU9XzEn.igEV4rSNLj8beubTCj6xv8R1Nywc_A3OgXlY-svEzzgW_m_R_OJcj9x4P1Z_ZqyFdyeGq0myOFT4TXqrMjLLAA_Xl4-evAKDeBmWNHoRBxRrW4xEmvHlb4WyNfrS5Ve6DwM-tmoNPJSe-8-jWtgNWGaK-LN3lOyrbc_D0W039GYSLGLtutkMS2PWYIDLW_nrciUZuEVpuwMxc12VdH0L.agcIEJDOh_Ohido1KCLr8w
97 changes: 96 additions & 1 deletion github.go
Expand Up @@ -2,10 +2,13 @@ package reviewdog

import (
"fmt"
"log"
"os/exec"
"sync"

"github.com/google/go-github/github"
"golang.org/x/sync/errgroup"

"github.com/google/go-github/github"
)

var _ = github.ScopeAdminOrg
Expand Down Expand Up @@ -50,6 +53,8 @@ type GitHubPullRequest struct {
sha string

postedcs postedcomments

muFlash sync.Mutex
}

// NewGitHubPullReqest returns a new GitHubPullRequest service.
Expand All @@ -66,6 +71,8 @@ func NewGitHubPullReqest(cli *github.Client, owner, repo string, pr int, sha str
// Post accepts a comment and holds it. Flash method actually posts comments to
// GitHub in parallel.
func (g *GitHubPullRequest) Post(c *Comment) error {
g.muFlash.Lock()
defer g.muFlash.Unlock()
g.postComments = append(g.postComments, c)
return nil
}
Expand All @@ -80,11 +87,55 @@ func commentBody(c *Comment) string {
return tool + bodyPrefix + "\n" + c.Body
}

var githubAPIHost = "api.github.com"

// Flash posts comments which has not been posted yet.
func (g *GitHubPullRequest) Flash() error {
g.muFlash.Lock()
defer g.muFlash.Unlock()

if err := g.setPostedComment(); err != nil {
return err
}
// TODO(haya14busa,#58): remove host check when GitHub Enterprise supports
// Pull Request API.
if g.cli.BaseURL.Host == githubAPIHost {
return g.postAsReviewComment()
}
return g.postCommentsForEach()
}

func (g *GitHubPullRequest) postAsReviewComment() error {
comments := make([]*ReviewComment, 0, len(g.postComments))
for _, c := range g.postComments {
if g.postedcs.IsPosted(c) {
continue
}
cbody := commentBody(c)
comments = append(comments, &ReviewComment{
Path: &c.Path,
Position: &c.LnumDiff,
Body: &cbody,
})
}

if len(comments) == 0 {
return nil
}

// TODO(haya14busa): it might be useful to report overview results by "body"
// field.
event := "COMMENT"
review := &Review{Event: &event, Comments: comments}

_, _, err := g.CreateReview(g.owner, g.repo, g.pr, review)
if err != nil {
return err
}
return nil
}

func (g *GitHubPullRequest) postCommentsForEach() error {
var eg errgroup.Group
for _, c := range g.postComments {
comment := c
Expand Down Expand Up @@ -157,3 +208,47 @@ func (g *GitHubPullRequest) comment() ([]*github.PullRequestComment, error) {
}
return comments, nil
}

// ---
// GitHub PullRequest Review API Implementation
// ref: https://github.com/google/go-github/issues/495

const (
mediaTypePullRequestReview = "application/vnd.github.black-cat-preview+json"
)

// Review represents a pull request review.
type Review struct {
Body *string `json:"body,omitempty"`
Event *string `json:"event,omitempty"`
Comments []*ReviewComment `json:"comments,omitempty"`
}

// ReviewComment represents draft review comments.
type ReviewComment struct {
Path *string `json:"path,omitempty"`
Position *int `json:"position,omitempty"`
Body *string `json:"body,omitempty"`
}

// CreateReview creates a new review comment on the specified pull request.
//
// GitHub API docs: https://developer.github.com/v3/pulls/reviews/#create-a-pull-request-review
func (g *GitHubPullRequest) CreateReview(owner, repo string, number int, review *Review) (*github.PullRequestReview, *github.Response, error) {
u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews", owner, repo, number)

req, err := g.cli.NewRequest("POST", u, review)
if err != nil {
return nil, nil, err
}

req.Header.Set("Accept", mediaTypePullRequestReview)

r := new(github.PullRequestReview)
resp, err := g.cli.Do(req, r)
if err != nil {
log.Printf("GitHub Review API error: %v", err)
return nil, resp, err
}
return r, resp, err
}
69 changes: 68 additions & 1 deletion github_test.go
Expand Up @@ -12,6 +12,22 @@ import (
"golang.org/x/oauth2"
)

func setupGithub() (*http.ServeMux, *httptest.Server, *github.Client) {
mux := http.NewServeMux()
server := httptest.NewServer(mux)
client := github.NewClient(nil)
url, _ := url.Parse(server.URL)
client.BaseURL = url
client.UploadURL = url
return mux, server, client
}

func testMethod(t *testing.T, r *http.Request, want string) {
if got := r.Method; got != want {
t.Errorf("Request method: %v, want %v", got, want)
}
}

const notokenSkipTestMes = "skipping test (requires actual Personal access tokens. export REVIEWDOG_TEST_GITHUB_API_TOKEN=<GitHub Personal Access Token>)"

func setupGitHubClient() *github.Client {
Expand Down Expand Up @@ -42,7 +58,7 @@ func TestGitHubPullRequest_Post(t *testing.T) {
g := NewGitHubPullReqest(client, owner, repo, pr, sha)
comment := &Comment{
CheckResult: &CheckResult{
Path: "reviewdog.go",
Path: "watchdogs.go",
},
LnumDiff: 17,
Body: "[reviewdog] test",
Expand Down Expand Up @@ -208,3 +224,54 @@ func TestGitHubPullRequest_Post_Flash_mock(t *testing.T) {
t.Errorf("API should be called 2 times, but %v times", apiCalled)
}
}

func TestGitHubPullRequest_Post_Flash_mock_review_api(t *testing.T) {
mux, server, client := setupGithub()
defer server.Close()
defer func(u string) { githubAPIHost = u }(githubAPIHost)
u, _ := url.Parse(server.URL)
githubAPIHost = u.Host

mux.HandleFunc("/repos/o/r/pulls/1/comments", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
})
mux.HandleFunc("/repos/o/r/pulls/1/reviews", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")

v := new(Review)
json.NewDecoder(r.Body).Decode(v)

if v.Body != nil {
t.Errorf("Review body = %v, want nil", *v.Body)
}
if want := "COMMENT"; *v.Event != want {
t.Errorf("Review event = %v, want %v", *v.Event, want)
}
c := v.Comments[0]

if want := bodyPrefix + "\n[reviewdog] test"; *c.Body != want {
t.Errorf("Review comment body = %v, want %v", *c.Body, want)
}
if want := "reviewdog.go"; *c.Path != want {
t.Errorf("Review comment position = %v, want %v", *c.Path, want)
}
if want := 17; *c.Position != want {
t.Errorf("Review comment position = %v, want %v", *c.Position, want)
}
})

g := NewGitHubPullReqest(client, "o", "r", 1, "SHA1")
comment := &Comment{
CheckResult: &CheckResult{
Path: "reviewdog.go",
},
LnumDiff: 17,
Body: "[reviewdog] test",
}
if err := g.Post(comment); err != nil {
t.Error(err)
}
if err := g.Flash(); err != nil {
t.Error(err)
}
}
7 changes: 4 additions & 3 deletions reviewdog.go
Expand Up @@ -91,9 +91,6 @@ func (w *Reviewdog) Run(r io.Reader) error {
return err
}

if bulk, ok := w.c.(BulkCommentService); ok {
defer bulk.Flash()
}
for _, result := range results {
addedline := addedlines.Get(result.Path, result.Lnum)
if filepath.IsAbs(result.Path) {
Expand All @@ -117,6 +114,10 @@ func (w *Reviewdog) Run(r io.Reader) error {
}
}

if bulk, ok := w.c.(BulkCommentService); ok {
return bulk.Flash()
}

return nil
}

Expand Down

0 comments on commit 4849d09

Please sign in to comment.