Skip to content

Commit bef62fa

Browse files
mislavmislav
committed
Make NewCmdApi testable
1 parent f58e0bf commit bef62fa

File tree

8 files changed

+238
-59
lines changed

8 files changed

+238
-59
lines changed

‎command/root.go‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package command
33
import (
44
"fmt"
55
"io"
6+
"net/http"
67
"os"
78
"regexp"
89
"runtime/debug"
@@ -14,6 +15,7 @@ import (
1415
"github.com/cli/cli/internal/ghrepo"
1516
apiCmd "github.com/cli/cli/pkg/cmd/api"
1617
"github.com/cli/cli/pkg/cmdutil"
18+
"github.com/cli/cli/pkg/iostreams"
1719
"github.com/cli/cli/utils"
1820

1921
"github.com/spf13/cobra"
@@ -64,7 +66,19 @@ func init() {
6466
return &cmdutil.FlagError{Err: err}
6567
})
6668

67-
RootCmd.AddCommand(apiCmd.NewCmdApi())
69+
// TODO: iron out how a factory incorporates context
70+
cmdFactory := &cmdutil.Factory{
71+
IOStreams: iostreams.System(),
72+
HttpClient: func() (*http.Client, error) {
73+
ctx := context.New()
74+
token, err := ctx.AuthToken()
75+
if err != nil {
76+
return nil, err
77+
}
78+
return httpClient(token), nil
79+
},
80+
}
81+
RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil))
6882
}
6983

7084
// RootCmd is the entry point of command-line execution
@@ -119,6 +133,19 @@ func contextForCommand(cmd *cobra.Command) context.Context {
119133
return ctx
120134
}
121135

136+
// for cmdutil-powered commands
137+
func httpClient(token string) *http.Client {
138+
var opts []api.ClientOption
139+
if verbose := os.Getenv("DEBUG"); verbose != "" {
140+
opts = append(opts, apiVerboseLog())
141+
}
142+
opts = append(opts,
143+
api.AddHeader("Authorization", fmt.Sprintf("token %s", token)),
144+
api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)),
145+
)
146+
return api.NewHTTPClient(opts...)
147+
}
148+
122149
// overridden in tests
123150
var apiClientForContext = func(ctx context.Context) (*api.Client, error) {
124151
token, err := ctx.AuthToken()

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ require (
2121
github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f // indirect
2222
github.com/spf13/cobra v0.0.6
2323
github.com/spf13/pflag v1.0.5
24-
github.com/stretchr/testify v1.4.0 // indirect
24+
github.com/stretchr/testify v1.5.1
2525
golang.org/x/crypto v0.0.0-20200219234226-1ad67e1f0ef4
2626
golang.org/x/net v0.0.0-20200219183655-46282727080f // indirect
2727
golang.org/x/text v0.3.2

‎go.sum‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,14 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
167167
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
168168
github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE=
169169
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
170+
github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A=
170171
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
171172
github.com/stretchr/testify v1.2.1/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
172173
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
173174
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
174175
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
175-
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
176-
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
176+
github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4=
177+
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
177178
github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
178179
github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
179180
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=

‎pkg/cmd/api/api.go‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"strconv"
1010
"strings"
1111

12-
"github.com/cli/cli/context"
1312
"github.com/cli/cli/pkg/cmdutil"
1413
"github.com/cli/cli/pkg/iostreams"
1514
"github.com/spf13/cobra"
@@ -29,8 +28,12 @@ type ApiOptions struct {
2928
HttpClient func() (*http.Client, error)
3029
}
3130

32-
func NewCmdApi() *cobra.Command {
33-
opts := ApiOptions{}
31+
func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command {
32+
opts := ApiOptions{
33+
IO: f.IOStreams,
34+
HttpClient: f.HttpClient,
35+
}
36+
3437
cmd := &cobra.Command{
3538
Use: "api <endpoint>",
3639
Short: "Make an authenticated GitHub API request",
@@ -58,18 +61,9 @@ on the format of the value:
5861
opts.RequestPath = args[0]
5962
opts.RequestMethodPassed = c.Flags().Changed("method")
6063

61-
// TODO: pass in via caller
62-
opts.IO = iostreams.System()
63-
64-
opts.HttpClient = func() (*http.Client, error) {
65-
ctx := context.New()
66-
token, err := ctx.AuthToken()
67-
if err != nil {
68-
return nil, err
69-
}
70-
return apiClientFromContext(token), nil
64+
if runF != nil {
65+
return runF(&opts)
7166
}
72-
7367
return apiRun(&opts)
7468
},
7569
}

‎pkg/cmd/api/api_test.go‎

Lines changed: 182 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,115 @@ package api
33
import (
44
"bytes"
55
"fmt"
6+
"io"
67
"io/ioutil"
78
"net/http"
8-
"reflect"
9+
"os"
910
"testing"
1011

1112
"github.com/cli/cli/pkg/cmdutil"
1213
"github.com/cli/cli/pkg/iostreams"
14+
"github.com/google/shlex"
15+
"github.com/stretchr/testify/assert"
1316
)
1417

18+
func Test_NewCmdApi(t *testing.T) {
19+
f := &cmdutil.Factory{}
20+
21+
tests := []struct {
22+
name string
23+
cli string
24+
wants ApiOptions
25+
wantsErr bool
26+
}{
27+
{
28+
name: "no flags",
29+
cli: "graphql",
30+
wants: ApiOptions{
31+
RequestMethod: "GET",
32+
RequestMethodPassed: false,
33+
RequestPath: "graphql",
34+
RawFields: []string(nil),
35+
MagicFields: []string(nil),
36+
RequestHeaders: []string(nil),
37+
ShowResponseHeaders: false,
38+
},
39+
wantsErr: false,
40+
},
41+
{
42+
name: "override method",
43+
cli: "repos/octocat/Spoon-Knife -XDELETE",
44+
wants: ApiOptions{
45+
RequestMethod: "DELETE",
46+
RequestMethodPassed: true,
47+
RequestPath: "repos/octocat/Spoon-Knife",
48+
RawFields: []string(nil),
49+
MagicFields: []string(nil),
50+
RequestHeaders: []string(nil),
51+
ShowResponseHeaders: false,
52+
},
53+
wantsErr: false,
54+
},
55+
{
56+
name: "with fields",
57+
cli: "graphql -f query=QUERY -F body=@file.txt",
58+
wants: ApiOptions{
59+
RequestMethod: "GET",
60+
RequestMethodPassed: false,
61+
RequestPath: "graphql",
62+
RawFields: []string{"query=QUERY"},
63+
MagicFields: []string{"body=@file.txt"},
64+
RequestHeaders: []string(nil),
65+
ShowResponseHeaders: false,
66+
},
67+
wantsErr: false,
68+
},
69+
{
70+
name: "with headers",
71+
cli: "user -H 'accept: text/plain' -i",
72+
wants: ApiOptions{
73+
RequestMethod: "GET",
74+
RequestMethodPassed: false,
75+
RequestPath: "user",
76+
RawFields: []string(nil),
77+
MagicFields: []string(nil),
78+
RequestHeaders: []string{"accept: text/plain"},
79+
ShowResponseHeaders: true,
80+
},
81+
wantsErr: false,
82+
},
83+
{
84+
name: "no arguments",
85+
cli: "",
86+
wantsErr: true,
87+
},
88+
}
89+
for _, tt := range tests {
90+
t.Run(tt.name, func(t *testing.T) {
91+
cmd := NewCmdApi(f, func(o *ApiOptions) error {
92+
assert.Equal(t, tt.wants.RequestMethod, o.RequestMethod)
93+
assert.Equal(t, tt.wants.RequestMethodPassed, o.RequestMethodPassed)
94+
assert.Equal(t, tt.wants.RequestPath, o.RequestPath)
95+
assert.Equal(t, tt.wants.RawFields, o.RawFields)
96+
assert.Equal(t, tt.wants.MagicFields, o.MagicFields)
97+
assert.Equal(t, tt.wants.RequestHeaders, o.RequestHeaders)
98+
assert.Equal(t, tt.wants.ShowResponseHeaders, o.ShowResponseHeaders)
99+
return nil
100+
})
101+
102+
argv, err := shlex.Split(tt.cli)
103+
assert.NoError(t, err)
104+
cmd.SetArgs(argv)
105+
_, err = cmd.ExecuteC()
106+
if tt.wantsErr {
107+
assert.Error(t, err)
108+
return
109+
}
110+
assert.NoError(t, err)
111+
})
112+
}
113+
}
114+
15115
func Test_apiRun(t *testing.T) {
16116
tests := []struct {
17117
name string
@@ -30,6 +130,16 @@ func Test_apiRun(t *testing.T) {
30130
stdout: `bam!`,
31131
stderr: ``,
32132
},
133+
{
134+
name: "success 204",
135+
httpResponse: &http.Response{
136+
StatusCode: 204,
137+
Body: nil,
138+
},
139+
err: nil,
140+
stdout: ``,
141+
stderr: ``,
142+
},
33143
{
34144
name: "failure",
35145
httpResponse: &http.Response{
@@ -109,7 +219,76 @@ func Test_parseFields(t *testing.T) {
109219
"enabled": true,
110220
"victories": 123,
111221
}
112-
if !reflect.DeepEqual(params, expect) {
113-
t.Errorf("expected %v, got %v", expect, params)
222+
assert.Equal(t, expect, params)
223+
}
224+
225+
func Test_magicFieldValue(t *testing.T) {
226+
f, err := ioutil.TempFile("", "gh-test")
227+
if err != nil {
228+
t.Fatal(err)
229+
}
230+
fmt.Fprint(f, "file contents")
231+
f.Close()
232+
t.Cleanup(func() { os.Remove(f.Name()) })
233+
234+
type args struct {
235+
v string
236+
stdin io.ReadCloser
237+
}
238+
tests := []struct {
239+
name string
240+
args args
241+
want interface{}
242+
wantErr bool
243+
}{
244+
{
245+
name: "string",
246+
args: args{v: "hello"},
247+
want: "hello",
248+
wantErr: false,
249+
},
250+
{
251+
name: "bool true",
252+
args: args{v: "true"},
253+
want: true,
254+
wantErr: false,
255+
},
256+
{
257+
name: "bool false",
258+
args: args{v: "false"},
259+
want: false,
260+
wantErr: false,
261+
},
262+
{
263+
name: "null",
264+
args: args{v: "null"},
265+
want: nil,
266+
wantErr: false,
267+
},
268+
{
269+
name: "file",
270+
args: args{v: "@" + f.Name()},
271+
want: []byte("file contents"),
272+
wantErr: false,
273+
},
274+
{
275+
name: "file error",
276+
args: args{v: "@"},
277+
want: nil,
278+
wantErr: true,
279+
},
280+
}
281+
for _, tt := range tests {
282+
t.Run(tt.name, func(t *testing.T) {
283+
got, err := magicFieldValue(tt.args.v, tt.args.stdin)
284+
if (err != nil) != tt.wantErr {
285+
t.Errorf("magicFieldValue() error = %v, wantErr %v", err, tt.wantErr)
286+
return
287+
}
288+
if tt.wantErr {
289+
return
290+
}
291+
assert.Equal(t, tt.want, got)
292+
})
114293
}
115294
}

‎pkg/cmd/api/http_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ import (
44
"bytes"
55
"io/ioutil"
66
"net/http"
7-
"reflect"
87
"testing"
8+
9+
"github.com/stretchr/testify/assert"
910
)
1011

1112
func Test_groupGraphQLVariables(t *testing.T) {
@@ -57,9 +58,8 @@ func Test_groupGraphQLVariables(t *testing.T) {
5758
}
5859
for _, tt := range tests {
5960
t.Run(tt.name, func(t *testing.T) {
60-
if got := groupGraphQLVariables(tt.args); !reflect.DeepEqual(got, tt.want) {
61-
t.Errorf("groupGraphQLVariables() = %v, want %v", got, tt.want)
62-
}
61+
got := groupGraphQLVariables(tt.args)
62+
assert.Equal(t, tt.want, got)
6363
})
6464
}
6565
}

‎pkg/cmd/api/legacy.go‎

Lines changed: 0 additions & 34 deletions
This file was deleted.

0 commit comments

Comments
 (0)