cmd,internal: replace fakeClient with mock test setup

Previously, issue.Client was an interface, so that we could use a fake
client for testing.

This made it difficult to jump to definition, when debugging issues with
the client. Additionally, the tests weren't very effective, since they
didn't traverse the githubClient code path at all.

Replace the test setup with a mock http server, and make issues.Client
to a struct.

Change-Id: I878846ce69c2979b45ff6e8494bc0f7c020055ea
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/458203
Run-TryBot: Julie Qiu <[email protected]>
Reviewed-by: Julie Qiu <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Tatiana Bradley <[email protected]>

Author: julieqiu

cmd/issue/main.go

@@ -50,7 +50,7 @@ func main() {
 	if err != nil {
 		log.Fatal(err)
 	}
-	c := issues.NewGitHubClient(owner, repoName, *githubToken)
+	c := issues.NewClient(&issues.Config{Owner: owner, Repo: repoName, Token: *githubToken})
 	switch cmd {
 	case "triage":
 		err = createIssueToTriage(ctx, c, *githubToken, filename)
@@ -64,7 +64,7 @@ func main() {
 	}
 }
 
-func createIssueToTriage(ctx context.Context, c issues.Client, ghToken, filename string) (err error) {
+func createIssueToTriage(ctx context.Context, c *issues.Client, ghToken, filename string) (err error) {
 	aliases, err := parseAliases(filename)
 	if err != nil {
 		return err
@@ -77,7 +77,7 @@ func createIssueToTriage(ctx context.Context, c issues.Client, ghToken, filename
 	return nil
 }
 
-func createExcluded(ctx context.Context, c issues.Client, ghToken, filename string) (err error) {
+func createExcluded(ctx context.Context, c *issues.Client, ghToken, filename string) (err error) {
 	records, err := parseExcluded(filename)
 	if err != nil {
 		return err
@@ -90,7 +90,7 @@ func createExcluded(ctx context.Context, c issues.Client, ghToken, filename stri
 	return nil
 }
 
-func constructIssue(ctx context.Context, c issues.Client, alias, ghToken string, labels []string) (err error) {
+func constructIssue(ctx context.Context, c *issues.Client, alias, ghToken string, labels []string) (err error) {
 	var ghsas []*ghsa.SecurityAdvisory
 	if strings.HasPrefix(alias, "GHSA") {
 		sa, err := ghsa.FetchGHSA(ctx, ghToken, alias)

cmd/vulnreport/main.go

@@ -235,7 +235,7 @@ func parseArgsToGithubIDs(args []string, existingByIssue map[int]*report.Report)
 type createCfg struct {
 	ghToken         string
 	repo            *git.Repository
-	issuesClient    issues.Client
+	issuesClient    *issues.Client
 	existingByFile  map[string]*report.Report
 	existingByIssue map[int]*report.Report
 	allowClosed     bool
@@ -272,7 +272,7 @@ func setupCreate(ctx context.Context, args []string) ([]int, *createCfg, error)
 	return githubIDs, &createCfg{
 		ghToken:         *githubToken,
 		repo:            repo,
-		issuesClient:    issues.NewGitHubClient(owner, repoName, *githubToken),
+		issuesClient:    issues.NewClient(&issues.Config{Owner: owner, Repo: repoName, Token: *githubToken}),
 		existingByFile:  existingByFile,
 		existingByIssue: existingByIssue,
 		allowClosed:     *closedOk,

cmd/worker/main.go

@@ -255,7 +255,7 @@ func createIssuesCommand(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	client := issues.NewGitHubClient(owner, repoName, cfg.GitHubAccessToken)
+	client := issues.NewClient(&issues.Config{Owner: owner, Repo: repoName, Token: cfg.GitHubAccessToken})
 	repo, err := gitrepo.Clone(ctx, "https://github.com/golang/vulndb")
 	if err != nil {
 		return err

internal/issues/fake.go

@@ -0,0 +1,39 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package githubtest provides a test client and server for testing the GitHub API
+// client.
+package githubtest
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"golang.org/x/vulndb/internal/issues"
+)
+
+const (
+	TestOwner = "test-owner"
+	TestRepo  = "test-repo"
+	TestToken = "test-token"
+
+	testBaseURLPath = "/api-test"
+)
+
+// Setup sets up a test HTTP server along with a issues.Client that is
+// configured to talk to that test server.
+func Setup(t *testing.T, cfg *issues.Config) (*issues.Client, *http.ServeMux) {
+	mux := http.NewServeMux()
+	apiHandler := http.NewServeMux()
+
+	apiHandler.Handle(testBaseURLPath+"/", http.StripPrefix(testBaseURLPath, mux))
+	server := httptest.NewServer(apiHandler)
+
+	url, _ := url.Parse(server.URL + testBaseURLPath + "/")
+	client := issues.NewTestClient(cfg, url)
+	t.Cleanup(server.Close)
+	return client, mux
+}

internal/issues/githubtest/setup.go

@@ -9,6 +9,7 @@ package issues
 import (
 	"context"
 	"fmt"
+	"net/url"
 	"time"
 
 	"github.com/google/go-github/v41/github"
@@ -37,56 +38,60 @@ type GetIssuesOptions struct {
 }
 
 // Client is a client that can create and retrieve issues.
-type Client interface {
-	// Destination describes where issues will be created.
-	Destination() string
-
-	// Reference returns a string that refers to the issue with number.
-	Reference(number int) string
-
-	// IssueExists reports whether an issue with the given ID exists.
-	IssueExists(ctx context.Context, number int) (bool, error)
-
-	// CreateIssue creates a new issue.
-	CreateIssue(ctx context.Context, iss *Issue) (number int, err error)
-
-	// GetIssue returns an issue with the given issue number.
-	GetIssue(ctx context.Context, number int) (iss *Issue, err error)
-
-	GetIssues(ctx context.Context, opts GetIssuesOptions) (issues []*Issue, err error)
-}
-
-type githubClient struct {
+type Client struct {
 	client *github.Client
 	owner  string
 	repo   string
 }
 
-// NewGitHubClient creates a Client that will create issues in
+// Config is used to initialize a new Client.
+type Config struct {
+	// Owner is the owner of a GitHub repo. For example, "golang" is the owner
+	// for github.com/golang/vulndb.
+	Owner string
+
+	// Repo is the name of a GitHub repo. For example, "vulndb" is the repo
+	// name for github.com/golang/vulndb.
+	Repo string
+
+	// Token is access token that authorizes and authenticates
+	// requests to the GitHub API.
+	Token string
+}
+
+// NewClient creates a Client that will create issues in
 // the a GitHub repo.
-// A GitHub access token is required to create issues.
-func NewGitHubClient(owner, repo, accessToken string) *githubClient {
-	ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: accessToken})
+func NewClient(cfg *Config) *Client {
+	ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: cfg.Token})
 	tc := oauth2.NewClient(context.Background(), ts)
-	return &githubClient{
-		client: github.NewClient(tc),
-		owner:  owner,
-		repo:   repo,
+	c := github.NewClient(tc)
+	return &Client{
+		client: c,
+		owner:  cfg.Owner,
+		repo:   cfg.Repo,
 	}
 }
 
+// NewTestClient creates a Client for use in tests.
+func NewTestClient(cfg *Config, baseURL *url.URL) *Client {
+	c := NewClient(cfg)
+	c.client.BaseURL = baseURL
+	c.client.UploadURL = baseURL
+	return c
+}
+
 // Destination implements Client.Destination.
-func (c *githubClient) Destination() string {
+func (c *Client) Destination() string {
 	return fmt.Sprintf("https://github.com/%s/%s", c.owner, c.repo)
 }
 
 // Reference implements Client.Reference.
-func (c *githubClient) Reference(num int) string {
+func (c *Client) Reference(num int) string {
 	return fmt.Sprintf("%s/issues/%d", c.Destination(), num)
 }
 
 // IssueExists implements Client.IssueExists.
-func (c *githubClient) IssueExists(ctx context.Context, number int) (_ bool, err error) {
+func (c *Client) IssueExists(ctx context.Context, number int) (_ bool, err error) {
 	defer derrors.Wrap(&err, "IssueExists(%d)", number)
 
 	iss, _, err := c.client.Issues.Get(ctx, c.owner, c.repo, number)
@@ -131,7 +136,7 @@ func convertGithubIssueToIssue(ghIss *github.Issue) *Issue {
 }
 
 // GetIssue implements Client.GetIssue.
-func (c *githubClient) GetIssue(ctx context.Context, number int) (_ *Issue, err error) {
+func (c *Client) GetIssue(ctx context.Context, number int) (_ *Issue, err error) {
 	defer derrors.Wrap(&err, "GetIssue(%d)", number)
 	ghIss, _, err := c.client.Issues.Get(ctx, c.owner, c.repo, number)
 	if err != nil {
@@ -143,7 +148,7 @@ func (c *githubClient) GetIssue(ctx context.Context, number int) (_ *Issue, err
 }
 
 // GetIssues implements Client.GetIssues
-func (c *githubClient) GetIssues(ctx context.Context, opts GetIssuesOptions) (_ []*Issue, err error) {
+func (c *Client) GetIssues(ctx context.Context, opts GetIssuesOptions) (_ []*Issue, err error) {
 	defer derrors.Wrap(&err, "GetIssues()")
 	clientOpts := &github.IssueListByRepoOptions{
 		State:  opts.State,
@@ -175,7 +180,7 @@ func (c *githubClient) GetIssues(ctx context.Context, opts GetIssuesOptions) (_
 }
 
 // CreateIssue implements Client.CreateIssue.
-func (c *githubClient) CreateIssue(ctx context.Context, iss *Issue) (number int, err error) {
+func (c *Client) CreateIssue(ctx context.Context, iss *Issue) (number int, err error) {
 	defer derrors.Wrap(&err, "CreateIssue(%s)", iss.Title)
 
 	req := &github.IssueRequest{