Refactor and cleanup github package (#32)

Author: endorama

cmd/find_pr.go

@@ -9,41 +9,23 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"log"
-	"os"
 
-	gb "github.com/elastic/elastic-agent-changelog-tool/internal/github"
-	"github.com/google/go-github/v32/github"
+	"github.com/elastic/elastic-agent-changelog-tool/internal/github"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
-	"golang.org/x/oauth2"
 )
 
 var errListPRCmdMissingCommitHash = errors.New("find-pr requires commit hash argument")
 
 const defaultOwner = "elastic"
 const defaultRepo = "beats"
 
-const repoFlagName = "repo"
-const repoFlagDescription = "target repository"
 const findPRLongDescription = `Use this command to find the original PR that included the commit in the repository.
 
 argument with commit hash is required
 --repo flag is optional and will default to elastic/beats if left unspecified.`
 
-type PRInfo struct {
-	CommitHash    string `json:"commit"`
-	PullRequestID string `json:"pull-request"`
-}
-
-func prToDomain(commitHash string, pr *github.PullRequest) PRInfo {
-	return PRInfo{
-		CommitHash:    commitHash,
-		PullRequestID: fmt.Sprintf("%d", *pr.Number),
-	}
-}
-
-func FindPRCommand() *cobra.Command {
+func FindPRCommand(appFs afero.Fs) *cobra.Command {
 	findPRCommand := &cobra.Command{
 		Use:  "find-pr",
 		Long: findPRLongDescription,
@@ -55,66 +37,34 @@ func FindPRCommand() *cobra.Command {
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-			authToken := gb.NewAuthToken(&afero.OsFs{})
-
-			var GithubClient *gb.GithubClient
-
-			githubAccessToken, err := authToken.AuthToken()
-			switch {
-			case errors.Is(err, os.ErrNotExist):
-				// Github authorization token is not found
-				// we continue using an unauthorized github client
-				GithubClient = gb.NewUnauthorizedClient()
-
-			case err != nil:
-				// If github token is found but couldn't read it's content
-				log.Fatal(err)
-
-			default:
-				// Github token is found and read successfully
-				GithubClient, err = gb.NewClient(gb.NewWrapper(github.NewClient(oauth2.NewClient(context.Background(), oauth2.StaticTokenSource(
-					&oauth2.Token{
-						AccessToken: githubAccessToken,
-					}),
-				))))
-				if err != nil {
-					log.Fatal(err)
-				}
+			hc, err := github.GetHTTPClient(appFs)
+			if err != nil {
+				return fmt.Errorf("cannot initialize http client: %w", err)
 			}
 
-			var commit string
+			c := github.NewClient(hc)
 
-			repo, err := cmd.Flags().GetString(repoFlagName)
+			repo, err := cmd.Flags().GetString("repo")
 			if err != nil {
 				return fmt.Errorf("repo flag malformed: %w", err)
 			}
 
-			commit = args[0]
-
-			if repo == "" {
-				repo = defaultRepo
-			}
-
-			prs, _, err := GithubClient.ListPullRequestsWithCommit(context.Background(), defaultOwner, repo, commit, nil)
+			owner, err := cmd.Flags().GetString("owner")
 			if err != nil {
-				return fmt.Errorf("failed listing prs with commit: %w", err)
-			}
-
-			type resp struct {
-				Items []PRInfo `json:"items"`
+				return fmt.Errorf("owner flag malformed: %w", err)
 			}
 
-			respData := resp{
-				Items: make([]PRInfo, len(prs)),
-			}
+			commit := args[0]
+			ctx := context.Background()
 
-			for i, pr := range prs {
-				respData.Items[i] = prToDomain(commit, pr)
+			res, err := github.FindPR(ctx, c, owner, repo, commit)
+			if err != nil {
+				return fmt.Errorf("failed listing prs with commit: %w", err)
 			}
 
-			respJSON, err := json.Marshal(respData)
+			respJSON, err := json.Marshal(res)
 			if err != nil {
-				return fmt.Errorf("failed listing prs with commit: %w", err)
+				return fmt.Errorf("failed marshalling JSON output: %w", err)
 			}
 
 			cmd.Println(string(respJSON))
@@ -123,7 +73,8 @@ func FindPRCommand() *cobra.Command {
 		},
 	}
 
-	findPRCommand.Flags().String(repoFlagName, "", repoFlagDescription)
+	findPRCommand.Flags().String("repo", defaultRepo, "target repository")
+	findPRCommand.Flags().String("owner", defaultOwner, "target repository owner")
 
 	return findPRCommand
 }

cmd/find_pr_test.go

@@ -0,0 +1,39 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License 2.0;
+// you may not use this file except in compliance with the Elastic License 2.0.
+
+package cmd_test
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/elastic/elastic-agent-changelog-tool/cmd"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFindPrCmd_noArgs(t *testing.T) {
+	fs := afero.NewMemMapFs()
+	cmd := cmd.FindPRCommand(fs)
+
+	err := cmd.Execute()
+	require.Error(t, err)
+}
+
+func TestFindPrCmd_oneArg(t *testing.T) {
+	fs := afero.NewMemMapFs()
+	cmd := cmd.FindPRCommand(fs)
+
+	b := new(bytes.Buffer)
+	cmd.SetOut(b)
+
+	cmd.SetArgs([]string{"--repo", "elastic-agent-changelog-tool", "fe56b2e"})
+
+	err := cmd.Execute()
+	require.Nil(t, err)
+	expected := `{"items":[{"commit":"fe56b2e","pull-request":17}]}
+`
+	require.Equal(t, expected, b.String())
+
+}

go.mod

@@ -15,6 +15,7 @@ require (
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/dnaeon/go-vcr v1.2.0
 	github.com/fsnotify/fsnotify v1.5.1 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/go-querystring v1.0.0 // indirect
@@ -27,7 +28,6 @@ require (
 	github.com/spf13/cast v1.4.1 // indirect
 	github.com/spf13/jwalterweatherman v1.1.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
-	github.com/stretchr/objx v0.1.1 // indirect
 	github.com/subosito/gotenv v1.2.0 // indirect
 	golang.org/x/crypto v0.0.0-20211108221036-ceb1ce70b4fa // indirect
 	golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f // indirect

go.sum

@@ -105,6 +105,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/dnaeon/go-vcr v1.2.0 h1:zHCHvJYTMh1N7xnV7zf1m1GPBF9Ad0Jk/whtQ1663qI=
+github.com/dnaeon/go-vcr v1.2.0/go.mod h1:R4UdLID7HZT3taECzJs4YgbbH6PIGXB6W/sc5OLb6RQ=
 github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
 github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
 github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
@@ -331,6 +333,7 @@ github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJ
 github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
 github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
 github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
+github.com/modocache/gover v0.0.0-20171022184752-b58185e213c5/go.mod h1:caMODM3PzxT8aQXRPkAt8xlV/e7d7w8GM5g0fa5F0D8=
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
 github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=

internal/github/auth.go

@@ -6,59 +6,51 @@ package github
 
 import (
 	"fmt"
+	"log"
 	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/spf13/afero"
 )
 
 const (
-	envAuth       = "GITHUB_TOKEN"
-	authTokenFile = ".elastic/github.token"
+	envAuth = "GITHUB_TOKEN"
 )
 
-// EnsureAuthConfigured method ensures that GitHub auth token is available.
-func (f *AuthToken) EnsureAuthConfigured() error {
-	if _, err := f.AuthToken(); err != nil {
-		return fmt.Errorf("GitHub authorization token is missing. Please use either environment variable %s or ~/%s: %w",
-			envAuth, authTokenFile, err)
-	}
-
-	return nil
-}
+type envProvider func(env string) string
 
 type AuthToken struct {
-	fs *afero.Afero
+	fs       afero.Fs
+	location string
+
+	envProvider envProvider
 }
 
-func NewAuthToken(fs afero.Fs) *AuthToken {
-	return &AuthToken{
-		fs: &afero.Afero{
-			Fs: fs,
-		},
+func NewAuthToken(fs afero.Fs, tkLoc string) AuthToken {
+	return AuthToken{
+		fs:          fs,
+		location:    tkLoc,
+		envProvider: os.Getenv,
 	}
 }
 
-// AuthToken method finds and returns the GitHub authorization token.
-func (f *AuthToken) AuthToken() (string, error) {
-	githubTokenVar := os.Getenv(envAuth)
+func NewTestAuthToken(fs afero.Fs, tkLoc string, ep envProvider) AuthToken {
+	at := NewAuthToken(fs, tkLoc)
+	at.envProvider = ep
 
-	if githubTokenVar != "" {
-		fmt.Println("Using GitHub token from environment variable.")
-		return githubTokenVar, nil
-	}
+	return at
+}
 
-	homeDir, err := os.UserHomeDir()
-	if err != nil {
-		return "", fmt.Errorf("reading user home directory failed: %w", err)
+// AuthToken method finds and returns the GitHub authorization token.
+func (f AuthToken) AuthToken() (string, error) {
+	if githubTokenVar := f.envProvider(envAuth); githubTokenVar != "" {
+		log.Println("Using GitHub token from environment variable")
+		return githubTokenVar, nil
 	}
 
-	githubTokenPath := filepath.Join(homeDir, authTokenFile)
-
-	token, err := f.fs.ReadFile(githubTokenPath)
+	token, err := afero.ReadFile(f.fs, f.location)
 	if err != nil {
-		return "", fmt.Errorf("reading Github token file failed (path: %s): %w", githubTokenPath, err)
+		return "", fmt.Errorf("cannot read Github token file failed (path: %s): %w", f.location, err)
 	}
 
 	return strings.TrimSpace(string(token)), nil

internal/github/auth_test.go

@@ -2,50 +2,54 @@
 // or more contributor license agreements. Licensed under the Elastic License 2.0;
 // you may not use this file except in compliance with the Elastic License 2.0.
 
-package github
+package github_test
 
 import (
 	"io/fs"
-	"os"
-	"path/filepath"
 	"testing"
 
+	"github.com/elastic/elastic-agent-changelog-tool/internal/github"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/require"
 )
 
 func TestAuthToken(t *testing.T) {
 	expectedToken := "ghp_tuQprmeVXWdaMhatQiw8pJdEXPxHWm9tkTJb"
+	testFs := afero.MemMapFs{}
 
-	AuthToken := NewAuthToken(&afero.MemMapFs{})
-
-	homeDir, err := os.UserHomeDir()
+	tokenLocation, err := github.TokenLocation()
+	require.NoError(t, err, "cannot get token location")
+	err = afero.WriteFile(&testFs, tokenLocation, []byte(expectedToken), fs.ModeAppend)
 	require.NoError(t, err)
 
-	githubTokenPath := filepath.Join(homeDir, authTokenFile)
-
-	err = AuthToken.fs.WriteFile(githubTokenPath, []byte(expectedToken), fs.ModeAppend)
-	require.NoError(t, err)
+	tk := github.NewAuthToken(&testFs, tokenLocation)
 
-	token, err := AuthToken.AuthToken()
+	token, err := tk.AuthToken()
 	require.NoError(t, err)
 	require.NotEmpty(t, token)
 	require.Equal(t, expectedToken, token)
 }
 
-func TestEnsureAuthConfigured(t *testing.T) {
+func TestAuthToken_fromEnv(t *testing.T) {
 	expectedToken := "ghp_tuQprmeVXWdaMhatQiw8pJdEXPxHWm9tkTJb"
+	testFs := afero.MemMapFs{}
 
-	homeDir, err := os.UserHomeDir()
-	require.NoError(t, err)
+	tk := github.NewTestAuthToken(&testFs, "using env", func(key string) string {
+		return expectedToken
+	})
 
-	githubTokenPath := filepath.Join(homeDir, authTokenFile)
+	token, err := tk.AuthToken()
+	require.NoError(t, err)
+	require.NotEmpty(t, token)
+	require.Equal(t, expectedToken, token)
+}
 
-	AuthToken := NewAuthToken(&afero.MemMapFs{})
+func TestAuthToken_readFailure(t *testing.T) {
+	testFs := afero.MemMapFs{}
 
-	err = AuthToken.fs.WriteFile(githubTokenPath, []byte(expectedToken), fs.ModeAppend)
-	require.NoError(t, err)
+	tk := github.NewAuthToken(&testFs, "foobar")
 
-	err = AuthToken.EnsureAuthConfigured()
-	require.NoError(t, err)
+	token, err := tk.AuthToken()
+	require.Error(t, err)
+	require.Empty(t, token)
 }