From 1d4065e4c48f824978fb63869dc277d9ca2337dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 6 May 2020 22:03:32 +0200 Subject: [PATCH 1/3] Add a HTTP mocking interface that matches incoming requests This adds a thread-safe RoundTripper that can be used for mocking HTTP requests in tests. Incoming requests are matched based on their contents, not the order the stubs were registered in. --- pkg/httpmock/httpmock.go | 148 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 pkg/httpmock/httpmock.go diff --git a/pkg/httpmock/httpmock.go b/pkg/httpmock/httpmock.go new file mode 100644 index 000000000..9b34b9063 --- /dev/null +++ b/pkg/httpmock/httpmock.go @@ -0,0 +1,148 @@ +package httpmock + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "io/ioutil" + "net/http" + "regexp" + "strings" + "sync" +) + +type Registry struct { + mu sync.Mutex + stubs []*Stub +} + +func (r *Registry) Register(m Matcher, resp Responder) { + r.stubs = append(r.stubs, &Stub{ + Matcher: m, + Responder: resp, + }) +} + +type Testing interface { + Errorf(string, ...interface{}) +} + +func (r *Registry) Verify(t Testing) { + n := 0 + for _, s := range r.stubs { + if !s.matched { + n++ + } + } + if n > 0 { + t.Errorf("%d unmatched HTTP stubs", n) + } +} + +// RoundTrip satisfies http.RoundTripper +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 + } + r.mu.Unlock() + + if stub == nil { + return nil, fmt.Errorf("no registered stubs matched %v", req) + } + return stub.Responder(req) +} + +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 GraphQL(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" { + 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 = ioutil.NopCloser(bodyCopy) + return ioutil.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(*http.Request) (*http.Response, error) { + return httpResponse(200, bytes.NewBufferString(body)), nil + } +} + +func JSONResponse(body interface{}) Responder { + return func(*http.Request) (*http.Response, error) { + b, _ := json.Marshal(body) + return httpResponse(200, bytes.NewBuffer(b)), nil + } +} + +func GraphQLMutation(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, bytes.NewBufferString(body)), nil + } +} + +func httpResponse(status int, body io.Reader) *http.Response { + return &http.Response{ + StatusCode: status, + Body: ioutil.NopCloser(body), + } +} From 256d31950a6caa3f8b7d72223176e6a2f43adb0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 6 May 2020 22:06:37 +0200 Subject: [PATCH 2/3] Migrate a single test over to httpmock to demonstrate its use --- api/queries_repo.go | 14 ++++++++++++ command/pr_create_test.go | 48 ++++++++++++++------------------------- command/testing.go | 9 ++++++++ 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index b8ad760d0..c415aeec7 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -221,6 +221,20 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e return result, nil } +func RepoNetworkStubResponse(owner, repo, permission string) string { + return fmt.Sprintf(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "master" + }, + "viewerPermission": "%s" + } } } + `, repo, owner, permission) +} + // repositoryV3 is the repository result from GitHub API v3 type repositoryV3 struct { NodeID string diff --git a/command/pr_create_test.go b/command/pr_create_test.go index c84a8ab0c..7445f9a68 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -7,8 +7,10 @@ import ( "strings" "testing" + "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" + "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/test" ) @@ -407,21 +409,28 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) { func TestPRCreate_survey_defaults_monocommit(t *testing.T) { initBlankContext("", "OWNER/REPO", "feature") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } + http := initMockHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`\bviewerPermission\b`), httpmock.StringResponse(api.RepoNetworkStubResponse("OWNER", "REPO", "WRITE"))) + http.Register(httpmock.GraphQL(`\bforks\(`), httpmock.StringResponse(` + { "data": { "repository": { "forks": { "nodes": [ + ] } } } } `)) - http.StubResponse(200, bytes.NewBufferString(` + http.Register(httpmock.GraphQL(`\bpullRequests\(`), httpmock.StringResponse(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } `)) - http.StubResponse(200, bytes.NewBufferString(` + http.Register(httpmock.GraphQL(`\bcreatePullRequest\(`), httpmock.GraphQLMutation(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" } } } } - `)) + `, func(inputs map[string]interface{}) { + eq(t, inputs["repositoryId"], "REPOID") + eq(t, inputs["title"], "the sky above the port") + eq(t, inputs["body"], "was the color of a television, turned to a dead channel") + eq(t, inputs["baseRefName"], "master") + eq(t, inputs["headRefName"], "feature") + })) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -456,29 +465,6 @@ func TestPRCreate_survey_defaults_monocommit(t *testing.T) { output, err := RunCommand(`pr create`) eq(t, err, nil) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - expectedBody := "was the color of a television, turned to a dead channel" - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "the sky above the port") - eq(t, reqBody.Variables.Input.Body, expectedBody) - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } diff --git a/command/testing.go b/command/testing.go index 24f2e8b60..05b6c6117 100644 --- a/command/testing.go +++ b/command/testing.go @@ -11,6 +11,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/httpmock" "github.com/google/shlex" "github.com/spf13/pflag" ) @@ -101,6 +102,14 @@ func initFakeHTTP() *api.FakeHTTP { return http } +func initMockHTTP() *httpmock.Registry { + http := &httpmock.Registry{} + apiClientForContext = func(context.Context) (*api.Client, error) { + return api.NewClient(api.ReplaceTripper(http)), nil + } + return http +} + type cmdOut struct { outBuf, errBuf *bytes.Buffer } From 0a4d4ee0074c393b04e55c1135436c542648b40a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 7 May 2020 15:19:14 +0200 Subject: [PATCH 3/3] Replace `FakeHTTP` with httpmock which is now compatible --- api/client_test.go | 8 +- api/fake_http.go | 120 -------------------------- api/queries_issue_test.go | 3 +- api/queries_repo.go | 14 --- api/queries_repo_test.go | 4 +- command/pr_create_test.go | 5 +- command/testing.go | 10 +-- context/remote_test.go | 5 +- pkg/httpmock/legacy.go | 89 +++++++++++++++++++ pkg/httpmock/registry.go | 70 +++++++++++++++ pkg/httpmock/{httpmock.go => stub.go} | 60 +------------ update/update_test.go | 3 +- 12 files changed, 181 insertions(+), 210 deletions(-) delete mode 100644 api/fake_http.go create mode 100644 pkg/httpmock/legacy.go create mode 100644 pkg/httpmock/registry.go rename pkg/httpmock/{httpmock.go => stub.go} (66%) diff --git a/api/client_test.go b/api/client_test.go index 492609e5c..4c81bf315 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -5,6 +5,8 @@ import ( "io/ioutil" "reflect" "testing" + + "github.com/cli/cli/pkg/httpmock" ) func eq(t *testing.T, got interface{}, expected interface{}) { @@ -15,7 +17,7 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } func TestGraphQL(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient( ReplaceTripper(http), AddHeader("Authorization", "token OTOKEN"), @@ -40,7 +42,7 @@ func TestGraphQL(t *testing.T) { } func TestGraphQLError(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient(ReplaceTripper(http)) response := struct{}{} @@ -52,7 +54,7 @@ func TestGraphQLError(t *testing.T) { } func TestRESTGetDelete(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient( ReplaceTripper(http), diff --git a/api/fake_http.go b/api/fake_http.go deleted file mode 100644 index d8b7506a6..000000000 --- a/api/fake_http.go +++ /dev/null @@ -1,120 +0,0 @@ -package api - -import ( - "bytes" - "fmt" - "io" - "io/ioutil" - "net/http" - "os" - "path" - "strings" -) - -// FakeHTTP provides a mechanism by which to stub HTTP responses through -type FakeHTTP struct { - // Requests stores references to sequential requests that RoundTrip has received - Requests []*http.Request - count int - responseStubs []*http.Response -} - -// StubResponse pre-records an HTTP response -func (f *FakeHTTP) StubResponse(status int, body io.Reader) { - resp := &http.Response{ - StatusCode: status, - Body: ioutil.NopCloser(body), - } - f.responseStubs = append(f.responseStubs, resp) -} - -// RoundTrip satisfies http.RoundTripper -func (f *FakeHTTP) RoundTrip(req *http.Request) (*http.Response, error) { - if len(f.responseStubs) <= f.count { - return nil, fmt.Errorf("FakeHTTP: missing response stub for request %d", f.count) - } - resp := f.responseStubs[f.count] - f.count++ - resp.Request = req - f.Requests = append(f.Requests, req) - return resp, nil -} - -func (f *FakeHTTP) StubWithFixture(status int, fixtureFileName string) func() { - fixturePath := path.Join("../test/fixtures/", fixtureFileName) - fixtureFile, _ := os.Open(fixturePath) - f.StubResponse(status, fixtureFile) - return func() { fixtureFile.Close() } -} - -func (f *FakeHTTP) StubRepoResponse(owner, repo string) { - f.StubRepoResponseWithPermission(owner, repo, "WRITE") -} - -func (f *FakeHTTP) StubRepoResponseWithPermission(owner, repo, permission string) { - body := bytes.NewBufferString(fmt.Sprintf(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "%s", - "owner": {"login": "%s"}, - "defaultBranchRef": { - "name": "master" - }, - "viewerPermission": "%s" - } } } - `, repo, owner, permission)) - resp := &http.Response{ - StatusCode: 200, - Body: ioutil.NopCloser(body), - } - f.responseStubs = append(f.responseStubs, resp) -} - -func (f *FakeHTTP) StubRepoResponseWithDefaultBranch(owner, repo, defaultBranch string) { - body := bytes.NewBufferString(fmt.Sprintf(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "%s", - "owner": {"login": "%s"}, - "defaultBranchRef": { - "name": "%s" - }, - "viewerPermission": "READ" - } } } - `, repo, owner, defaultBranch)) - resp := &http.Response{ - StatusCode: 200, - Body: ioutil.NopCloser(body), - } - f.responseStubs = append(f.responseStubs, resp) -} - -func (f *FakeHTTP) StubForkedRepoResponse(forkFullName, parentFullName string) { - forkRepo := strings.SplitN(forkFullName, "/", 2) - parentRepo := strings.SplitN(parentFullName, "/", 2) - body := bytes.NewBufferString(fmt.Sprintf(` - { "data": { "repo_000": { - "id": "REPOID2", - "name": "%s", - "owner": {"login": "%s"}, - "defaultBranchRef": { - "name": "master" - }, - "viewerPermission": "ADMIN", - "parent": { - "id": "REPOID1", - "name": "%s", - "owner": {"login": "%s"}, - "defaultBranchRef": { - "name": "master" - }, - "viewerPermission": "READ" - } - } } } - `, forkRepo[1], forkRepo[0], parentRepo[1], parentRepo[0])) - resp := &http.Response{ - StatusCode: 200, - Body: ioutil.NopCloser(body), - } - f.responseStubs = append(f.responseStubs, resp) -} diff --git a/api/queries_issue_test.go b/api/queries_issue_test.go index b2dc7c819..c8ee505cf 100644 --- a/api/queries_issue_test.go +++ b/api/queries_issue_test.go @@ -7,10 +7,11 @@ import ( "testing" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/httpmock" ) func TestIssueList(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient(ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(` diff --git a/api/queries_repo.go b/api/queries_repo.go index c415aeec7..b8ad760d0 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -221,20 +221,6 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e return result, nil } -func RepoNetworkStubResponse(owner, repo, permission string) string { - return fmt.Sprintf(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "%s", - "owner": {"login": "%s"}, - "defaultBranchRef": { - "name": "master" - }, - "viewerPermission": "%s" - } } } - `, repo, owner, permission) -} - // repositoryV3 is the repository result from GitHub API v3 type repositoryV3 struct { NodeID string diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 49202f628..d8a75f4ff 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -5,10 +5,12 @@ import ( "encoding/json" "io/ioutil" "testing" + + "github.com/cli/cli/pkg/httpmock" ) func Test_RepoCreate(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient(ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(`{}`)) diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 7445f9a68..f69c5c0c6 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -7,7 +7,6 @@ import ( "strings" "testing" - "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/pkg/httpmock" @@ -409,9 +408,9 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) { func TestPRCreate_survey_defaults_monocommit(t *testing.T) { initBlankContext("", "OWNER/REPO", "feature") - http := initMockHTTP() + http := initFakeHTTP() defer http.Verify(t) - http.Register(httpmock.GraphQL(`\bviewerPermission\b`), httpmock.StringResponse(api.RepoNetworkStubResponse("OWNER", "REPO", "WRITE"))) + http.Register(httpmock.GraphQL(`\bviewerPermission\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) http.Register(httpmock.GraphQL(`\bforks\(`), httpmock.StringResponse(` { "data": { "repository": { "forks": { "nodes": [ ] } } } } diff --git a/command/testing.go b/command/testing.go index 05b6c6117..06d9a90a1 100644 --- a/command/testing.go +++ b/command/testing.go @@ -94,15 +94,7 @@ func initBlankContext(cfg, repo, branch string) { } } -func initFakeHTTP() *api.FakeHTTP { - http := &api.FakeHTTP{} - apiClientForContext = func(context.Context) (*api.Client, error) { - return api.NewClient(api.ReplaceTripper(http)), nil - } - return http -} - -func initMockHTTP() *httpmock.Registry { +func initFakeHTTP() *httpmock.Registry { http := &httpmock.Registry{} apiClientForContext = func(context.Context) (*api.Client, error) { return api.NewClient(api.ReplaceTripper(http)), nil diff --git a/context/remote_test.go b/context/remote_test.go index 4b6838e23..9cf7a0adf 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/httpmock" ) func eq(t *testing.T, got interface{}, expected interface{}) { @@ -70,7 +71,7 @@ func Test_translateRemotes(t *testing.T) { } func Test_resolvedRemotes_triangularSetup(t *testing.T) { - http := &api.FakeHTTP{} + http := &httpmock.Registry{} apiClient := api.NewClient(api.ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(` @@ -137,7 +138,7 @@ func Test_resolvedRemotes_triangularSetup(t *testing.T) { } func Test_resolvedRemotes_forkLookup(t *testing.T) { - http := &api.FakeHTTP{} + http := &httpmock.Registry{} apiClient := api.NewClient(api.ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(` diff --git a/pkg/httpmock/legacy.go b/pkg/httpmock/legacy.go new file mode 100644 index 000000000..9474c3dd8 --- /dev/null +++ b/pkg/httpmock/legacy.go @@ -0,0 +1,89 @@ +package httpmock + +import ( + "fmt" + "io" + "net/http" + "os" + "path" + "strings" +) + +// TODO: clean up methods in this file when there are no more callers + +func (r *Registry) StubResponse(status int, body io.Reader) { + r.Register(MatchAny, func(*http.Request) (*http.Response, error) { + return httpResponse(status, body), nil + }) +} + +func (r *Registry) StubWithFixture(status int, fixtureFileName string) func() { + fixturePath := path.Join("../test/fixtures/", fixtureFileName) + fixtureFile, err := os.Open(fixturePath) + r.Register(MatchAny, func(*http.Request) (*http.Response, error) { + if err != nil { + return nil, err + } + return httpResponse(200, fixtureFile), nil + }) + return func() { + if err == nil { + fixtureFile.Close() + } + } +} + +func (r *Registry) StubRepoResponse(owner, repo string) { + r.StubRepoResponseWithPermission(owner, repo, "WRITE") +} + +func (r *Registry) StubRepoResponseWithPermission(owner, repo, permission string) { + r.Register(MatchAny, StringResponse(RepoNetworkStubResponse(owner, repo, "master", permission))) +} + +func (r *Registry) StubRepoResponseWithDefaultBranch(owner, repo, defaultBranch string) { + r.Register(MatchAny, StringResponse(RepoNetworkStubResponse(owner, repo, defaultBranch, "WRITE"))) +} + +func (r *Registry) StubForkedRepoResponse(ownRepo, parentRepo string) { + r.Register(MatchAny, StringResponse(RepoNetworkStubForkResponse(ownRepo, parentRepo))) +} + +func RepoNetworkStubResponse(owner, repo, defaultBranch, permission string) string { + return fmt.Sprintf(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "%s" + }, + "viewerPermission": "%s" + } } } + `, repo, owner, defaultBranch, permission) +} + +func RepoNetworkStubForkResponse(forkFullName, parentFullName string) string { + forkRepo := strings.SplitN(forkFullName, "/", 2) + parentRepo := strings.SplitN(parentFullName, "/", 2) + return fmt.Sprintf(` + { "data": { "repo_000": { + "id": "REPOID2", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "master" + }, + "viewerPermission": "ADMIN", + "parent": { + "id": "REPOID1", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "master" + }, + "viewerPermission": "READ" + } + } } } + `, forkRepo[1], forkRepo[0], parentRepo[1], parentRepo[0]) +} diff --git a/pkg/httpmock/registry.go b/pkg/httpmock/registry.go new file mode 100644 index 000000000..486d79a06 --- /dev/null +++ b/pkg/httpmock/registry.go @@ -0,0 +1,70 @@ +package httpmock + +import ( + "fmt" + "net/http" + "sync" +) + +type Registry struct { + mu sync.Mutex + stubs []*Stub + Requests []*http.Request +} + +func (r *Registry) Register(m Matcher, resp Responder) { + r.stubs = append(r.stubs, &Stub{ + Matcher: m, + Responder: resp, + }) +} + +type Testing interface { + Errorf(string, ...interface{}) +} + +func (r *Registry) Verify(t Testing) { + n := 0 + for _, s := range r.stubs { + if !s.matched { + n++ + } + } + if n > 0 { + // 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) + } +} + +// RoundTrip satisfies http.RoundTripper +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 + } + // TODO: reinstate this check once the legacy layer has been cleaned up + // if stub != nil { + // r.mu.Unlock() + // return nil, fmt.Errorf("more than 1 stub matched %v", req) + // } + stub = s + break // TODO: remove + } + 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/pkg/httpmock/httpmock.go b/pkg/httpmock/stub.go similarity index 66% rename from pkg/httpmock/httpmock.go rename to pkg/httpmock/stub.go index 9b34b9063..b27ceea6c 100644 --- a/pkg/httpmock/httpmock.go +++ b/pkg/httpmock/stub.go @@ -3,69 +3,13 @@ package httpmock import ( "bytes" "encoding/json" - "fmt" "io" "io/ioutil" "net/http" "regexp" "strings" - "sync" ) -type Registry struct { - mu sync.Mutex - stubs []*Stub -} - -func (r *Registry) Register(m Matcher, resp Responder) { - r.stubs = append(r.stubs, &Stub{ - Matcher: m, - Responder: resp, - }) -} - -type Testing interface { - Errorf(string, ...interface{}) -} - -func (r *Registry) Verify(t Testing) { - n := 0 - for _, s := range r.stubs { - if !s.matched { - n++ - } - } - if n > 0 { - t.Errorf("%d unmatched HTTP stubs", n) - } -} - -// RoundTrip satisfies http.RoundTripper -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 - } - r.mu.Unlock() - - if stub == nil { - return nil, fmt.Errorf("no registered stubs matched %v", req) - } - return stub.Responder(req) -} - type Matcher func(req *http.Request) bool type Responder func(req *http.Request) (*http.Response, error) @@ -75,6 +19,10 @@ type Stub struct { Responder Responder } +func MatchAny(*http.Request) bool { + return true +} + func GraphQL(q string) Matcher { re := regexp.MustCompile(q) diff --git a/update/update_test.go b/update/update_test.go index bd919f530..2fcb2d6ab 100644 --- a/update/update_test.go +++ b/update/update_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/cli/cli/api" + "github.com/cli/cli/pkg/httpmock" ) func TestCheckForUpdate(t *testing.T) { @@ -51,7 +52,7 @@ func TestCheckForUpdate(t *testing.T) { for _, s := range scenarios { t.Run(s.Name, func(t *testing.T) { - http := &api.FakeHTTP{} + http := &httpmock.Registry{} client := api.NewClient(api.ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(fmt.Sprintf(`{ "tag_name": "%s",