Update CRD resource docs, increase unit test coverage.

cmd/pullrequest-init/README.md

@@ -57,6 +57,8 @@ GitHub pull requests will output these additional files:
 *   `$PATH/github/comments/#.json`: Comments associated to the PR as specified
     by https://developer.github.com/v3/issues/comments/#get-a-single-comment
 
+For now, these files are *read-only*.
+
 The binary will look for GitHub credentials in the `${GITHUBTOKEN}` environment
 variable. This should generally be specified as a secret with the field name
 `githubToken` in the `PullRequestResource` definition.

cmd/pullrequest-init/fake_github.go

@@ -5,11 +5,18 @@ import (
 	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 
 	"github.com/google/go-github/github"
 	"github.com/gorilla/mux"
 )
 
+const (
+	// ErrorKeyword is a magic const used to denote PRs/Comments that should
+	// return errors to the client to simulate issues communicating with GitHub.
+	ErrorKeyword = "~~ERROR~~"
+)
+
 // key defines keys for associating data to PRs/issues in the fake server.
 type key struct {
 	owner string
@@ -21,20 +28,28 @@ type key struct {
 type FakeGitHub struct {
 	*mux.Router
 
-	pr       map[key]*github.PullRequest
-	comments map[key][]*github.IssueComment
+	pr map[key]*github.PullRequest
+	// GitHub references comments in 2 ways:
+	// 1) List by issue (PR) ID.
+	// 2) Get by comment ID.
+	// We need to store references to both to emulate the API properly.
+	prComments map[key][]*github.IssueComment
+	comments   map[key]*github.IssueComment
 }
 
 // NewFakeGitHub returns a new FakeGitHub.
 func NewFakeGitHub() *FakeGitHub {
 	s := &FakeGitHub{
-		Router:   mux.NewRouter(),
-		pr:       make(map[key]*github.PullRequest),
-		comments: make(map[key][]*github.IssueComment),
+		Router:     mux.NewRouter(),
+		pr:         make(map[key]*github.PullRequest),
+		prComments: make(map[key][]*github.IssueComment),
+		comments:   make(map[key]*github.IssueComment),
 	}
 	s.HandleFunc("/repos/{owner}/{repo}/pulls/{number}", s.getPullRequest).Methods(http.MethodGet)
 	s.HandleFunc("/repos/{owner}/{repo}/issues/{number}/comments", s.getComments).Methods(http.MethodGet)
 	s.HandleFunc("/repos/{owner}/{repo}/issues/{number}/comments", s.createComment).Methods(http.MethodPost)
+	s.HandleFunc("/repos/{owner}/{repo}/issues/comments/{number}", s.updateComment).Methods(http.MethodPatch)
+	s.HandleFunc("/repos/{owner}/{repo}/issues/comments/{number}", s.deleteComment).Methods(http.MethodDelete)
 	s.HandleFunc("/repos/{owner}/{repo}/issues/{number}/labels", s.updateLabels).Methods(http.MethodPut)
 
 	return s
@@ -53,6 +68,8 @@ func prKey(r *http.Request) (key, error) {
 }
 
 // AddPullRequest adds the given pull request to the fake GitHub server.
+// This is done as a convenience over implementing PullReqests.Create in the
+// GitHub server since that method takes in a different type (NewPullRequest).
 func (g *FakeGitHub) AddPullRequest(pr *github.PullRequest) {
 	key := key{
 		owner: pr.GetBase().GetUser().GetLogin(),
@@ -62,26 +79,11 @@ func (g *FakeGitHub) AddPullRequest(pr *github.PullRequest) {
 	g.pr[key] = pr
 }
 
-// AddComment adds a comment to the fake GitHub server.
-func (g *FakeGitHub) AddComment(owner string, repo string, pr int64, comment *github.IssueComment) {
-	key := key{
-		owner: owner,
-		repo:  repo,
-		id:    pr,
-	}
-	c := g.comments[key]
-	if c == nil {
-		c = []*github.IssueComment{comment}
-	} else {
-		c = append(c, comment)
-	}
-	g.comments[key] = c
-}
-
 func (g *FakeGitHub) getPullRequest(w http.ResponseWriter, r *http.Request) {
 	key, err := prKey(r)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
 	}
 
 	pr, ok := g.pr[key]
@@ -102,7 +104,7 @@ func (g *FakeGitHub) getComments(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 	}
 
-	comments, ok := g.comments[key]
+	comments, ok := g.prComments[key]
 	if !ok {
 		comments = []*github.IssueComment{}
 	}
@@ -113,22 +115,91 @@ func (g *FakeGitHub) getComments(w http.ResponseWriter, r *http.Request) {
 }
 
 func (g *FakeGitHub) createComment(w http.ResponseWriter, r *http.Request) {
-	key, err := prKey(r)
+	prKey, err := prKey(r)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
 	}
-
 	c := new(github.IssueComment)
 	if err := json.NewDecoder(r.Body).Decode(c); err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
-	comments, ok := g.comments[key]
+	if strings.Contains(c.GetBody(), ErrorKeyword) {
+		http.Error(w, "intentional error", http.StatusInternalServerError)
+		return
+	}
+
+	c.ID = github.Int64(int64(len(g.comments) + 1))
+
+	if _, ok := g.prComments[prKey]; !ok {
+		g.prComments[prKey] = []*github.IssueComment{}
+	}
+	g.prComments[prKey] = append(g.prComments[prKey], c)
+
+	commentKey := key{
+		owner: prKey.owner,
+		repo:  prKey.repo,
+		id:    c.GetID(),
+	}
+	g.comments[commentKey] = c
+
+	if err := json.NewEncoder(w).Encode(c); err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+}
+
+func (g *FakeGitHub) updateComment(w http.ResponseWriter, r *http.Request) {
+	key, err := prKey(r)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+	update := new(github.IssueComment)
+	if err := json.NewDecoder(r.Body).Decode(update); err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	if strings.Contains(update.GetBody(), ErrorKeyword) {
+		http.Error(w, "intentional error", http.StatusInternalServerError)
+		return
+	}
+
+	existing, ok := g.comments[key]
 	if !ok {
-		comments = []*github.IssueComment{}
+		http.Error(w, err.Error(), http.StatusNotFound)
+		return
+	}
+
+	*existing = *update
+	w.WriteHeader(http.StatusOK)
+}
+
+func (g *FakeGitHub) deleteComment(w http.ResponseWriter, r *http.Request) {
+	key, err := prKey(r)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	if _, ok := g.comments[key]; !ok {
+		http.Error(w, fmt.Sprintf("comment %+v not found", key), http.StatusNotFound)
+		return
+	}
+
+	// Remove comment from PR storage. Not particularly efficient, but we don't
+	// generally expect this to be used for large number of comments in unit
+	// tests.
+	for k, _ := range g.prComments {
+		for i, c := range g.prComments[k] {
+			if c.GetID() == key.id {
+				g.prComments[k] = append(g.prComments[k][:i], g.prComments[k][i+1:]...)
+				break
+			}
+		}
 	}
-	c.ID = github.Int64(int64(len(comments) + 1))
-	g.comments[key] = append(comments, c)
+	delete(g.comments, key)
 
 	w.WriteHeader(http.StatusOK)
 }

cmd/pullrequest-init/fake_github_test.go

@@ -0,0 +1,66 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-github/github"
+)
+
+func TestFakeGitHubPullRequest(t *testing.T) {
+	ctx := context.Background()
+	gh := NewFakeGitHub()
+	client, close := githubClient(t, gh)
+	defer close()
+
+	if _, resp, err := client.PullRequests.Get(ctx, owner, repo, prNum); err == nil || resp.StatusCode != http.StatusNotFound {
+		t.Fatalf("Get PullRequest: wanted not found, got %+v, %v", resp, err)
+	}
+	gh.AddPullRequest(pr)
+
+	got, resp, err := client.PullRequests.Get(ctx, owner, repo, prNum)
+	if err != nil || resp.StatusCode != http.StatusOK {
+		t.Fatalf("Get PullRequest: wanted OK, got %+v, %v", resp, err)
+	}
+	if diff := cmp.Diff(pr, got); diff != "" {
+		t.Errorf("Get PullRequest: -want +got: %s", diff)
+	}
+}
+
+func TestFakeGitHubComments(t *testing.T) {
+	ctx := context.Background()
+	gh := NewFakeGitHub()
+	client, close := githubClient(t, gh)
+	defer close()
+
+	if got, resp, err := client.Issues.ListComments(ctx, owner, repo, prNum, nil); err != nil || resp.StatusCode != http.StatusOK || len(got) != 0 {
+		t.Fatalf("List Issues: wanted [], got %+v, %+v, %v", got, resp, err)
+	}
+
+	if _, _, err := client.Issues.CreateComment(ctx, owner, repo, prNum, comment); err != nil {
+		t.Fatalf("CreateComment: %v", err)
+	}
+
+	got, resp, err := client.Issues.ListComments(ctx, owner, repo, prNum, nil)
+	if err != nil || resp.StatusCode != http.StatusOK {
+		t.Fatalf("List Issues: wanted OK, got %+v, %v", resp, err)
+	}
+	want := []*github.IssueComment{comment}
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("List Issues: -want +got: %s", diff)
+	}
+}
+
+func TestFakeGitHubBadKey(t *testing.T) {
+	gh := NewFakeGitHub()
+	s := httptest.NewServer(gh)
+	defer s.Close()
+
+	if resp, err := http.Get(fmt.Sprintf("%s/repos/1/2/pulls/foo", s.URL)); err != nil || resp.StatusCode != http.StatusBadRequest {
+		t.Errorf("want BadRequest, got %+v, %v", resp, err)
+	}
+}