GitHub to post reviews using PRReviewReporter by default

For pull requests, this enables the Pull Request Review reporter to
approve pull requests or post comments if issues are found.

The unit test for GitHub has been simplified further to just check
if a comment has been made. In the future the github package won't
require this mocking at all as it will just provide reports to an
analyse function, and instead rely on the integration tests to ensure
issues are reported.

An additional integration test has been included to verify a review
is posted when there are no issues, and its state is APPROVED.

Resolves #112.

integration_test.go

@@ -296,6 +296,20 @@ func TestGitHubComments_pr(t *testing.T) {
 
 	time.Sleep(5 * time.Second) // wait for comments to appear
 
+	// Make sure review was completed
+	reviews, _, err := it.github.PullRequests.ListReviews(ctx, it.owner, it.repo, *pr.Number, nil)
+	if err != nil {
+		t.Fatalf("could not get pull request reviews: %v", err)
+	}
+
+	if want := 1; len(reviews) != want {
+		t.Fatalf("have %v reviews want %v", len(reviews), want)
+	}
+
+	if want := "COMMENTED"; reviews[0].GetState() != want {
+		t.Fatalf("have %v review state want %v", reviews[0].GetState(), want)
+	}
+
 	// Make sure the expected comments appear
 	comments, _, err := it.github.PullRequests.ListComments(ctx, it.owner, it.repo, *pr.Number, nil)
 	if err != nil {
@@ -371,6 +385,56 @@ func TestGitHubComments_ignoreGenerated(t *testing.T) {
 	}
 }
 
+func TestGitHubComments_none(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
+	defer cancel()
+
+	it := NewIntegrationTest(ctx, t)
+	defer it.Close()
+
+	// Push a branch which contains no issues
+	branch := "issue-no-comments"
+	it.Exec("issue-no-comments.sh", branch)
+
+	// Make PR
+	pr, _, err := it.github.PullRequests.Create(ctx, it.owner, it.repo, &github.NewPullRequest{
+		Title: github.String("pr title"),
+		Head:  github.String(branch),
+		Base:  github.String("master"),
+	})
+	if err != nil {
+		t.Fatalf("could not create pull request: %v", err)
+	}
+
+	it.WaitForSuccess(branch, "ci/gopherci/pr")
+
+	time.Sleep(5 * time.Second) // wait for comments to appear (hopefully none)
+
+	// Make sure review was completed
+	reviews, _, err := it.github.PullRequests.ListReviews(ctx, it.owner, it.repo, *pr.Number, nil)
+	if err != nil {
+		t.Fatalf("could not get pull request reviews: %v", err)
+	}
+
+	if want := 1; len(reviews) != want {
+		t.Fatalf("have %v reviews want %v", len(reviews), want)
+	}
+
+	if want := "APPROVED"; reviews[0].GetState() != want {
+		t.Fatalf("have %v review state want %v", reviews[0].GetState(), want)
+	}
+
+	// Make sure no comments appear
+	comments, _, err := it.github.PullRequests.ListComments(ctx, it.owner, it.repo, *pr.Number, nil)
+	if err != nil {
+		t.Fatalf("could not get pull request comments: %v", err)
+	}
+
+	if want := 0; len(comments) != want {
+		t.Fatalf("have %v comments want %v", len(comments), want)
+	}
+}
+
 // TestAnalysisView tests the HTML generated when viewing an analysis.
 func TestAnalysisView(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)

internal/github/handlers.go

@@ -421,7 +421,7 @@ func (g *GitHub) Analyse(cfg AnalyseConfig) (err error) {
 	switch {
 	case cfg.pr != 0:
 		// Inline code comments on the PR.
-		reporters = append(reporters, NewPRCommentReporter(install.client, cfg.owner, cfg.repo, cfg.pr, cfg.sha))
+		reporters = append(reporters, NewPRReviewReporter(install.client, cfg.owner, cfg.repo, cfg.pr, cfg.sha))
 	case cfg.commitCount == 1:
 		// Comment on the single commit the issues inline.
 		reporters = append(reporters, NewInlineCommitCommentReporter(install.client, cfg.owner, cfg.repo, cfg.sha))

internal/github/handlers_test.go

@@ -622,10 +622,6 @@ func TestAnalyse(t *testing.T) {
 	)
 
 	var (
-		expectedCmtBody   = "Name: error"
-		expectedCmtPath   = "main.go"
-		expectedCmtPos    = 1
-		expectedCmtSHA    = "error"
 		expectedOwner     = "owner"
 		expectedRepo      = "repo"
 		expectedPR        = 3
@@ -644,22 +640,20 @@ func TestAnalyse(t *testing.T) {
 				fmt.Fprintln(w, "[]")
 				break
 			}
-			expected := github.PullRequestComment{
-				Body:     github.String(expectedCmtBody),
-				Path:     github.String(expectedCmtPath),
-				Position: github.Int(expectedCmtPos),
-				CommitID: github.String(expectedCmtSHA),
-			}
-			var comment github.PullRequestComment
-			err := decoder.Decode(&comment)
+		case fmt.Sprintf("/repos/%v/%v/pulls/%v/reviews", expectedOwner, expectedRepo, expectedPR):
+			// Integration test handles the details, we just want to ensure a
+			// review was posted.
+			var have github.PullRequestReviewRequest
+			err := decoder.Decode(&have)
 			if err != nil {
-				t.Fatalf("unexpected error: %v", err)
+				t.Errorf("unexpected error: %v", err)
+				break
 			}
-			if !reflect.DeepEqual(expected, comment) {
-				t.Fatalf("expected cmt:\n%#v\ngot:\n%#v", expected, comment)
-			} else {
-				postedComment = true
+			if want := 1; len(have.Comments) != want {
+				t.Errorf("have review comments count: %v, want: %v", len(have.Comments), want)
+				break
 			}
+			postedComment = true
 		default:
 			t.Logf(r.RequestURI)
 		}
@@ -691,7 +685,7 @@ func TestAnalyse(t *testing.T) {
 		owner:           expectedOwner,
 		repo:            expectedRepo,
 		pr:              expectedPR,
-		sha:             expectedCmtSHA,
+		sha:             "abc123",
 	}
 
 	err := g.Analyse(cfg)

testdata/issue-no-comments.sh

@@ -0,0 +1,12 @@
+#!/bin/bash -eux
+
+git checkout -b $1
+
+cat > foo.go <<EOF
+package foo
+EOF
+
+git add .
+git commit -m "commit"
+
+git push -f -u origin HEAD