Skip to content

Commit

Permalink
Merge pull request #151 from smacker/fix_github_out_of_range_poster
Browse files Browse the repository at this point in the history
Skip out of range comments
  • Loading branch information
smacker authored Aug 17, 2018
2 parents 341254d + 534ba9a commit 58f789c
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pb/event.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion provider/github/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (d *diffLines) convertLine(ranges []*posRange, line int) (int, error) {
}
}

return 0, fmt.Errorf("line position is not in diff range")
return 0, ErrLineOutOfDiff.New()
}

func (d *diffLines) ranges(file string) ([]*posRange, error) {
Expand Down
8 changes: 8 additions & 0 deletions provider/github/poster.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/google/go-github/github"
"gopkg.in/src-d/go-errors.v1"
log "gopkg.in/src-d/go-log.v1"
)

var (
Expand Down Expand Up @@ -171,6 +172,13 @@ func (p *Poster) createReviewRequest(
req.Comments = append(req.Comments, comment)
} else {
line, err := dl.ConvertLine(c.File, int(c.Line))
if ErrLineOutOfDiff.Is(err) {
log.Debugf(
"comment is out the diff range. analyzer: %s, file %s, line %d",
aComments.Config.Name, c.File, c.Line)
continue
}

if err != nil {
return nil, err
}
Expand Down
58 changes: 58 additions & 0 deletions provider/github/poster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,64 @@ func (s *PosterTestSuite) TestPostHttpJSONErr() {
s.IsType(ErrGitHubAPI.New(), err)
}

func (s *PosterTestSuite) TestPostOutOfRange() {
compareCalled := false
s.compareHandle(&compareCalled)

createReviewsCalled := false
s.mux.HandleFunc("/repos/foo/bar/pulls/42/reviews", func(w http.ResponseWriter, r *http.Request) {
s.False(createReviewsCalled)
createReviewsCalled = true

body, err := ioutil.ReadAll(r.Body)
s.NoError(err)

expected, _ := json.Marshal(&github.PullRequestReviewRequest{
Body: strptr(""),
Event: strptr("APPROVE"),
Comments: []*github.DraftReviewComment{&github.DraftReviewComment{
Path: strptr("main.go"),
Position: intptr(1),
Body: strptr("Line comment"),
}}})
s.JSONEq(string(expected), string(body))

resp := &github.Response{Response: &http.Response{StatusCode: 200}}
json.NewEncoder(w).Encode(resp)
})

mockComments = []*lookout.Comment{
&lookout.Comment{
File: "main.go",
Line: 1,
Text: "out of range comment before",
},
&lookout.Comment{
File: "main.go",
Line: 3,
Text: "Line comment",
},
&lookout.Comment{
File: "main.go",
Line: 205,
Text: "out of range comment after",
}}

mockAnalyzerComments = []lookout.AnalyzerComments{
lookout.AnalyzerComments{
Config: lookout.AnalyzerConfig{
Name: "mock",
},
Comments: mockComments,
}}

p := &Poster{pool: s.pool}
err := p.Post(context.Background(), mockEvent, mockAnalyzerComments)
s.NoError(err)

s.True(createReviewsCalled)
}

func (s *PosterTestSuite) TestStatusOK() {
createStatusCalled := false

Expand Down

0 comments on commit 58f789c

Please sign in to comment.