From 3018275cb4d3dde3360dca38e9c18b5cb95fbb5f Mon Sep 17 00:00:00 2001 From: Maxim Sukharev Date: Fri, 17 Aug 2018 14:26:28 +0200 Subject: [PATCH 1/3] Skip out of range comments Signed-off-by: Maxim Sukharev --- provider/github/diff.go | 2 +- provider/github/poster.go | 4 +++ provider/github/poster_test.go | 58 ++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/provider/github/diff.go b/provider/github/diff.go index dc8e6e09..c0a7e5a6 100644 --- a/provider/github/diff.go +++ b/provider/github/diff.go @@ -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) { diff --git a/provider/github/poster.go b/provider/github/poster.go index d97356e2..5176e285 100644 --- a/provider/github/poster.go +++ b/provider/github/poster.go @@ -171,6 +171,10 @@ func (p *Poster) createReviewRequest( req.Comments = append(req.Comments, comment) } else { line, err := dl.ConvertLine(c.File, int(c.Line)) + if ErrLineOutOfDiff.Is(err) { + continue + } + if err != nil { return nil, err } diff --git a/provider/github/poster_test.go b/provider/github/poster_test.go index 051ba5da..67a18def 100644 --- a/provider/github/poster_test.go +++ b/provider/github/poster_test.go @@ -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 From 8230b40b15b3efb0098a5659d830645e61b9658f Mon Sep 17 00:00:00 2001 From: Maxim Sukharev Date: Fri, 17 Aug 2018 17:09:05 +0200 Subject: [PATCH 2/3] add debug log for comment out of diff range Signed-off-by: Maxim Sukharev --- provider/github/poster.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/provider/github/poster.go b/provider/github/poster.go index 5176e285..b7aaba28 100644 --- a/provider/github/poster.go +++ b/provider/github/poster.go @@ -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 ( @@ -172,6 +173,9 @@ func (p *Poster) createReviewRequest( } 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 } From 534ba9ac7b26087b62e0b84729131858bb06e479 Mon Sep 17 00:00:00 2001 From: Maxim Sukharev Date: Fri, 17 Aug 2018 17:46:20 +0200 Subject: [PATCH 3/3] regenerate go code from proto (fix master) Signed-off-by: Maxim Sukharev --- pb/event.pb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pb/event.pb.go b/pb/event.pb.go index 34f11df7..883312e9 100644 --- a/pb/event.pb.go +++ b/pb/event.pb.go @@ -113,7 +113,7 @@ type ReviewEvent struct { IsMergeable bool `protobuf:"varint,5,opt,name=is_mergeable,json=isMergeable,proto3" json:"is_mergeable,omitempty"` // Source reference to the original branch and repository where the changes came from. Source ReferencePointer `protobuf:"bytes,8,opt,name=source" json:"source"` - // Merge branch in the destination repository where the Pull Request is stored. + // Merge reference to the branch and repository where the merged Pull Request is stored. Merge ReferencePointer `protobuf:"bytes,9,opt,name=merge" json:"merge"` // Configuration contains any configuration related to specific analyzer Configuration google_protobuf2.Struct `protobuf:"bytes,10,opt,name=configuration" json:"configuration"`