Make NewCmdApi testable

This commit is contained in:
Mislav Marohnić 2020-05-20 12:54:09 +02:00
parent f58e0bf710
commit bef62faaea
8 changed files with 238 additions and 59 deletions

View file

@ -3,6 +3,7 @@ package command
import (
"fmt"
"io"
"net/http"
"os"
"regexp"
"runtime/debug"
@ -14,6 +15,7 @@ import (
"github.com/cli/cli/internal/ghrepo"
apiCmd "github.com/cli/cli/pkg/cmd/api"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
@ -64,7 +66,19 @@ func init() {
return &cmdutil.FlagError{Err: err}
})
RootCmd.AddCommand(apiCmd.NewCmdApi())
// TODO: iron out how a factory incorporates context
cmdFactory := &cmdutil.Factory{
IOStreams: iostreams.System(),
HttpClient: func() (*http.Client, error) {
ctx := context.New()
token, err := ctx.AuthToken()
if err != nil {
return nil, err
}
return httpClient(token), nil
},
}
RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil))
}
// RootCmd is the entry point of command-line execution
@ -119,6 +133,19 @@ func contextForCommand(cmd *cobra.Command) context.Context {
return ctx
}
// for cmdutil-powered commands
func httpClient(token string) *http.Client {
var opts []api.ClientOption
if verbose := os.Getenv("DEBUG"); verbose != "" {
opts = append(opts, apiVerboseLog())
}
opts = append(opts,
api.AddHeader("Authorization", fmt.Sprintf("token %s", token)),
api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)),
)
return api.NewHTTPClient(opts...)
}
// overridden in tests
var apiClientForContext = func(ctx context.Context) (*api.Client, error) {
token, err := ctx.AuthToken()

2
go.mod
View file

@ -21,7 +21,7 @@ require (
github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f // indirect
github.com/spf13/cobra v0.0.6
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.4.0 // indirect
github.com/stretchr/testify v1.5.1
golang.org/x/crypto v0.0.0-20200219234226-1ad67e1f0ef4
golang.org/x/net v0.0.0-20200219183655-46282727080f // indirect
golang.org/x/text v0.3.2

5
go.sum
View file

@ -167,13 +167,14 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A=
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.2.1/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4=
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=

View file

@ -9,7 +9,6 @@ import (
"strconv"
"strings"
"github.com/cli/cli/context"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/spf13/cobra"
@ -29,8 +28,12 @@ type ApiOptions struct {
HttpClient func() (*http.Client, error)
}
func NewCmdApi() *cobra.Command {
opts := ApiOptions{}
func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command {
opts := ApiOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
}
cmd := &cobra.Command{
Use: "api <endpoint>",
Short: "Make an authenticated GitHub API request",
@ -58,18 +61,9 @@ on the format of the value:
opts.RequestPath = args[0]
opts.RequestMethodPassed = c.Flags().Changed("method")
// TODO: pass in via caller
opts.IO = iostreams.System()
opts.HttpClient = func() (*http.Client, error) {
ctx := context.New()
token, err := ctx.AuthToken()
if err != nil {
return nil, err
}
return apiClientFromContext(token), nil
if runF != nil {
return runF(&opts)
}
return apiRun(&opts)
},
}

View file

@ -3,15 +3,115 @@ package api
import (
"bytes"
"fmt"
"io"
"io/ioutil"
"net/http"
"reflect"
"os"
"testing"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
)
func Test_NewCmdApi(t *testing.T) {
f := &cmdutil.Factory{}
tests := []struct {
name string
cli string
wants ApiOptions
wantsErr bool
}{
{
name: "no flags",
cli: "graphql",
wants: ApiOptions{
RequestMethod: "GET",
RequestMethodPassed: false,
RequestPath: "graphql",
RawFields: []string(nil),
MagicFields: []string(nil),
RequestHeaders: []string(nil),
ShowResponseHeaders: false,
},
wantsErr: false,
},
{
name: "override method",
cli: "repos/octocat/Spoon-Knife -XDELETE",
wants: ApiOptions{
RequestMethod: "DELETE",
RequestMethodPassed: true,
RequestPath: "repos/octocat/Spoon-Knife",
RawFields: []string(nil),
MagicFields: []string(nil),
RequestHeaders: []string(nil),
ShowResponseHeaders: false,
},
wantsErr: false,
},
{
name: "with fields",
cli: "graphql -f query=QUERY -F body=@file.txt",
wants: ApiOptions{
RequestMethod: "GET",
RequestMethodPassed: false,
RequestPath: "graphql",
RawFields: []string{"query=QUERY"},
MagicFields: []string{"body=@file.txt"},
RequestHeaders: []string(nil),
ShowResponseHeaders: false,
},
wantsErr: false,
},
{
name: "with headers",
cli: "user -H 'accept: text/plain' -i",
wants: ApiOptions{
RequestMethod: "GET",
RequestMethodPassed: false,
RequestPath: "user",
RawFields: []string(nil),
MagicFields: []string(nil),
RequestHeaders: []string{"accept: text/plain"},
ShowResponseHeaders: true,
},
wantsErr: false,
},
{
name: "no arguments",
cli: "",
wantsErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd := NewCmdApi(f, func(o *ApiOptions) error {
assert.Equal(t, tt.wants.RequestMethod, o.RequestMethod)
assert.Equal(t, tt.wants.RequestMethodPassed, o.RequestMethodPassed)
assert.Equal(t, tt.wants.RequestPath, o.RequestPath)
assert.Equal(t, tt.wants.RawFields, o.RawFields)
assert.Equal(t, tt.wants.MagicFields, o.MagicFields)
assert.Equal(t, tt.wants.RequestHeaders, o.RequestHeaders)
assert.Equal(t, tt.wants.ShowResponseHeaders, o.ShowResponseHeaders)
return nil
})
argv, err := shlex.Split(tt.cli)
assert.NoError(t, err)
cmd.SetArgs(argv)
_, err = cmd.ExecuteC()
if tt.wantsErr {
assert.Error(t, err)
return
}
assert.NoError(t, err)
})
}
}
func Test_apiRun(t *testing.T) {
tests := []struct {
name string
@ -30,6 +130,16 @@ func Test_apiRun(t *testing.T) {
stdout: `bam!`,
stderr: ``,
},
{
name: "success 204",
httpResponse: &http.Response{
StatusCode: 204,
Body: nil,
},
err: nil,
stdout: ``,
stderr: ``,
},
{
name: "failure",
httpResponse: &http.Response{
@ -109,7 +219,76 @@ func Test_parseFields(t *testing.T) {
"enabled": true,
"victories": 123,
}
if !reflect.DeepEqual(params, expect) {
t.Errorf("expected %v, got %v", expect, params)
assert.Equal(t, expect, params)
}
func Test_magicFieldValue(t *testing.T) {
f, err := ioutil.TempFile("", "gh-test")
if err != nil {
t.Fatal(err)
}
fmt.Fprint(f, "file contents")
f.Close()
t.Cleanup(func() { os.Remove(f.Name()) })
type args struct {
v string
stdin io.ReadCloser
}
tests := []struct {
name string
args args
want interface{}
wantErr bool
}{
{
name: "string",
args: args{v: "hello"},
want: "hello",
wantErr: false,
},
{
name: "bool true",
args: args{v: "true"},
want: true,
wantErr: false,
},
{
name: "bool false",
args: args{v: "false"},
want: false,
wantErr: false,
},
{
name: "null",
args: args{v: "null"},
want: nil,
wantErr: false,
},
{
name: "file",
args: args{v: "@" + f.Name()},
want: []byte("file contents"),
wantErr: false,
},
{
name: "file error",
args: args{v: "@"},
want: nil,
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := magicFieldValue(tt.args.v, tt.args.stdin)
if (err != nil) != tt.wantErr {
t.Errorf("magicFieldValue() error = %v, wantErr %v", err, tt.wantErr)
return
}
if tt.wantErr {
return
}
assert.Equal(t, tt.want, got)
})
}
}

View file

@ -4,8 +4,9 @@ import (
"bytes"
"io/ioutil"
"net/http"
"reflect"
"testing"
"github.com/stretchr/testify/assert"
)
func Test_groupGraphQLVariables(t *testing.T) {
@ -57,9 +58,8 @@ func Test_groupGraphQLVariables(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := groupGraphQLVariables(tt.args); !reflect.DeepEqual(got, tt.want) {
t.Errorf("groupGraphQLVariables() = %v, want %v", got, tt.want)
}
got := groupGraphQLVariables(tt.args)
assert.Equal(t, tt.want, got)
})
}
}

View file

@ -1,34 +0,0 @@
package api
import (
"fmt"
"net/http"
"os"
"strings"
"github.com/cli/cli/api"
"github.com/cli/cli/utils"
)
// TODO
func apiClientFromContext(token string) *http.Client {
var opts []api.ClientOption
if verbose := os.Getenv("DEBUG"); verbose != "" {
opts = append(opts, apiVerboseLog())
}
opts = append(opts,
api.AddHeader("Authorization", fmt.Sprintf("token %s", token)),
// FIXME
// api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", command.Version)),
// antiope-preview: Checks
api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"),
)
return api.NewHTTPClient(opts...)
}
// TODO
func apiVerboseLog() api.ClientOption {
logTraffic := strings.Contains(os.Getenv("DEBUG"), "api")
colorize := utils.IsTerminal(os.Stderr)
return api.VerboseLog(utils.NewColorable(os.Stderr), logTraffic, colorize)
}

12
pkg/cmdutil/factory.go Normal file
View file

@ -0,0 +1,12 @@
package cmdutil
import (
"net/http"
"github.com/cli/cli/pkg/iostreams"
)
type Factory struct {
IOStreams *iostreams.IOStreams
HttpClient func() (*http.Client, error)
}