From 5569e84944b0ca79f2b9d8affb5126892f796d65 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Fri, 30 Sep 2022 11:15:25 -0400 Subject: [PATCH] Make Cloner testable; add tests; add mock library Signed-off-by: jesus m. rodriguez --- cmd/clone/cmd.go | 2 +- internal/jira/cloner.go | 20 +-- internal/jira/cloner_test.go | 116 +++++++++++++++- internal/jira/mock/endpointpattern.go | 6 + internal/jira/mock/server.go | 189 ++++++++++++++++++++++++++ internal/jira/mock/server_options.go | 106 +++++++++++++++ internal/jira/mock/utils.go | 29 ++++ 7 files changed, 453 insertions(+), 15 deletions(-) create mode 100644 internal/jira/mock/endpointpattern.go create mode 100644 internal/jira/mock/server.go create mode 100644 internal/jira/mock/server_options.go create mode 100644 internal/jira/mock/utils.go diff --git a/cmd/clone/cmd.go b/cmd/clone/cmd.go index 0b1771d..82b3097 100644 --- a/cmd/clone/cmd.go +++ b/cmd/clone/cmd.go @@ -42,7 +42,7 @@ func NewCmd() *cobra.Command { if err != nil { return err } - err = jira.Clone(issue, jira.WithProject(project), jira.WithDryRun(dryRun)) + _, err = jira.Clone(issue, jira.WithProject(project), jira.WithDryRun(dryRun)) if err != nil { return nil } diff --git a/internal/jira/cloner.go b/internal/jira/cloner.go index c4af9f1..c4b4c32 100644 --- a/internal/jira/cloner.go +++ b/internal/jira/cloner.go @@ -20,7 +20,6 @@ import ( "os" "strings" - "github.com/andygrunwald/go-jira" gojira "github.com/andygrunwald/go-jira" "github.com/google/go-github/v47/github" ) @@ -97,24 +96,24 @@ func getWebURL(url string) string { return strings.Replace(strings.Replace(url, "api.github.com", "github.com", 1), "repos/", "", 1) } -func Clone(issue *github.Issue, opts ...Option) error { +func Clone(issue *github.Issue, opts ...Option) (*gojira.Issue, error) { config := ClonerConfig{} for _, opt := range opts { if err := opt(&config); err != nil { - return err + return nil, err } } if err := config.setDefaults(); err != nil { - return err + return nil, err } jiraClient, err := gojira.NewClient(config.client, config.jiraURL) if err != nil { - return err + return nil, err } - ji := jira.Issue{ + ji := gojira.Issue{ Fields: &gojira.IssueFields{ // Assignee: &gojira.User{ // Name: "myuser", @@ -133,6 +132,8 @@ func Clone(issue *github.Issue, opts ...Option) error { }, } + var daIssue *gojira.Issue + if config.dryRun { fmt.Println("\n############# DRY RUN MODE #############") fmt.Printf("Cloning issue #%d to jira project board: %s\n\n", issue.GetNumber(), ji.Fields.Project.Key) @@ -143,9 +144,10 @@ func Clone(issue *github.Issue, opts ...Option) error { fmt.Println("\n############# DRY RUN MODE #############") } else { fmt.Printf("Cloning issue #%d to jira project board: %s\n\n", issue.GetNumber(), ji.Fields.Project.Key) - daIssue, _, err := jiraClient.Issue.Create(&ji) + var err error + daIssue, _, err = jiraClient.Issue.Create(&ji) if err != nil { - return err + return daIssue, err } if daIssue != nil { @@ -154,5 +156,5 @@ func Clone(issue *github.Issue, opts ...Option) error { } } - return nil + return daIssue, nil } diff --git a/internal/jira/cloner_test.go b/internal/jira/cloner_test.go index 56a9b13..6e11f5c 100644 --- a/internal/jira/cloner_test.go +++ b/internal/jira/cloner_test.go @@ -15,9 +15,16 @@ package jira import ( + "fmt" + "io" + "net/http" "os" + "strings" + + gojira "github.com/andygrunwald/go-jira" + "github.com/google/go-github/v47/github" + jmock "github.com/jmrodri/gh2jira/internal/jira/mock" - "github.com/migueleliasweb/go-github-mock/src/mock" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -74,7 +81,7 @@ var _ = Describe("Cloner", func() { Expect(options.client).To(BeNil()) }) It("should set the client if given one", func() { - mc := mock.NewMockedHTTPClient() + mc := jmock.NewMockedHTTPClient() opt := WithClient(mc) err := opt(&options) Expect(err).NotTo(HaveOccurred()) @@ -151,14 +158,113 @@ var _ = Describe("Cloner", func() { err := os.Unsetenv("JIRA_TOKEN") Expect(err).NotTo(HaveOccurred()) - err = Clone(nil) + _, err = Clone(nil) Expect(err).To(HaveOccurred()) }) It("should print out issue when dryRun is true", func() { - }) - It("should save issue in jira when dryRun is false", func() { + mockedHTTPClient := jmock.NewMockedHTTPClient( + jmock.WithRequestMatch(jmock.PostIssue), + ) + + // giving it a github issue + ghissue := &github.Issue{ + ID: github.Int64(12213123), + Number: github.Int(123), + Title: github.String("Issue 1"), + State: github.String("open"), + Body: github.String("body of the issue"), + URL: github.String("https://api.github.com/repos/foo/bar/issues/123"), + } + + // Capture stdout to verify the printing + r, w, _ := os.Pipe() + tmp := os.Stdout + defer func() { + os.Stdout = tmp + }() + os.Stdout = w + go func() { + // Test the clone function + _, err := Clone(ghissue, WithClient(mockedHTTPClient), + WithDryRun(true), + WithJiraURL("http://localhost"), + ) + w.Close() + Expect(err).NotTo(HaveOccurred()) + }() + stdout, _ := io.ReadAll(r) + + Expect(strings.Contains(string(stdout), "DRY RUN MODE")).To(BeTrue()) }) It("should return an error if jira client returns an error", func() { + // if our request returns an error ListIssues should return + // that error + mockedHTTPClient := jmock.NewMockedHTTPClient( + jmock.WithRequestMatchHandler( + jmock.PostIssue, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + jmock.WriteError( + w, + http.StatusInternalServerError, + "jira went belly up or something", + ) + }), + ), + ) + _, err := Clone(nil, WithClient(mockedHTTPClient), + WithDryRun(false), + WithJiraURL("http://localhost"), + ) + Expect(err).To(HaveOccurred()) + }) + It("should return error if Options return an error", func() { + _, err := Clone(nil, func(c *ClonerConfig) error { + return fmt.Errorf("do you see me") + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("do you see me")) + }) + It("should create a jira issue from the given github issue", func() { + // expected return jira issue + expectedissue := gojira.Issue{ + Fields: &gojira.IssueFields{ + Description: "body of the issue\n\nUpstream Github issue: " + + "https://github.com/foo/bar/issues/123\n", + Type: gojira.IssueType{ + Name: "Story", + }, + Project: gojira.Project{ + Key: "OSDK", + }, + Summary: "[UPSTREAM] Issue 1 #123", + }, + } + + mockedHTTPClient := jmock.NewMockedHTTPClient( + jmock.WithRequestMatch(jmock.PostIssue, expectedissue), + ) + + // giving it a github issue + ghissue := &github.Issue{ + ID: github.Int64(12213123), + Number: github.Int(123), + Title: github.String("Issue 1"), + State: github.String("open"), + Body: github.String("body of the issue"), + URL: github.String("https://api.github.com/repos/foo/bar/issues/123"), + } + + // Test the clone function + jissue, err := Clone(ghissue, WithClient(mockedHTTPClient), + WithDryRun(false), + WithJiraURL("http://localhost"), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(jissue).NotTo(BeNil()) + Expect(jissue.Fields.Description).To(Equal(expectedissue.Fields.Description)) + Expect(jissue.Fields.Type).To(Equal(expectedissue.Fields.Type)) + Expect(jissue.Fields.Project).To(Equal(expectedissue.Fields.Project)) + Expect(jissue.Fields.Summary).To(Equal(expectedissue.Fields.Summary)) }) }) }) diff --git a/internal/jira/mock/endpointpattern.go b/internal/jira/mock/endpointpattern.go new file mode 100644 index 0000000..aff066a --- /dev/null +++ b/internal/jira/mock/endpointpattern.go @@ -0,0 +1,6 @@ +package mock + +var PostIssue EndpointPattern = EndpointPattern{ + Pattern: "/rest/api/2/issue", + Method: "POST", +} diff --git a/internal/jira/mock/server.go b/internal/jira/mock/server.go new file mode 100644 index 0000000..6d5cb97 --- /dev/null +++ b/internal/jira/mock/server.go @@ -0,0 +1,189 @@ +package mock + +import ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + + "github.com/gorilla/mux" +) + +// EndpointPattern models the GitHub's API endpoints +type EndpointPattern struct { + Pattern string // eg. "/repos/{owner}/{repo}/actions/artifacts" + Method string // "GET", "POST", "PATCH", etc +} + +// MockBackendOption is used to configure the *mux.router +// for the mocked backend +type MockBackendOption func(*mux.Router) + +// FIFOReponseHandler handler implementation that +// responds to the HTTP requests following a FIFO approach. +// +// Once all available `Responses` have been used, this handler will panic()! +type FIFOReponseHandler struct { + lock sync.Mutex + Responses [][]byte + CurrentIndex int +} + +// ServeHTTP implementation of `http.Handler` +func (srh *FIFOReponseHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + + srh.lock.Lock() + defer srh.lock.Unlock() + if srh.CurrentIndex > len(srh.Responses) { + panic(fmt.Sprintf( + "go-github-mock: no more mocks available for %s", + r.URL.Path, + )) + } + + defer func() { + srh.CurrentIndex++ + }() + + w.Write(srh.Responses[srh.CurrentIndex]) +} + +// PaginatedReponseHandler handler implementation that +// responds to the HTTP requests and honors the pagination headers +// +// Header e.g: `Link: ; rel="next", +// ; rel="last", +// ; rel="first", +// ; rel="prev"` +// +// See: https://docs.github.com/en/rest/guides/traversing-with-pagination +// type PaginatedReponseHandler struct { +// ResponsePages [][]byte +// } + +// func (prh *PaginatedReponseHandler) getCurrentPage(r *http.Request) int { +// strPage := r.URL.Query().Get("page") +// +// if strPage == "" { +// return 1 +// } +// +// page, err := strconv.Atoi(r.URL.Query().Get("page")) +// +// if err == nil { +// return page +// } +// +// // this should never happen as the request is being made by the SDK +// panic(fmt.Sprintf("invalid page: %s", strPage)) +// } + +// func (prh *PaginatedReponseHandler) generateLinkHeader( +// w http.ResponseWriter, +// r *http.Request, +// ) { +// currentPage := prh.getCurrentPage(r) +// lastPage := len(prh.ResponsePages) +// +// buf := bytes.NewBufferString(`; rel="first",`) +// buf.WriteString(fmt.Sprintf(`; rel="last",`, lastPage)) +// +// if currentPage < lastPage { +// // when resp.NextPage == 0, it means no more pages +// // which is basically as not setting it in the response +// buf.WriteString(fmt.Sprintf(`; rel="next",`, currentPage+1)) +// } +// +// if currentPage > 1 { +// buf.WriteString(fmt.Sprintf(`; rel="prev",`, currentPage-1)) +// } +// +// w.Header().Add("Link", buf.String()) +// } + +// ServeHTTP implementation of `http.Handler` +// func (prh *PaginatedReponseHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { +// prh.generateLinkHeader(w, r) +// w.Write(prh.ResponsePages[prh.getCurrentPage(r)-1]) +// } + +// EnforceHostRoundTripper rewrites all requests with the given `Host`. +type EnforceHostRoundTripper struct { + Host string + UpstreamRoundTripper http.RoundTripper +} + +// RoundTrip implementation of `http.RoundTripper` +func (efrt *EnforceHostRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { + splitHost := strings.Split(efrt.Host, "://") + r.URL.Scheme = splitHost[0] + r.URL.Host = splitHost[1] + + return efrt.UpstreamRoundTripper.RoundTrip(r) +} + +// NewMockedHTTPClient creates and configures an http.Client that points to +// a mocked GitHub's backend API. +// +// Example: +// +// mockedHTTPClient := NewMockedHTTPClient( +// +// WithRequestMatch( +// GetUsersByUsername, +// github.User{ +// Name: github.String("foobar"), +// }, +// ), +// WithRequestMatch( +// GetUsersOrgsByUsername, +// []github.Organization{ +// { +// Name: github.String("foobar123thisorgwasmocked"), +// }, +// }, +// ), +// WithRequestMatchHandler( +// GetOrgsProjectsByOrg, +// func(w http.ResponseWriter, _ *http.Request) { +// w.Write(MustMarshal([]github.Project{ +// { +// Name: github.String("mocked-proj-1"), +// }, +// { +// Name: github.String("mocked-proj-2"), +// }, +// })) +// }, +// ), +// +// ) +// +// c := github.NewClient(mockedHTTPClient) +func NewMockedHTTPClient(options ...MockBackendOption) *http.Client { + router := mux.NewRouter() + + router.NotFoundHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + WriteError( + w, + http.StatusNotFound, + fmt.Sprintf("mock response not found for %s", r.URL.Path), + ) + }) + + for _, o := range options { + o(router) + } + + mockServer := httptest.NewServer(router) + + c := mockServer.Client() + + c.Transport = &EnforceHostRoundTripper{ + Host: mockServer.URL, + UpstreamRoundTripper: mockServer.Client().Transport, + } + + return c +} diff --git a/internal/jira/mock/server_options.go b/internal/jira/mock/server_options.go new file mode 100644 index 0000000..fde0653 --- /dev/null +++ b/internal/jira/mock/server_options.go @@ -0,0 +1,106 @@ +package mock + +import ( + "net/http" + + "github.com/gorilla/mux" +) + +// WithRequestMatchHandler implements a request callback +// for the given `pattern`. +// +// For custom implementations, this handler usage is encouraged. +// +// Example: +// +// WithRequestMatchHandler( +// GetOrgsProjectsByOrg, +// func(w http.ResponseWriter, _ *http.Request) { +// w.Write(MustMarshal([]github.Project{ +// { +// Name: github.String("mocked-proj-1"), +// }, +// { +// Name: github.String("mocked-proj-2"), +// }, +// })) +// }, +// ) +func WithRequestMatchHandler( + ep EndpointPattern, + handler http.Handler, +) MockBackendOption { + return func(router *mux.Router) { + router.Handle(ep.Pattern, handler).Methods(ep.Method) + } +} + +// WithRequestMatch implements a simple FIFO for requests +// of the given `pattern`. +// +// Once all responses have been used, it shall panic()! +// +// Example: +// +// WithRequestMatch( +// GetUsersByUsername, +// github.User{ +// Name: github.String("foobar"), +// }, +// ) +func WithRequestMatch( + ep EndpointPattern, + responsesFIFO ...interface{}, +) MockBackendOption { + responses := [][]byte{} + + for _, r := range responsesFIFO { + responses = append(responses, MustMarshal(r)) + } + + return WithRequestMatchHandler(ep, &FIFOReponseHandler{ + Responses: responses, + }) +} + +// WithRequestMatchPages honors pagination directives. +// +// Pages can be requested in any order and each page can be called multiple times. +// +// E.g. +// +// mockedHTTPClient := NewMockedHTTPClient( +// WithRequestMatchPages( +// GetOrgsReposByOrg, +// []github.Repository{ +// { +// Name: github.String("repo-A-on-first-page"), +// }, +// { +// Name: github.String("repo-B-on-first-page"), +// }, +// }, +// []github.Repository{ +// { +// Name: github.String("repo-C-on-second-page"), +// }, +// { +// Name: github.String("repo-D-on-second-page"), +// }, +// }, +// ), +// ) +// func WithRequestMatchPages( +// ep EndpointPattern, +// pages ...interface{}, +// ) MockBackendOption { +// p := [][]byte{} +// +// for _, r := range pages { +// p = append(p, MustMarshal(r)) +// } +// +// return WithRequestMatchHandler(ep, &PaginatedReponseHandler{ +// ResponsePages: p, +// }) +// } diff --git a/internal/jira/mock/utils.go b/internal/jira/mock/utils.go new file mode 100644 index 0000000..1441def --- /dev/null +++ b/internal/jira/mock/utils.go @@ -0,0 +1,29 @@ +package mock + +import ( + "encoding/json" + "errors" + "net/http" +) + +// MustMarshal helper function that wraps json.Marshal +func MustMarshal(v interface{}) []byte { + b, err := json.Marshal(v) + + if err == nil { + return b + } + + panic(err) +} + +// WriteError helper function to write errors to HTTP handlers +func WriteError( + w http.ResponseWriter, + httpStatus int, + msg string, +) { + w.WriteHeader(httpStatus) + + w.Write(MustMarshal(errors.New(msg))) +}