From e9e3639cc707272f02d5a65faab39fa48b2b33fa Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Fri, 29 Apr 2022 17:08:00 +0200 Subject: [PATCH 1/3] Migrate from httpmock to gock --- gh_test.go | 51 +++++----- go.mod | 1 + go.sum | 5 + internal/api/gql_client_test.go | 89 ++++++++++------ internal/api/rest_client_test.go | 166 +++++++++++++++++------------- internal/httpmock/registry.go | 77 -------------- internal/httpmock/stub.go | 168 ------------------------------- 7 files changed, 186 insertions(+), 371 deletions(-) delete mode 100644 internal/httpmock/registry.go delete mode 100644 internal/httpmock/stub.go diff --git a/gh_test.go b/gh_test.go index e0d3cea..006b1fd 100644 --- a/gh_test.go +++ b/gh_test.go @@ -6,9 +6,9 @@ import ( "testing" "github.com/cli/go-gh/internal/config" - "github.com/cli/go-gh/internal/httpmock" "github.com/cli/go-gh/pkg/api" "github.com/stretchr/testify/assert" + "gopkg.in/h2non/gock.v1" ) func TestHelperProcess(t *testing.T) { @@ -47,6 +47,7 @@ func TestRunError(t *testing.T) { } func TestRESTClient(t *testing.T) { + t.Cleanup(gock.Off) tempDir := t.TempDir() orig_GH_CONFIG_DIR := os.Getenv("GH_CONFIG_DIR") orig_GH_TOKEN := os.Getenv("GH_TOKEN") @@ -57,24 +58,24 @@ func TestRESTClient(t *testing.T) { os.Setenv("GH_CONFIG_DIR", tempDir) os.Setenv("GH_TOKEN", "GH_TOKEN") - http := httpmock.NewRegistry(t) - http.Register( - httpmock.REST("GET", "some/test/path"), - httpmock.StatusStringResponse(200, `{"message": "success"}`), - ) + gock.New("https://api.github.com"). + Get("/some/test/path"). + MatchHeader("Authorization", "token GH_TOKEN"). + Reply(200). + JSON(`{"message": "success"}`) - client, err := RESTClient(&api.ClientOptions{Transport: http}) + client, err := RESTClient(nil) assert.NoError(t, err) res := struct{ Message string }{} err = client.Do("GET", "some/test/path", nil, &res) assert.NoError(t, err) + assert.True(t, gock.IsDone()) assert.Equal(t, "success", res.Message) - assert.Equal(t, "api.github.com", http.Requests[0].URL.Hostname()) - assert.Equal(t, "token GH_TOKEN", http.Requests[0].Header.Get("Authorization")) } func TestGQLClient(t *testing.T) { + t.Cleanup(gock.Off) tempDir := t.TempDir() orig_GH_CONFIG_DIR := os.Getenv("GH_CONFIG_DIR") orig_GH_TOKEN := os.Getenv("GH_TOKEN") @@ -85,25 +86,26 @@ func TestGQLClient(t *testing.T) { os.Setenv("GH_CONFIG_DIR", tempDir) os.Setenv("GH_TOKEN", "GH_TOKEN") - http := httpmock.NewRegistry(t) - http.Register( - httpmock.GQL("QUERY"), - httpmock.StringResponse(`{"data":{"viewer":{"login":"hubot"}}}`), - ) + gock.New("https://api.github.com"). + Post("/graphql"). + MatchHeader("Authorization", "token GH_TOKEN"). + BodyString("QUERY"). + Reply(200). + JSON(`{"data":{"viewer":{"login":"hubot"}}}`) - client, err := GQLClient(&api.ClientOptions{Transport: http}) + client, err := GQLClient(nil) assert.NoError(t, err) vars := map[string]interface{}{"var": "test"} res := struct{ Viewer struct{ Login string } }{} err = client.Do("QUERY", vars, &res) assert.NoError(t, err) + assert.True(t, gock.IsDone()) assert.Equal(t, "hubot", res.Viewer.Login) - assert.Equal(t, "api.github.com", http.Requests[0].URL.Hostname()) - assert.Equal(t, "token GH_TOKEN", http.Requests[0].Header.Get("Authorization")) } func TestHTTPClient(t *testing.T) { + t.Cleanup(gock.Off) tempDir := t.TempDir() orig_GH_CONFIG_DIR := os.Getenv("GH_CONFIG_DIR") orig_GH_TOKEN := os.Getenv("GH_TOKEN") @@ -114,20 +116,19 @@ func TestHTTPClient(t *testing.T) { os.Setenv("GH_CONFIG_DIR", tempDir) os.Setenv("GH_TOKEN", "GH_TOKEN") - http := httpmock.NewRegistry(t) - http.Register( - httpmock.REST("GET", "some/test/path"), - httpmock.StatusStringResponse(200, `{"message": "success"}`), - ) + gock.New("https://api.github.com"). + Get("/some/test/path"). + MatchHeader("Authorization", "token GH_TOKEN"). + Reply(200). + JSON(`{"message": "success"}`) - client, err := HTTPClient(&api.ClientOptions{Transport: http}) + client, err := HTTPClient(nil) assert.NoError(t, err) res, err := client.Get("https://api.github.com/some/test/path") assert.NoError(t, err) + assert.True(t, gock.IsDone()) assert.Equal(t, 200, res.StatusCode) - assert.Equal(t, "api.github.com", http.Requests[0].URL.Hostname()) - assert.Equal(t, "token GH_TOKEN", http.Requests[0].Header.Get("Authorization")) } func TestResolveOptions(t *testing.T) { diff --git a/go.mod b/go.mod index ad0c182..c225a39 100644 --- a/go.mod +++ b/go.mod @@ -10,5 +10,6 @@ require ( github.com/kr/pretty v0.1.0 // indirect github.com/stretchr/testify v1.7.0 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect + gopkg.in/h2non/gock.v1 v1.1.2 // indirect gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b ) diff --git a/go.sum b/go.sum index f50fc75..44b9995 100644 --- a/go.sum +++ b/go.sum @@ -6,6 +6,8 @@ github.com/cli/shurcooL-graphql v0.0.1 h1:/9J3t9O6p1B8zdBBtQighq5g7DQRItBwuwGh3S github.com/cli/shurcooL-graphql v0.0.1/go.mod h1:U7gCSuMZP/Qy7kbqkk5PrqXEeDgtfG5K+W+u8weorps= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 h1:2VTzZjLZBgl62/EtslCrtky5vbi9dd7HrQPQIx6wqiw= +github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542/go.mod h1:Ow0tF8D4Kplbc8s8sSb3V2oUCygFHVp8gC3Dn6U4MNI= github.com/henvic/httpretty v0.0.6 h1:JdzGzKZBajBfnvlMALXXMVQWxWMF/ofTy8C3/OSUTxs= github.com/henvic/httpretty v0.0.6/go.mod h1:X38wLjWXHkXT7r2+uK8LjCMne9rsuNaBLJ+5cU2/Pmo= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= @@ -13,6 +15,7 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32/go.mod h1:9wM+0iRr9ahx58uYLpLIr5fm8diHn0JbqRycJi6w0Ms= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -28,6 +31,8 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/h2non/gock.v1 v1.1.2 h1:jBbHXgGBK/AoPVfJh5x4r/WxIrElvbLel8TCZkkZJoY= +gopkg.in/h2non/gock.v1 v1.1.2/go.mod h1:n7UGz/ckNChHiK05rDoiC4MYSunEC/lyaUm2WWaDva0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/api/gql_client_test.go b/internal/api/gql_client_test.go index fbcccfe..ad2e706 100644 --- a/internal/api/gql_client_test.go +++ b/internal/api/gql_client_test.go @@ -3,83 +3,112 @@ package api import ( "testing" - "github.com/cli/go-gh/internal/httpmock" - "github.com/cli/go-gh/pkg/api" "github.com/stretchr/testify/assert" + "gopkg.in/h2non/gock.v1" ) func TestGQLClientDo(t *testing.T) { tests := []struct { name string host string - matcher httpmock.Matcher - responder httpmock.Responder + httpMocks func() wantErr bool wantErrMsg string - wantHost string wantLogin string }{ { - name: "success request", - responder: httpmock.StringResponse(`{"data":{"viewer":{"login":"hubot"}}}`), + name: "success request", + httpMocks: func() { + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString("QUERY"). + Reply(200). + JSON(`{"data":{"viewer":{"login":"hubot"}}}`) + }, wantLogin: "hubot", }, { - name: "fail request", - responder: httpmock.StringResponse(`{"errors":[{"message":"OH NO"},{"message":"this is fine"}]}`), + name: "fail request", + httpMocks: func() { + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString("QUERY"). + Reply(200). + JSON(`{"errors":[{"message":"OH NO"},{"message":"this is fine"}]}`) + }, wantErr: true, wantErrMsg: "GQL error: OH NO\nthis is fine", }, { - name: "http fail request empty response", - responder: httpmock.StatusStringResponse(404, `{}`), + name: "http fail request empty response", + httpMocks: func() { + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString("QUERY"). + Reply(404). + JSON(`{}`) + }, wantErr: true, wantErrMsg: "HTTP 404 (https://api.github.com/graphql)", }, { - name: "http fail request message response", - responder: httpmock.StatusStringResponse(422, `{"message": "OH NO"}`), + name: "http fail request message response", + httpMocks: func() { + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString("QUERY"). + Reply(422). + JSON(`{"message": "OH NO"}`) + }, wantErr: true, wantErrMsg: "HTTP 422: OH NO (https://api.github.com/graphql)", }, { - name: "http fail request errors response", - responder: httpmock.StatusStringResponse(502, `{"errors":[{"message":"Something went wrong"}]}`), + name: "http fail request errors response", + httpMocks: func() { + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString("QUERY"). + Reply(502). + JSON(`{"errors":[{"message":"Something went wrong"}]}`) + }, wantErr: true, wantErrMsg: "HTTP 502: Something went wrong (https://api.github.com/graphql)", }, { - name: "support enterprise hosts", - responder: httpmock.StatusStringResponse(204, "{}"), - host: "enterprise.com", - wantHost: "enterprise.com", + name: "support enterprise hosts", + host: "enterprise.com", + httpMocks: func() { + gock.New("https://enterprise.com"). + Post("/api/graphql"). + BodyString("QUERY"). + Reply(200). + JSON(`{"data":{"viewer":{"login":"hubot"}}}`) + }, + wantLogin: "hubot", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Cleanup(gock.Off) if tt.host == "" { tt.host = "github.com" } - if tt.wantHost == "" { - tt.wantHost = "api.github.com" + if tt.httpMocks != nil { + tt.httpMocks() } - http := httpmock.NewRegistry(t) - client := NewGQLClient(tt.host, &api.ClientOptions{Transport: http}) - http.Register( - httpmock.GQL("QUERY"), - tt.responder, - ) + client := NewGQLClient(tt.host, nil) vars := map[string]interface{}{"var": "test"} res := struct{ Viewer struct{ Login string } }{} err := client.Do("QUERY", vars, &res) if tt.wantErr { assert.EqualError(t, err, tt.wantErrMsg) - return + } else { + assert.NoError(t, err) } - assert.NoError(t, err) + assert.True(t, gock.IsDone()) assert.Equal(t, tt.wantLogin, res.Viewer.Login) - assert.Equal(t, tt.wantHost, http.Requests[0].URL.Hostname()) }) } } diff --git a/internal/api/rest_client_test.go b/internal/api/rest_client_test.go index 246fa94..31fe873 100644 --- a/internal/api/rest_client_test.go +++ b/internal/api/rest_client_test.go @@ -4,9 +4,8 @@ import ( "bytes" "testing" - "github.com/cli/go-gh/internal/httpmock" - "github.com/cli/go-gh/pkg/api" "github.com/stretchr/testify/assert" + "gopkg.in/h2non/gock.v1" ) func TestRESTClientDo(t *testing.T) { @@ -14,139 +13,164 @@ func TestRESTClientDo(t *testing.T) { name string host string path string - matcher httpmock.Matcher - responder httpmock.Responder + httpMocks func() wantErr bool wantErrMsg string - wantHost string wantMsg string }{ { name: "success request empty response", path: "some/test/path", + httpMocks: func() { + gock.New("https://api.github.com"). + Get("/some/test/path"). + Reply(204). + JSON(`{}`) + }, }, { - name: "success request non-empty response", - path: "some/test/path", - responder: httpmock.StatusStringResponse(200, `{"message": "success"}`), - wantMsg: "success", + name: "success request non-empty response", + path: "some/test/path", + httpMocks: func() { + gock.New("https://api.github.com"). + Get("/some/test/path"). + Reply(200). + JSON(`{"message": "success"}`) + }, + wantMsg: "success", }, { - name: "fail request empty response", - path: "some/test/path", - responder: httpmock.StatusStringResponse(404, `{}`), + name: "fail request empty response", + path: "some/test/path", + httpMocks: func() { + gock.New("https://api.github.com"). + Get("/some/test/path"). + Reply(404). + JSON(`{}`) + }, wantErr: true, wantErrMsg: "HTTP 404 (https://api.github.com/some/test/path)", }, { - name: "fail request non-empty response", - path: "some/test/path", - responder: httpmock.StatusStringResponse(422, `{"message": "OH NO"}`), + name: "fail request non-empty response", + path: "some/test/path", + httpMocks: func() { + gock.New("https://api.github.com"). + Get("/some/test/path"). + Reply(422). + JSON(`{"message": "OH NO"}`) + }, wantErr: true, wantErrMsg: "HTTP 422: OH NO (https://api.github.com/some/test/path)", }, { - name: "support full urls", - path: "https://example.com/someother/test/path", - matcher: httpmock.REST("GET", "someother/test/path"), - wantHost: "example.com", + name: "support full urls", + path: "https://example.com/someother/test/path", + httpMocks: func() { + gock.New("https://example.com"). + Get("/someother/test/path"). + Reply(204). + JSON(`{}`) + }, }, { - name: "support enterprise hosts", - host: "enterprise.com", - path: "some/test/path", - matcher: httpmock.REST("GET", "api/v3/some/test/path"), - wantHost: "enterprise.com", + name: "support enterprise hosts", + host: "enterprise.com", + path: "some/test/path", + httpMocks: func() { + gock.New("https://enterprise.com"). + Get("/some/test/path"). + Reply(204). + JSON(`{}`) + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Cleanup(gock.Off) if tt.host == "" { tt.host = "github.com" } - if tt.wantHost == "" { - tt.wantHost = "api.github.com" - } - if tt.matcher == nil { - tt.matcher = httpmock.REST("GET", "some/test/path") - } - if tt.responder == nil { - tt.responder = httpmock.StatusStringResponse(204, "{}") + if tt.httpMocks != nil { + tt.httpMocks() } - http := httpmock.NewRegistry(t) - client := NewRESTClient(tt.host, &api.ClientOptions{Transport: http}) - http.Register( - tt.matcher, - tt.responder, - ) + client := NewRESTClient(tt.host, nil) res := struct{ Message string }{} err := client.Do("GET", tt.path, nil, &res) if tt.wantErr { assert.EqualError(t, err, tt.wantErrMsg) - return + } else { + assert.NoError(t, err) } - assert.NoError(t, err) + assert.True(t, gock.IsDone()) assert.Equal(t, tt.wantMsg, res.Message) - assert.Equal(t, tt.wantHost, http.Requests[0].URL.Hostname()) }) } } func TestRESTClientDelete(t *testing.T) { - http := httpmock.NewRegistry(t) - client := NewRESTClient("github.com", &api.ClientOptions{Transport: http}) - http.Register( - httpmock.REST("DELETE", "some/path/here"), - httpmock.StatusStringResponse(204, "{}"), - ) + t.Cleanup(gock.Off) + gock.New("https://api.github.com"). + Delete("/some/path/here"). + Reply(204). + JSON(`{}`) + client := NewRESTClient("github.com", nil) err := client.Delete("some/path/here", nil) assert.NoError(t, err) + assert.True(t, gock.IsDone()) } func TestRESTClientGet(t *testing.T) { - http := httpmock.NewRegistry(t) - client := NewRESTClient("github.com", &api.ClientOptions{Transport: http}) - http.Register( - httpmock.REST("GET", "some/path/here"), - httpmock.StatusStringResponse(204, "{}"), - ) + t.Cleanup(gock.Off) + gock.New("https://api.github.com"). + Get("/some/path/here"). + Reply(204). + JSON(`{}`) + client := NewRESTClient("github.com", nil) err := client.Get("some/path/here", nil) assert.NoError(t, err) + assert.True(t, gock.IsDone()) } func TestRESTClientPatch(t *testing.T) { - http := httpmock.NewRegistry(t) - client := NewRESTClient("github.com", &api.ClientOptions{Transport: http}) - http.Register( - httpmock.REST("PATCH", "some/path/here"), - httpmock.StatusStringResponse(204, "{}"), - ) + t.Cleanup(gock.Off) + gock.New("https://api.github.com"). + Patch("/some/path/here"). + BodyString(`{}`). + Reply(204). + JSON(`{}`) + client := NewRESTClient("github.com", nil) r := bytes.NewReader([]byte(`{}`)) err := client.Patch("some/path/here", r, nil) assert.NoError(t, err) + assert.True(t, gock.IsDone()) } func TestRESTClientPost(t *testing.T) { - http := httpmock.NewRegistry(t) - client := NewRESTClient("github.com", &api.ClientOptions{Transport: http}) - http.Register( - httpmock.REST("POST", "some/path/here"), - httpmock.StatusStringResponse(204, "{}"), - ) + t.Cleanup(gock.Off) + gock.New("https://api.github.com"). + Post("/some/path/here"). + BodyString(`{}`). + Reply(204). + JSON(`{}`) + client := NewRESTClient("github.com", nil) r := bytes.NewReader([]byte(`{}`)) err := client.Post("some/path/here", r, nil) assert.NoError(t, err) + assert.True(t, gock.IsDone()) } func TestRESTClientPut(t *testing.T) { - http := httpmock.NewRegistry(t) - client := NewRESTClient("github.com", &api.ClientOptions{Transport: http}) - http.Register( - httpmock.REST("PUT", "some/path/here"), - httpmock.StatusStringResponse(204, "{}"), - ) + t.Cleanup(gock.Off) + gock.New("https://api.github.com"). + Put("/some/path/here"). + BodyString(`{}`). + Reply(204). + JSON(`{}`) + client := NewRESTClient("github.com", nil) r := bytes.NewReader([]byte(`{}`)) err := client.Put("some/path/here", r, nil) assert.NoError(t, err) + assert.True(t, gock.IsDone()) } diff --git a/internal/httpmock/registry.go b/internal/httpmock/registry.go deleted file mode 100644 index 43277ef..0000000 --- a/internal/httpmock/registry.go +++ /dev/null @@ -1,77 +0,0 @@ -package httpmock - -import ( - "fmt" - "net/http" - "sync" - "testing" -) - -type Registry struct { - mu sync.Mutex - stubs []*Stub - Requests []*http.Request -} - -func NewRegistry(t *testing.T) *Registry { - reg := Registry{} - t.Cleanup(func() { reg.Verify(t) }) - return ® -} - -func (r *Registry) Register(m Matcher, resp Responder) { - r.stubs = append(r.stubs, &Stub{ - Matcher: m, - Responder: resp, - }) -} - -type Testing interface { - Errorf(string, ...interface{}) - Helper() -} - -func (r *Registry) Verify(t Testing) { - n := 0 - for _, s := range r.stubs { - if !s.matched { - n++ - } - } - if n > 0 { - t.Helper() - // NOTE: Stubs offer no useful reflection, so we can't print details - // about dead stubs and what they were trying to match. - t.Errorf("%d unmatched HTTP stubs", n) - } -} - -// Registry satisfies http.RoundTripper interface. -func (r *Registry) RoundTrip(req *http.Request) (*http.Response, error) { - var stub *Stub - - r.mu.Lock() - for _, s := range r.stubs { - if s.matched || !s.Matcher(req) { - continue - } - if stub != nil { - r.mu.Unlock() - return nil, fmt.Errorf("more than 1 stub matched %v", req) - } - stub = s - } - if stub != nil { - stub.matched = true - } - - if stub == nil { - r.mu.Unlock() - return nil, fmt.Errorf("no registered stubs matched %v", req) - } - - r.Requests = append(r.Requests, req) - r.mu.Unlock() - - return stub.Responder(req) -} diff --git a/internal/httpmock/stub.go b/internal/httpmock/stub.go deleted file mode 100644 index e15a995..0000000 --- a/internal/httpmock/stub.go +++ /dev/null @@ -1,168 +0,0 @@ -package httpmock - -import ( - "bytes" - "encoding/json" - "io" - "net/http" - "os" - "regexp" - "strings" -) - -type Matcher func(req *http.Request) bool -type Responder func(req *http.Request) (*http.Response, error) - -type Stub struct { - matched bool - Matcher Matcher - Responder Responder -} - -func MatchAny(*http.Request) bool { - return true -} - -func REST(method, p string) Matcher { - return func(req *http.Request) bool { - if !strings.EqualFold(req.Method, method) { - return false - } - if req.URL.Path != "/"+p { - return false - } - return true - } -} - -func GQL(q string) Matcher { - re := regexp.MustCompile(q) - - return func(req *http.Request) bool { - if !strings.EqualFold(req.Method, "POST") { - return false - } - if req.URL.Path != "/graphql" && req.URL.Path != "/api/graphql" { - return false - } - - var bodyData struct { - Query string - } - _ = decodeJSONBody(req, &bodyData) - - return re.MatchString(bodyData.Query) - } -} - -func readBody(req *http.Request) ([]byte, error) { - bodyCopy := &bytes.Buffer{} - r := io.TeeReader(req.Body, bodyCopy) - req.Body = io.NopCloser(bodyCopy) - return io.ReadAll(r) -} - -func decodeJSONBody(req *http.Request, dest interface{}) error { - b, err := readBody(req) - if err != nil { - return err - } - return json.Unmarshal(b, dest) -} - -func StringResponse(body string) Responder { - return func(req *http.Request) (*http.Response, error) { - return httpResponse(200, req, bytes.NewBufferString(body)), nil - } -} - -func StatusStringResponse(status int, body string) Responder { - return func(req *http.Request) (*http.Response, error) { - return httpResponse(status, req, bytes.NewBufferString(body)), nil - } -} - -func JSONResponse(body interface{}) Responder { - return func(req *http.Request) (*http.Response, error) { - b, _ := json.Marshal(body) - return httpResponse(200, req, bytes.NewBuffer(b)), nil - } -} - -func FileResponse(filename string) Responder { - return func(req *http.Request) (*http.Response, error) { - f, err := os.Open(filename) - if err != nil { - return nil, err - } - return httpResponse(200, req, f), nil - } -} - -func RESTPayload(responseStatus int, responseBody string, cb func(payload map[string]interface{})) Responder { - return func(req *http.Request) (*http.Response, error) { - bodyData := make(map[string]interface{}) - err := decodeJSONBody(req, &bodyData) - if err != nil { - return nil, err - } - cb(bodyData) - return httpResponse(responseStatus, req, bytes.NewBufferString(responseBody)), nil - } -} -func GQLMutation(body string, cb func(map[string]interface{})) Responder { - return func(req *http.Request) (*http.Response, error) { - var bodyData struct { - Variables struct { - Input map[string]interface{} - } - } - err := decodeJSONBody(req, &bodyData) - if err != nil { - return nil, err - } - cb(bodyData.Variables.Input) - - return httpResponse(200, req, bytes.NewBufferString(body)), nil - } -} - -func GQLQuery(body string, cb func(string, map[string]interface{})) Responder { - return func(req *http.Request) (*http.Response, error) { - var bodyData struct { - Query string - Variables map[string]interface{} - } - err := decodeJSONBody(req, &bodyData) - if err != nil { - return nil, err - } - cb(bodyData.Query, bodyData.Variables) - - return httpResponse(200, req, bytes.NewBufferString(body)), nil - } -} - -func ScopesResponder(scopes string) func(*http.Request) (*http.Response, error) { - return func(req *http.Request) (*http.Response, error) { - return &http.Response{ - StatusCode: 200, - Request: req, - Header: map[string][]string{ - "X-Oauth-Scopes": {scopes}, - }, - Body: io.NopCloser(bytes.NewBufferString("")), - }, nil - } -} - -func httpResponse(status int, req *http.Request, body io.Reader) *http.Response { - return &http.Response{ - StatusCode: status, - Request: req, - Header: map[string][]string{ - "Content-Type": {"application/json; charset=utf-8"}, - }, - Body: io.NopCloser(body), - } -} From 95a3e162680670e3fb09f6c9c87b69a96a916627 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Fri, 6 May 2022 15:28:08 +0200 Subject: [PATCH 2/3] Match on full body string --- gh_test.go | 2 +- internal/api/gql_client_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gh_test.go b/gh_test.go index 006b1fd..9882dbb 100644 --- a/gh_test.go +++ b/gh_test.go @@ -89,7 +89,7 @@ func TestGQLClient(t *testing.T) { gock.New("https://api.github.com"). Post("/graphql"). MatchHeader("Authorization", "token GH_TOKEN"). - BodyString("QUERY"). + BodyString(`{"query":"QUERY","variables":{"var":"test"}}`). Reply(200). JSON(`{"data":{"viewer":{"login":"hubot"}}}`) diff --git a/internal/api/gql_client_test.go b/internal/api/gql_client_test.go index ad2e706..be78356 100644 --- a/internal/api/gql_client_test.go +++ b/internal/api/gql_client_test.go @@ -21,7 +21,7 @@ func TestGQLClientDo(t *testing.T) { httpMocks: func() { gock.New("https://api.github.com"). Post("/graphql"). - BodyString("QUERY"). + BodyString(`{"query":"QUERY","variables":{"var":"test"}}`). Reply(200). JSON(`{"data":{"viewer":{"login":"hubot"}}}`) }, @@ -32,7 +32,7 @@ func TestGQLClientDo(t *testing.T) { httpMocks: func() { gock.New("https://api.github.com"). Post("/graphql"). - BodyString("QUERY"). + BodyString(`{"query":"QUERY","variables":{"var":"test"}}`). Reply(200). JSON(`{"errors":[{"message":"OH NO"},{"message":"this is fine"}]}`) }, @@ -44,7 +44,7 @@ func TestGQLClientDo(t *testing.T) { httpMocks: func() { gock.New("https://api.github.com"). Post("/graphql"). - BodyString("QUERY"). + BodyString(`{"query":"QUERY","variables":{"var":"test"}}`). Reply(404). JSON(`{}`) }, @@ -56,7 +56,7 @@ func TestGQLClientDo(t *testing.T) { httpMocks: func() { gock.New("https://api.github.com"). Post("/graphql"). - BodyString("QUERY"). + BodyString(`{"query":"QUERY","variables":{"var":"test"}}`). Reply(422). JSON(`{"message": "OH NO"}`) }, @@ -68,7 +68,7 @@ func TestGQLClientDo(t *testing.T) { httpMocks: func() { gock.New("https://api.github.com"). Post("/graphql"). - BodyString("QUERY"). + BodyString(`{"query":"QUERY","variables":{"var":"test"}}`). Reply(502). JSON(`{"errors":[{"message":"Something went wrong"}]}`) }, @@ -81,7 +81,7 @@ func TestGQLClientDo(t *testing.T) { httpMocks: func() { gock.New("https://enterprise.com"). Post("/api/graphql"). - BodyString("QUERY"). + BodyString(`{"query":"QUERY","variables":{"var":"test"}}`). Reply(200). JSON(`{"data":{"viewer":{"login":"hubot"}}}`) }, From 168a8f83f007e5e053265db41fcd6d69176d8518 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Sat, 7 May 2022 11:48:53 +0200 Subject: [PATCH 3/3] Better error messaging on test failure --- gh_test.go | 15 ++++++++++++--- internal/api/gql_client_test.go | 2 +- internal/api/rest_client_test.go | 22 ++++++++++++++++------ 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/gh_test.go b/gh_test.go index 9882dbb..ade5f1b 100644 --- a/gh_test.go +++ b/gh_test.go @@ -3,6 +3,7 @@ package gh import ( "fmt" "os" + "strings" "testing" "github.com/cli/go-gh/internal/config" @@ -70,7 +71,7 @@ func TestRESTClient(t *testing.T) { res := struct{ Message string }{} err = client.Do("GET", "some/test/path", nil, &res) assert.NoError(t, err) - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) assert.Equal(t, "success", res.Message) } @@ -100,7 +101,7 @@ func TestGQLClient(t *testing.T) { res := struct{ Viewer struct{ Login string } }{} err = client.Do("QUERY", vars, &res) assert.NoError(t, err) - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) assert.Equal(t, "hubot", res.Viewer.Login) } @@ -127,7 +128,7 @@ func TestHTTPClient(t *testing.T) { res, err := client.Get("https://api.github.com/some/test/path") assert.NoError(t, err) - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) assert.Equal(t, 200, res.StatusCode) } @@ -178,3 +179,11 @@ hosts: cfg, _ := config.FromString(data) return cfg } + +func printPendingMocks(mocks []gock.Mock) string { + paths := []string{} + for _, mock := range mocks { + paths = append(paths, mock.Request().URLStruct.String()) + } + return fmt.Sprintf("%d unmatched mocks: %s", len(paths), strings.Join(paths, ", ")) +} diff --git a/internal/api/gql_client_test.go b/internal/api/gql_client_test.go index be78356..8400004 100644 --- a/internal/api/gql_client_test.go +++ b/internal/api/gql_client_test.go @@ -107,7 +107,7 @@ func TestGQLClientDo(t *testing.T) { } else { assert.NoError(t, err) } - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) assert.Equal(t, tt.wantLogin, res.Viewer.Login) }) } diff --git a/internal/api/rest_client_test.go b/internal/api/rest_client_test.go index 31fe873..48fa149 100644 --- a/internal/api/rest_client_test.go +++ b/internal/api/rest_client_test.go @@ -2,6 +2,8 @@ package api import ( "bytes" + "fmt" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -103,7 +105,7 @@ func TestRESTClientDo(t *testing.T) { } else { assert.NoError(t, err) } - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) assert.Equal(t, tt.wantMsg, res.Message) }) } @@ -118,7 +120,7 @@ func TestRESTClientDelete(t *testing.T) { client := NewRESTClient("github.com", nil) err := client.Delete("some/path/here", nil) assert.NoError(t, err) - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) } func TestRESTClientGet(t *testing.T) { @@ -130,7 +132,7 @@ func TestRESTClientGet(t *testing.T) { client := NewRESTClient("github.com", nil) err := client.Get("some/path/here", nil) assert.NoError(t, err) - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) } func TestRESTClientPatch(t *testing.T) { @@ -144,7 +146,7 @@ func TestRESTClientPatch(t *testing.T) { r := bytes.NewReader([]byte(`{}`)) err := client.Patch("some/path/here", r, nil) assert.NoError(t, err) - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) } func TestRESTClientPost(t *testing.T) { @@ -158,7 +160,7 @@ func TestRESTClientPost(t *testing.T) { r := bytes.NewReader([]byte(`{}`)) err := client.Post("some/path/here", r, nil) assert.NoError(t, err) - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) } func TestRESTClientPut(t *testing.T) { @@ -172,5 +174,13 @@ func TestRESTClientPut(t *testing.T) { r := bytes.NewReader([]byte(`{}`)) err := client.Put("some/path/here", r, nil) assert.NoError(t, err) - assert.True(t, gock.IsDone()) + assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending())) +} + +func printPendingMocks(mocks []gock.Mock) string { + paths := []string{} + for _, mock := range mocks { + paths = append(paths, mock.Request().URLStruct.String()) + } + return fmt.Sprintf("%d unmatched mocks: %s", len(paths), strings.Join(paths, ", ")) }