From ef4c3760a9166ad9495073db891cc3dce862cf7a Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Tue, 11 Feb 2020 15:02:33 +1100 Subject: [PATCH 1/8] Create GitLab client with CreateRelease support --- internal/gitlab/client.go | 70 ++++++++++++++ internal/gitlab/client_test.go | 57 ++++++++++++ internal/gitlab/data_test.go | 103 +++++++++++++++++++++ internal/gitlab/release.go | 137 +++++++++++++++++++++++++++ internal/gitlab/release_test.go | 159 ++++++++++++++++++++++++++++++++ 5 files changed, 526 insertions(+) create mode 100644 internal/gitlab/client.go create mode 100644 internal/gitlab/client_test.go create mode 100644 internal/gitlab/data_test.go create mode 100644 internal/gitlab/release.go create mode 100644 internal/gitlab/release_test.go diff --git a/internal/gitlab/client.go b/internal/gitlab/client.go new file mode 100644 index 0000000..4adf5f5 --- /dev/null +++ b/internal/gitlab/client.go @@ -0,0 +1,70 @@ +package gitlab + +import ( + "context" + "fmt" + "io" + "net/http" + + log "github.com/sirupsen/logrus" +) + +// ErrorResponse expected from the API +type ErrorResponse struct { + statusCode int + Message string `json:"message,omitempty"` + Err string `json:"error,omitempty"` // TODO the API returns different fields for some different status codes +} + +// Error implements the error interface. Wraps an error message from the API into an error type +func (er *ErrorResponse) Error() string { + if er.Err != "" { + return fmt.Sprintf("API Error Response status_code: %d message: %s error: %s", er.statusCode, er.Message, er.Err) + } + + return fmt.Sprintf("API Error Response status_code: %d message: %s", er.statusCode, er.Message) +} + +type httpClient interface { + Do(*http.Request) (*http.Response, error) +} + +// Client contains the GitLab client configuration +type Client struct { + baseURL string + token string + projectID string + httpClient httpClient +} + +// New creates a new GitLab Client +func New(baseURL, token, projectID string, httpClient httpClient) *Client { + return &Client{ + baseURL: baseURL, + token: token, + projectID: projectID, + httpClient: httpClient, + } +} + +// request creates a new request and attaches +func (gc *Client) request(ctx context.Context, method, url string, body io.Reader) (*http.Request, error) { + req, err := http.NewRequest(method, gc.baseURL+url, body) + if err != nil { + return nil, err + } + + req = req.WithContext(ctx) + + // TODO support `PRIVATE-TOKEN` when the cli is not used in CI + req.Header.Set("JOB-TOKEN", gc.token) + req.Header.Set("Content-Type", "application/json") + + return req, nil +} + +func checkClosed(closer io.Closer) { + if err := closer.Close(); err != nil { + log.WithError(err).Warn("failed to close") + } +} diff --git a/internal/gitlab/client_test.go b/internal/gitlab/client_test.go new file mode 100644 index 0000000..8e30a34 --- /dev/null +++ b/internal/gitlab/client_test.go @@ -0,0 +1,57 @@ +package gitlab + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestClient_request(t *testing.T) { + tests := []struct { + name string + method string + baseURL string + token string + projectID string + wantErrMsg string + }{ + { + name: "no_error", + baseURL: "http://127.0.0.1", + token: "token", + projectID: "projectID", + }, + { + name: "invalid_url", + baseURL: "%", + wantErrMsg: "parse %%: invalid URL escape \"%%\"", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gc := New(tt.baseURL, tt.token, tt.projectID, &mockClient{}) + got, err := gc.request(context.Background(), tt.method, tt.baseURL, nil) + if tt.wantErrMsg != "" { + require.EqualError(t, err, tt.wantErrMsg) + return + } + + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, got.Header.Get("JOB-TOKEN"), tt.token) + require.Equal(t, got.Header.Get("Content-Type"), "application/json") + }) + } +} + +// mockClient very simple mocking function +type mockClient struct { + res *http.Response + err error +} + +func (mc *mockClient) Do(r *http.Request) (*http.Response, error) { + return mc.res, mc.err +} diff --git a/internal/gitlab/data_test.go b/internal/gitlab/data_test.go new file mode 100644 index 0000000..6decbb2 --- /dev/null +++ b/internal/gitlab/data_test.go @@ -0,0 +1,103 @@ +package gitlab + +const createReleaseSuccessResponse = ` +{ + "tag_name":"v0.1", + "description":"## CHANGELOG\r\n\r\n- Remove limit of 100 when searching repository code. !8671\r\n- Show error message when attempting to reopen an MR and there is an open MR for the same branch. !16447 (Akos Gyimesi)\r\n- Fix a bug where internal email pattern wasn't respected. !22516", + "name":"new name", + "description_html":"\u003ch2 dir=\"auto\"\u003e\n\u003ca id=\"user-content-changelog\" class=\"anchor\" href=\"#changelog\" aria-hidden=\"true\"\u003e\u003c/a\u003eCHANGELOG\u003c/h2\u003e\n\u003cul dir=\"auto\"\u003e\n\u003cli\u003eRemove limit of 100 when searching repository code. !8671\u003c/li\u003e\n\u003cli\u003eShow error message when attempting to reopen an MR and there is an open MR for the same branch. !16447 (Akos Gyimesi)\u003c/li\u003e\n\u003cli\u003eFix a bug where internal email pattern wasn't respected. !22516\u003c/li\u003e\n\u003c/ul\u003e", + "created_at":"2019-01-03T01:55:18.203Z", + "released_at":"2019-01-03T01:55:18.203Z", + "author":{ + "id":1, + "name":"Administrator", + "username":"root", + "state":"active", + "avatar_url":"https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon", + "web_url":"https://gitlab.example.com/root" + }, + "commit":{ + "id":"f8d3d94cbd347e924aa7b715845e439d00e80ca4", + "short_id":"f8d3d94c", + "title":"Initial commit", + "created_at":"2019-01-03T01:53:28.000Z", + "parent_ids":[ + + ], + "message":"Initial commit", + "author_name":"Administrator", + "author_email":"admin@example.com", + "authored_date":"2019-01-03T01:53:28.000Z", + "committer_name":"Administrator", + "committer_email":"admin@example.com", + "committed_date":"2019-01-03T01:53:28.000Z" + }, + "milestones": [ + { + "id":53, + "iid":3, + "project_id":24, + "title":"v1.0", + "description":"Voluptate fugiat possimus quis quod aliquam expedita.", + "state":"active", + "created_at":"2019-09-01T13:00:00.256Z", + "updated_at":"2019-09-01T13:00:00.256Z", + "due_date":"2019-09-20T13:00:00.256Z", + "start_date":"2019-09-05T12:00:00.256Z", + "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/3" + } + ], + "commit_path":"/root/awesome-app/commit/588440f66559714280628a4f9799f0c4eb880a4a", + "tag_path":"/root/awesome-app/-/tags/v0.11.1", + "evidence_sha":"760d6cdfb0879c3ffedec13af470e0f71cf52c6cde4d", + "assets":{ + "count":4, + "sources":[ + { + "format":"zip", + "url":"https://gitlab.example.com/root/awesome-app/-/archive/v0.1/awesome-app-v0.1.zip" + }, + { + "format":"tar.gz", + "url":"https://gitlab.example.com/root/awesome-app/-/archive/v0.1/awesome-app-v0.1.tar.gz" + }, + { + "format":"tar.bz2", + "url":"https://gitlab.example.com/root/awesome-app/-/archive/v0.1/awesome-app-v0.1.tar.bz2" + }, + { + "format":"tar", + "url":"https://gitlab.example.com/root/awesome-app/-/archive/v0.1/awesome-app-v0.1.tar" + } + ], + "links":[ + + ], + "evidence_file_path":"https://gitlab.example.com/root/awesome-app/-/releases/v0.1/evidence.json" + } +} +` + +const createReleaseForbiddenResponse = ` +{ + "message":"403 Forbidden" +} +` + +const createReleaseBadRequestResponse = ` +{ + "error":"tag_name is missing, description is missing" +} +` + +const createReleaseConflictResponse = ` +{ + "message":"Release already exists" +} +` + +const createReleaseInternalErrorResponse = ` +{ + "message":"500 Internal Server Error" +} +` diff --git a/internal/gitlab/release.go b/internal/gitlab/release.go new file mode 100644 index 0000000..a3f98d1 --- /dev/null +++ b/internal/gitlab/release.go @@ -0,0 +1,137 @@ +package gitlab + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "time" +) + +// CreateReleaseRequest body +type CreateReleaseRequest struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + TagName string `json:"tag_name"` + Ref string `json:"ref,omitempty"` + Milestones []string `json:"milestones,omitempty"` + Assets *Assets `json:"assets,omitempty"` + ReleasedAt *time.Time `json:"released_at,omitempty"` +} + +// CreateReleaseResponse full body +type CreateReleaseResponse struct { + Name string `json:"name"` + Description string `json:"description"` + DescriptionHTML string `json:"description_html"` + TagName string `json:"tag_name"` + CreatedAt time.Time `json:"created_at"` + ReleasedAt time.Time `json:"released_at"` + Assets *Assets `json:"assets"` + Author *Author `json:"author"` + Milestones []*Milestone `json:"milestones"` + CommitPath string `json:"commit_path"` + TagPath string `json:"tag_path"` + EvidenceSha string `json:"evidence_sha"` +} + +// Author of the release +type Author struct { + ID int `json:"id"` + Name string `json:"name"` + Username string `json:"username"` + State string `json:"state"` + AvatarURL string `json:"avatar_url"` + WebURL string `json:"web_url"` +} + +// Commit metadata +type Commit struct { + ID string `json:"id"` + ShortID string `json:"short_id"` + Title string `json:"title"` + CreatedAt time.Time `json:"created_at"` + ParentIds []string `json:"parent_ids"` + Message string `json:"message"` + AuthorName string `json:"author_name"` + AuthorEmail string `json:"author_email"` + AuthoredDate time.Time `json:"authored_date"` + CommitterName string `json:"committer_name"` + CommitterEmail string `json:"committer_email"` + CommittedDate time.Time `json:"committed_date"` +} + +// Milestone information +type Milestone struct { + ID int `json:"id"` + Iid int `json:"iid"` + ProjectID int `json:"project_id"` + Title string `json:"title"` + Description string `json:"description"` + State string `json:"state"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + DueDate time.Time `json:"due_date"` + StartDate time.Time `json:"start_date"` + WebURL string `json:"web_url"` +} + +// Assets of a release +type Assets struct { + Count int64 `json:"count,omitempty"` + Sources []struct { + Format string `json:"format"` + URL string `json:"url"` + } `json:"sources,omitempty"` + Links []struct { + ID string `json:"id"` + Name string `json:"name"` + URL string `json:"url"` + External bool `json:"external"` + } `json:"links,omitempty"` + EvidenceFilePath string `json:"evidence_file_path,omitempty"` +} + +// CreateRelease will try to create a release via GitLab's Releases API +func (gc *Client) CreateRelease(ctx context.Context, createReleaseReq *CreateReleaseRequest) (*CreateReleaseResponse, error) { + body, err := json.Marshal(createReleaseReq) + if err != nil { + return nil, fmt.Errorf("failed to marshal request body: %w", err) + } + + req, err := gc.request(ctx, "POST", "/projects/"+gc.projectID+"/releases", bytes.NewBuffer(body)) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + res, err := gc.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to do request: %w", err) + } + + defer checkClosed(res.Body) + + if res.StatusCode >= http.StatusBadRequest { + errResponse := ErrorResponse{ + statusCode: res.StatusCode, + } + + err := json.NewDecoder(res.Body).Decode(&errResponse) + if err != nil { + return nil, fmt.Errorf("failed to decode error response: %w", err) + } + + return nil, &errResponse + } + + var response CreateReleaseResponse + + err = json.NewDecoder(res.Body).Decode(&response) + if err != nil { + return nil, fmt.Errorf("failed to decode response: %w", err) + } + + return &response, nil +} diff --git a/internal/gitlab/release_test.go b/internal/gitlab/release_test.go new file mode 100644 index 0000000..2336c46 --- /dev/null +++ b/internal/gitlab/release_test.go @@ -0,0 +1,159 @@ +package gitlab + +import ( + "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestClient_CreateRelease(t *testing.T) { + baseCRR := CreateReleaseRequest{ + ID: "projectID", + Name: "name", + TagName: "tag name", + Description: "description", + Ref: "ref", + } + + tests := []struct { + name string + createReleaseRequest *CreateReleaseRequest + mockClient *mockClient + wantResponse *CreateReleaseResponse + wantErrResponse *ErrorResponse + }{ + { + name: "success", + mockClient: &mockClient{ + res: &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(strings.NewReader(createReleaseSuccessResponse)), + }, + }, + wantResponse: func() *CreateReleaseResponse { + var res CreateReleaseResponse + err := json.Unmarshal([]byte(createReleaseSuccessResponse), &res) + require.NoError(t, err) + return &res + }(), + }, + { + name: "forbidden", + mockClient: &mockClient{ + res: &http.Response{ + StatusCode: http.StatusForbidden, + Body: ioutil.NopCloser(strings.NewReader(createReleaseForbiddenResponse)), + }, + }, + wantErrResponse: &ErrorResponse{statusCode: http.StatusForbidden, Message: "403 Forbidden"}, + }, + { + name: "bad_request", + mockClient: &mockClient{ + res: &http.Response{ + StatusCode: http.StatusBadRequest, + Body: ioutil.NopCloser(strings.NewReader(createReleaseBadRequestResponse)), + }, + }, + wantErrResponse: &ErrorResponse{statusCode: http.StatusBadRequest, Err: "tag_name is missing, description is missing"}, + }, + { + name: "conflict", + mockClient: &mockClient{ + res: &http.Response{ + StatusCode: http.StatusConflict, + Body: ioutil.NopCloser(strings.NewReader(createReleaseConflictResponse)), + }, + }, + wantErrResponse: &ErrorResponse{statusCode: http.StatusConflict, Message: "Release already exists"}, + }, + { + name: "internal_server_error", + mockClient: &mockClient{ + res: &http.Response{ + StatusCode: http.StatusInternalServerError, + Body: ioutil.NopCloser(strings.NewReader(createReleaseInternalErrorResponse)), + }, + }, + wantErrResponse: &ErrorResponse{statusCode: http.StatusInternalServerError, Message: "500 Internal Server Error"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gc := &Client{ + baseURL: "http://127.0.0.1", + token: "privateToken", + projectID: "projectID", + httpClient: tt.mockClient, + } + got, err := gc.CreateRelease(context.Background(), &baseCRR) + if tt.wantErrResponse != nil { + require.Error(t, err) + require.EqualError(t, err, tt.wantErrResponse.Error()) + + return + } + + require.NoError(t, err) + require.NotNil(t, got) + }) + } +} + +func TestClient_CreateRelease_NonAPIErrors(t *testing.T) { + tests := []struct { + name string + baseURL string + httpClient *mockClient + resBody string + wantErrMsg string + }{ + { + name: "invalid_url", + baseURL: "%", + httpClient: &mockClient{}, + wantErrMsg: "failed to create request:", + }, + { + name: "failed_to_call_api", + httpClient: &mockClient{err: fmt.Errorf("something went wrong")}, + wantErrMsg: "failed to do request:", + }, + { + name: "not_json_error_response", + httpClient: &mockClient{ + res: &http.Response{ + StatusCode: http.StatusBadRequest, + Body: ioutil.NopCloser(strings.NewReader("")), + }}, + wantErrMsg: "failed to decode error response:", + }, + { + name: "not_json_success_response", + httpClient: &mockClient{ + res: &http.Response{ + StatusCode: http.StatusCreated, + Body: ioutil.NopCloser(strings.NewReader("")), + }}, + wantErrMsg: "failed to decode response:", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := New(tt.baseURL, "token", "projectID", tt.httpClient) + + res, err := client.CreateRelease(context.Background(), &CreateReleaseRequest{}) + require.Nil(t, res) + require.Error(t, err) + + require.Contains(t, err.Error(), tt.wantErrMsg) + }) + } +} -- GitLab From 289b35f1aa3130ea6b03a0ede26390dcc0009e50 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Wed, 4 Mar 2020 15:51:15 +1100 Subject: [PATCH 2/8] Add testdata folder to gitlab package Add issue url for TODO Add reference to Releases API documentation Remove unused structs in the Release response body for now --- internal/gitlab/client.go | 4 +- internal/gitlab/release.go | 99 ++++--------------- internal/gitlab/release_test.go | 14 +-- .../create_release_responses.go} | 13 +-- 4 files changed, 37 insertions(+), 93 deletions(-) rename internal/gitlab/{data_test.go => testdata/create_release_responses.go} (93%) diff --git a/internal/gitlab/client.go b/internal/gitlab/client.go index 4adf5f5..f7862ef 100644 --- a/internal/gitlab/client.go +++ b/internal/gitlab/client.go @@ -13,7 +13,9 @@ import ( type ErrorResponse struct { statusCode int Message string `json:"message,omitempty"` - Err string `json:"error,omitempty"` // TODO the API returns different fields for some different status codes + // TODO the API returns different fields for some different status codes + // https://gitlab.com/gitlab-org/gitlab-releaser/issues/18 + Err string `json:"error,omitempty"` } // Error implements the error interface. Wraps an error message from the API into an error type diff --git a/internal/gitlab/release.go b/internal/gitlab/release.go index a3f98d1..75fe4c2 100644 --- a/internal/gitlab/release.go +++ b/internal/gitlab/release.go @@ -9,89 +9,28 @@ import ( "time" ) -// CreateReleaseRequest body +// CreateReleaseRequest body. +// The full documentation can be found at https://docs.gitlab.com/ee/api/releases/index.html#create-a-release type CreateReleaseRequest struct { - ID string `json:"id"` - Name string `json:"name"` - Description string `json:"description"` - TagName string `json:"tag_name"` - Ref string `json:"ref,omitempty"` - Milestones []string `json:"milestones,omitempty"` - Assets *Assets `json:"assets,omitempty"` - ReleasedAt *time.Time `json:"released_at,omitempty"` + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + TagName string `json:"tag_name"` + Ref string `json:"ref,omitempty"` + // Milestones []string `json:"milestones,omitempty"` + // Assets *Assets `json:"assets,omitempty"` + // ReleasedAt *time.Time `json:"released_at,omitempty"` } -// CreateReleaseResponse full body +// CreateReleaseResponse body. +// The full documentation can be found at https://docs.gitlab.com/ee/api/releases/index.html#create-a-release type CreateReleaseResponse struct { - Name string `json:"name"` - Description string `json:"description"` - DescriptionHTML string `json:"description_html"` - TagName string `json:"tag_name"` - CreatedAt time.Time `json:"created_at"` - ReleasedAt time.Time `json:"released_at"` - Assets *Assets `json:"assets"` - Author *Author `json:"author"` - Milestones []*Milestone `json:"milestones"` - CommitPath string `json:"commit_path"` - TagPath string `json:"tag_path"` - EvidenceSha string `json:"evidence_sha"` -} - -// Author of the release -type Author struct { - ID int `json:"id"` - Name string `json:"name"` - Username string `json:"username"` - State string `json:"state"` - AvatarURL string `json:"avatar_url"` - WebURL string `json:"web_url"` -} - -// Commit metadata -type Commit struct { - ID string `json:"id"` - ShortID string `json:"short_id"` - Title string `json:"title"` - CreatedAt time.Time `json:"created_at"` - ParentIds []string `json:"parent_ids"` - Message string `json:"message"` - AuthorName string `json:"author_name"` - AuthorEmail string `json:"author_email"` - AuthoredDate time.Time `json:"authored_date"` - CommitterName string `json:"committer_name"` - CommitterEmail string `json:"committer_email"` - CommittedDate time.Time `json:"committed_date"` -} - -// Milestone information -type Milestone struct { - ID int `json:"id"` - Iid int `json:"iid"` - ProjectID int `json:"project_id"` - Title string `json:"title"` - Description string `json:"description"` - State string `json:"state"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - DueDate time.Time `json:"due_date"` - StartDate time.Time `json:"start_date"` - WebURL string `json:"web_url"` -} - -// Assets of a release -type Assets struct { - Count int64 `json:"count,omitempty"` - Sources []struct { - Format string `json:"format"` - URL string `json:"url"` - } `json:"sources,omitempty"` - Links []struct { - ID string `json:"id"` - Name string `json:"name"` - URL string `json:"url"` - External bool `json:"external"` - } `json:"links,omitempty"` - EvidenceFilePath string `json:"evidence_file_path,omitempty"` + Name string `json:"name"` + Description string `json:"description"` + DescriptionHTML string `json:"description_html"` + TagName string `json:"tag_name"` + CreatedAt time.Time `json:"created_at"` + ReleasedAt time.Time `json:"released_at"` } // CreateRelease will try to create a release via GitLab's Releases API @@ -101,7 +40,7 @@ func (gc *Client) CreateRelease(ctx context.Context, createReleaseReq *CreateRel return nil, fmt.Errorf("failed to marshal request body: %w", err) } - req, err := gc.request(ctx, "POST", "/projects/"+gc.projectID+"/releases", bytes.NewBuffer(body)) + req, err := gc.request(ctx, http.MethodPost, fmt.Sprintf("/projects/%s/releases", gc.projectID), bytes.NewBuffer(body)) if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) } diff --git a/internal/gitlab/release_test.go b/internal/gitlab/release_test.go index 2336c46..1f984d7 100644 --- a/internal/gitlab/release_test.go +++ b/internal/gitlab/release_test.go @@ -10,6 +10,8 @@ import ( "testing" "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-releaser/internal/gitlab/testdata" ) func TestClient_CreateRelease(t *testing.T) { @@ -33,12 +35,12 @@ func TestClient_CreateRelease(t *testing.T) { mockClient: &mockClient{ res: &http.Response{ StatusCode: http.StatusOK, - Body: ioutil.NopCloser(strings.NewReader(createReleaseSuccessResponse)), + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseSuccessResponse)), }, }, wantResponse: func() *CreateReleaseResponse { var res CreateReleaseResponse - err := json.Unmarshal([]byte(createReleaseSuccessResponse), &res) + err := json.Unmarshal([]byte(testdata.CreateReleaseSuccessResponse), &res) require.NoError(t, err) return &res }(), @@ -48,7 +50,7 @@ func TestClient_CreateRelease(t *testing.T) { mockClient: &mockClient{ res: &http.Response{ StatusCode: http.StatusForbidden, - Body: ioutil.NopCloser(strings.NewReader(createReleaseForbiddenResponse)), + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseForbiddenResponse)), }, }, wantErrResponse: &ErrorResponse{statusCode: http.StatusForbidden, Message: "403 Forbidden"}, @@ -58,7 +60,7 @@ func TestClient_CreateRelease(t *testing.T) { mockClient: &mockClient{ res: &http.Response{ StatusCode: http.StatusBadRequest, - Body: ioutil.NopCloser(strings.NewReader(createReleaseBadRequestResponse)), + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseBadRequestResponse)), }, }, wantErrResponse: &ErrorResponse{statusCode: http.StatusBadRequest, Err: "tag_name is missing, description is missing"}, @@ -68,7 +70,7 @@ func TestClient_CreateRelease(t *testing.T) { mockClient: &mockClient{ res: &http.Response{ StatusCode: http.StatusConflict, - Body: ioutil.NopCloser(strings.NewReader(createReleaseConflictResponse)), + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseConflictResponse)), }, }, wantErrResponse: &ErrorResponse{statusCode: http.StatusConflict, Message: "Release already exists"}, @@ -78,7 +80,7 @@ func TestClient_CreateRelease(t *testing.T) { mockClient: &mockClient{ res: &http.Response{ StatusCode: http.StatusInternalServerError, - Body: ioutil.NopCloser(strings.NewReader(createReleaseInternalErrorResponse)), + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseInternalErrorResponse)), }, }, wantErrResponse: &ErrorResponse{statusCode: http.StatusInternalServerError, Message: "500 Internal Server Error"}, diff --git a/internal/gitlab/data_test.go b/internal/gitlab/testdata/create_release_responses.go similarity index 93% rename from internal/gitlab/data_test.go rename to internal/gitlab/testdata/create_release_responses.go index 6decbb2..0892f6a 100644 --- a/internal/gitlab/data_test.go +++ b/internal/gitlab/testdata/create_release_responses.go @@ -1,6 +1,6 @@ -package gitlab +package testdata -const createReleaseSuccessResponse = ` +const CreateReleaseSuccessResponse = ` { "tag_name":"v0.1", "description":"## CHANGELOG\r\n\r\n- Remove limit of 100 when searching repository code. !8671\r\n- Show error message when attempting to reopen an MR and there is an open MR for the same branch. !16447 (Akos Gyimesi)\r\n- Fix a bug where internal email pattern wasn't respected. !22516", @@ -23,6 +23,7 @@ const createReleaseSuccessResponse = ` "created_at":"2019-01-03T01:53:28.000Z", "parent_ids":[ + ], "message":"Initial commit", "author_name":"Administrator", @@ -78,25 +79,25 @@ const createReleaseSuccessResponse = ` } ` -const createReleaseForbiddenResponse = ` +const CreateReleaseForbiddenResponse = ` { "message":"403 Forbidden" } ` -const createReleaseBadRequestResponse = ` +const CreateReleaseBadRequestResponse = ` { "error":"tag_name is missing, description is missing" } ` -const createReleaseConflictResponse = ` +const CreateReleaseConflictResponse = ` { "message":"Release already exists" } ` -const createReleaseInternalErrorResponse = ` +const CreateReleaseInternalErrorResponse = ` { "message":"500 Internal Server Error" } -- GitLab From b94a8989db41894698ddf08ec8de2d2479fdf232 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Wed, 4 Mar 2020 16:46:17 +1100 Subject: [PATCH 3/8] Use mockery to generate mocks Use testify/mock for mocking Update Makefile with mockery steps --- Makefile.util.mk | 17 ++++- go.sum | 1 + internal/gitlab/client_test.go | 13 +--- internal/gitlab/mock_httpClient_test.go | 36 +++++++++++ internal/gitlab/release_test.go | 83 +++++++++++-------------- 5 files changed, 91 insertions(+), 59 deletions(-) create mode 100644 internal/gitlab/mock_httpClient_test.go diff --git a/Makefile.util.mk b/Makefile.util.mk index 60fe061..080c1d6 100644 --- a/Makefile.util.mk +++ b/Makefile.util.mk @@ -1,6 +1,11 @@ GOLANGCI_LINT_IMAGE := registry.gitlab.com/gitlab-org/gitlab-build-images:golangci-lint-alpine -.PHONY: lint test cover list deps-check deps-download +MOCKERY = mockery +DEVELOPMENT_TOOLS = $(MOCKERY) + +MOCKERY_FLAGS = -note="This comment works around https://github.com/vektra/mockery/issues/155" + +.PHONY: lint test cover list deps-check mocks lint: docker run -v $(PWD):/app -w /app $(GOLANGCI_LINT_IMAGE) sh -c "golangci-lint run --out-format code-climate $(if $V,-v) | tee gl-code-quality-report.json | jq -r '.[] | \"\(.location.path):\(.location.lines.begin) \(.description)\"'" @@ -13,7 +18,7 @@ test-race: # The acceptance tests cannot count for coverage -cover: setup +cover: setup mocks @echo "NOTE: make cover does not exit 1 on failure, don't use it to check for tests success!" $Q rm -f cover/*.out cover/all.merged $Q go test ./... -coverprofile=cover/unit.out @@ -37,3 +42,11 @@ deps-check: exit 1; \ fi; + +# development tools +mocks: $(MOCKERY) + rm -rf ./helpers/service/mocks + mockery $(MOCKERY_FLAGS) -dir=./internal/gitlab/ -all -inpkg -testonly + +$(MOCKERY): + go get github.com/vektra/mockery/cmd/mockery diff --git a/go.sum b/go.sum index d3a27e1..a1ba404 100644 --- a/go.sum +++ b/go.sum @@ -13,6 +13,7 @@ github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5I github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= +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.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= diff --git a/internal/gitlab/client_test.go b/internal/gitlab/client_test.go index 8e30a34..0a031ac 100644 --- a/internal/gitlab/client_test.go +++ b/internal/gitlab/client_test.go @@ -2,7 +2,6 @@ package gitlab import ( "context" - "net/http" "testing" "github.com/stretchr/testify/require" @@ -31,7 +30,7 @@ func TestClient_request(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gc := New(tt.baseURL, tt.token, tt.projectID, &mockClient{}) + gc := New(tt.baseURL, tt.token, tt.projectID, &mockHttpClient{}) got, err := gc.request(context.Background(), tt.method, tt.baseURL, nil) if tt.wantErrMsg != "" { require.EqualError(t, err, tt.wantErrMsg) @@ -45,13 +44,3 @@ func TestClient_request(t *testing.T) { }) } } - -// mockClient very simple mocking function -type mockClient struct { - res *http.Response - err error -} - -func (mc *mockClient) Do(r *http.Request) (*http.Response, error) { - return mc.res, mc.err -} diff --git a/internal/gitlab/mock_httpClient_test.go b/internal/gitlab/mock_httpClient_test.go new file mode 100644 index 0000000..f236148 --- /dev/null +++ b/internal/gitlab/mock_httpClient_test.go @@ -0,0 +1,36 @@ +// Code generated by mockery v1.0.0. DO NOT EDIT. + +// This comment works around https://github.com/vektra/mockery/issues/155 + +package gitlab + +import http "net/http" +import mock "github.com/stretchr/testify/mock" + +// mockHttpClient is an autogenerated mock type for the httpClient type +type mockHttpClient struct { + mock.Mock +} + +// Do provides a mock function with given fields: _a0 +func (_m *mockHttpClient) Do(_a0 *http.Request) (*http.Response, error) { + ret := _m.Called(_a0) + + var r0 *http.Response + if rf, ok := ret.Get(0).(func(*http.Request) *http.Response); ok { + r0 = rf(_a0) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*http.Response) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*http.Request) error); ok { + r1 = rf(_a0) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/internal/gitlab/release_test.go b/internal/gitlab/release_test.go index 1f984d7..fad851f 100644 --- a/internal/gitlab/release_test.go +++ b/internal/gitlab/release_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-releaser/internal/gitlab/testdata" @@ -24,19 +25,16 @@ func TestClient_CreateRelease(t *testing.T) { } tests := []struct { - name string - createReleaseRequest *CreateReleaseRequest - mockClient *mockClient - wantResponse *CreateReleaseResponse - wantErrResponse *ErrorResponse + name string + res *http.Response + wantResponse *CreateReleaseResponse + wantErrResponse *ErrorResponse }{ { name: "success", - mockClient: &mockClient{ - res: &http.Response{ - StatusCode: http.StatusOK, - Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseSuccessResponse)), - }, + res: &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseSuccessResponse)), }, wantResponse: func() *CreateReleaseResponse { var res CreateReleaseResponse @@ -47,52 +45,47 @@ func TestClient_CreateRelease(t *testing.T) { }, { name: "forbidden", - mockClient: &mockClient{ - res: &http.Response{ - StatusCode: http.StatusForbidden, - Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseForbiddenResponse)), - }, + res: &http.Response{ + StatusCode: http.StatusForbidden, + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseForbiddenResponse)), }, wantErrResponse: &ErrorResponse{statusCode: http.StatusForbidden, Message: "403 Forbidden"}, }, { name: "bad_request", - mockClient: &mockClient{ - res: &http.Response{ - StatusCode: http.StatusBadRequest, - Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseBadRequestResponse)), - }, + res: &http.Response{ + StatusCode: http.StatusBadRequest, + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseBadRequestResponse)), }, wantErrResponse: &ErrorResponse{statusCode: http.StatusBadRequest, Err: "tag_name is missing, description is missing"}, }, { name: "conflict", - mockClient: &mockClient{ - res: &http.Response{ - StatusCode: http.StatusConflict, - Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseConflictResponse)), - }, + res: &http.Response{ + StatusCode: http.StatusConflict, + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseConflictResponse)), }, wantErrResponse: &ErrorResponse{statusCode: http.StatusConflict, Message: "Release already exists"}, }, { name: "internal_server_error", - mockClient: &mockClient{ - res: &http.Response{ - StatusCode: http.StatusInternalServerError, - Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseInternalErrorResponse)), - }, + res: &http.Response{ + StatusCode: http.StatusInternalServerError, + Body: ioutil.NopCloser(strings.NewReader(testdata.CreateReleaseInternalErrorResponse)), }, wantErrResponse: &ErrorResponse{statusCode: http.StatusInternalServerError, Message: "500 Internal Server Error"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + mhc := &mockHttpClient{} + mhc.On("Do", mock.Anything).Return(tt.res, nil) + gc := &Client{ baseURL: "http://127.0.0.1", token: "privateToken", projectID: "projectID", - httpClient: tt.mockClient, + httpClient: mhc, } got, err := gc.CreateRelease(context.Background(), &baseCRR) if tt.wantErrResponse != nil { @@ -112,44 +105,44 @@ func TestClient_CreateRelease_NonAPIErrors(t *testing.T) { tests := []struct { name string baseURL string - httpClient *mockClient + res *http.Response + err error resBody string wantErrMsg string }{ { name: "invalid_url", baseURL: "%", - httpClient: &mockClient{}, wantErrMsg: "failed to create request:", }, { name: "failed_to_call_api", - httpClient: &mockClient{err: fmt.Errorf("something went wrong")}, + err: fmt.Errorf("something went wrong"), wantErrMsg: "failed to do request:", }, { name: "not_json_error_response", - httpClient: &mockClient{ - res: &http.Response{ - StatusCode: http.StatusBadRequest, - Body: ioutil.NopCloser(strings.NewReader("")), - }}, + res: &http.Response{ + StatusCode: http.StatusBadRequest, + Body: ioutil.NopCloser(strings.NewReader("")), + }, wantErrMsg: "failed to decode error response:", }, { name: "not_json_success_response", - httpClient: &mockClient{ - res: &http.Response{ - StatusCode: http.StatusCreated, - Body: ioutil.NopCloser(strings.NewReader("")), - }}, + res: &http.Response{ + StatusCode: http.StatusCreated, + Body: ioutil.NopCloser(strings.NewReader("")), + }, wantErrMsg: "failed to decode response:", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - client := New(tt.baseURL, "token", "projectID", tt.httpClient) + mhc := &mockHttpClient{} + mhc.On("Do", mock.Anything).Return(tt.res, tt.err) + client := New(tt.baseURL, "token", "projectID", mhc) res, err := client.CreateRelease(context.Background(), &CreateReleaseRequest{}) require.Nil(t, res) -- GitLab From c9dad2207be46ec2fe458f59f0a0fd00ee7001cc Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Wed, 4 Mar 2020 16:52:41 +1100 Subject: [PATCH 4/8] Assert mock expectations --- Makefile.util.mk | 2 +- internal/gitlab/release_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile.util.mk b/Makefile.util.mk index 080c1d6..179b274 100644 --- a/Makefile.util.mk +++ b/Makefile.util.mk @@ -18,7 +18,7 @@ test-race: # The acceptance tests cannot count for coverage -cover: setup mocks +cover: setup @echo "NOTE: make cover does not exit 1 on failure, don't use it to check for tests success!" $Q rm -f cover/*.out cover/all.merged $Q go test ./... -coverprofile=cover/unit.out diff --git a/internal/gitlab/release_test.go b/internal/gitlab/release_test.go index fad851f..db5f1d9 100644 --- a/internal/gitlab/release_test.go +++ b/internal/gitlab/release_test.go @@ -97,6 +97,8 @@ func TestClient_CreateRelease(t *testing.T) { require.NoError(t, err) require.NotNil(t, got) + mhc.AssertExpectations(t) + }) } } @@ -149,6 +151,10 @@ func TestClient_CreateRelease_NonAPIErrors(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), tt.wantErrMsg) + + if tt.res != nil { + mhc.AssertExpectations(t) + } }) } } -- GitLab From e73731d2f898d0358e8451baa7342d3d230c0fa6 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Wed, 4 Mar 2020 16:55:59 +1100 Subject: [PATCH 5/8] Add issue url to TODO for private token Fix lint issue --- internal/gitlab/client.go | 1 + internal/gitlab/release_test.go | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gitlab/client.go b/internal/gitlab/client.go index f7862ef..33c8eda 100644 --- a/internal/gitlab/client.go +++ b/internal/gitlab/client.go @@ -59,6 +59,7 @@ func (gc *Client) request(ctx context.Context, method, url string, body io.Reade req = req.WithContext(ctx) // TODO support `PRIVATE-TOKEN` when the cli is not used in CI + // https://gitlab.com/gitlab-org/gitlab-releaser/issues/11 req.Header.Set("JOB-TOKEN", gc.token) req.Header.Set("Content-Type", "application/json") diff --git a/internal/gitlab/release_test.go b/internal/gitlab/release_test.go index db5f1d9..f4db1ee 100644 --- a/internal/gitlab/release_test.go +++ b/internal/gitlab/release_test.go @@ -98,7 +98,6 @@ func TestClient_CreateRelease(t *testing.T) { require.NoError(t, err) require.NotNil(t, got) mhc.AssertExpectations(t) - }) } } -- GitLab From 689a21107026b9f3677913f1476f3e5c7f412812 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Thu, 5 Mar 2020 11:44:07 +1100 Subject: [PATCH 6/8] Update stages to build first checking dependencies and mocks as well Add tools.go to main package to help track dev tools Update tests with mocks and assert being called --- .gitlab-ci.yml | 4 +++- Makefile.util.mk | 14 +++++++++++--- cmd/gitlab-releaser/tools.go | 10 ++++++++++ go.mod | 1 + go.sum | 4 ++++ internal/gitlab/mock_httpClient_test.go | 2 -- internal/gitlab/release.go | 3 --- internal/gitlab/release_test.go | 4 ++-- 8 files changed, 31 insertions(+), 11 deletions(-) create mode 100644 cmd/gitlab-releaser/tools.go diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4d60f8e..2749298 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -17,8 +17,8 @@ variables: image: golang:${GO} stages: - - test - build + - test - security license_management: @@ -31,6 +31,8 @@ build: extends: .go-cache stage: build script: + - make mocks-check + - make deps-check - make build artifacts: paths: diff --git a/Makefile.util.mk b/Makefile.util.mk index 179b274..ffc9b57 100644 --- a/Makefile.util.mk +++ b/Makefile.util.mk @@ -3,7 +3,6 @@ GOLANGCI_LINT_IMAGE := registry.gitlab.com/gitlab-org/gitlab-build-images:golang MOCKERY = mockery DEVELOPMENT_TOOLS = $(MOCKERY) -MOCKERY_FLAGS = -note="This comment works around https://github.com/vektra/mockery/issues/155" .PHONY: lint test cover list deps-check mocks @@ -42,11 +41,20 @@ deps-check: exit 1; \ fi; +mocks-check: mocks + @if git diff --color=always --exit-code -- *mock_*; then \ + echo "mocks are ok"; \ + else \ + echo ""; \ + echo "mocks are modified, please commit them";\ + exit 1; \ + fi; # development tools mocks: $(MOCKERY) - rm -rf ./helpers/service/mocks - mockery $(MOCKERY_FLAGS) -dir=./internal/gitlab/ -all -inpkg -testonly + find . -type f ! -path '*vendor/*' -name 'mock_*' -delete && \ + ${GOPATH}/bin/mockery -dir=./internal/gitlab/ -all -inpkg -testonly $(MOCKERY): go get github.com/vektra/mockery/cmd/mockery + diff --git a/cmd/gitlab-releaser/tools.go b/cmd/gitlab-releaser/tools.go new file mode 100644 index 0000000..d45a18f --- /dev/null +++ b/cmd/gitlab-releaser/tools.go @@ -0,0 +1,10 @@ +// +build tools + +package main + +// These imports are to force `go mod tidy` not to remove that tools we depend +// on development. This is explained in great detail in +// https://marcofranssen.nl/manage-go-tools-via-go-modules/ +import ( + _ "github.com/vektra/mockery/cmd/mockery" +) diff --git a/go.mod b/go.mod index 76390b7..0e647a3 100644 --- a/go.mod +++ b/go.mod @@ -6,4 +6,5 @@ require ( github.com/sirupsen/logrus v1.4.2 github.com/stretchr/testify v1.2.2 github.com/urfave/cli/v2 v2.1.1 + github.com/vektra/mockery v0.0.0-20181123154057-e78b021dcbb5 ) diff --git a/go.sum b/go.sum index a1ba404..2f72895 100644 --- a/go.sum +++ b/go.sum @@ -19,7 +19,11 @@ github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/urfave/cli/v2 v2.1.1 h1:Qt8FeAtxE/vfdrLmR3rxR6JRE0RoVmbXu8+6kZtYU4k= github.com/urfave/cli/v2 v2.1.1/go.mod h1:SE9GqnLQmjVa0iPEY0f1w3ygNIYcIJ0OKPMoW2caLfQ= +github.com/vektra/mockery v0.0.0-20181123154057-e78b021dcbb5 h1:Xim2mBRFdXzXmKRO8DJg/FJtn/8Fj9NOEpO6+WuMPmk= +github.com/vektra/mockery v0.0.0-20181123154057-e78b021dcbb5/go.mod h1:ppEjwdhyy7Y31EnHRDm1JkChoC7LXIJ7Ex0VYLWtZtQ= golang.org/x/sys v0.0.0-20190422165155-953cdadca894 h1:Cz4ceDQGXuKRnVBDTS23GTn/pU5OE2C0WrNTOYK1Uuc= golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/tools v0.0.0-20181112210238-4b1f3b6b1646 h1:JEEoTsNEpPwxsebhPLC6P2jNr+6RFZLY4elUBVcMb+I= +golang.org/x/tools v0.0.0-20181112210238-4b1f3b6b1646/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/internal/gitlab/mock_httpClient_test.go b/internal/gitlab/mock_httpClient_test.go index f236148..116500b 100644 --- a/internal/gitlab/mock_httpClient_test.go +++ b/internal/gitlab/mock_httpClient_test.go @@ -1,7 +1,5 @@ // Code generated by mockery v1.0.0. DO NOT EDIT. -// This comment works around https://github.com/vektra/mockery/issues/155 - package gitlab import http "net/http" diff --git a/internal/gitlab/release.go b/internal/gitlab/release.go index 75fe4c2..00ef0b0 100644 --- a/internal/gitlab/release.go +++ b/internal/gitlab/release.go @@ -17,9 +17,6 @@ type CreateReleaseRequest struct { Description string `json:"description"` TagName string `json:"tag_name"` Ref string `json:"ref,omitempty"` - // Milestones []string `json:"milestones,omitempty"` - // Assets *Assets `json:"assets,omitempty"` - // ReleasedAt *time.Time `json:"released_at,omitempty"` } // CreateReleaseResponse body. diff --git a/internal/gitlab/release_test.go b/internal/gitlab/release_test.go index f4db1ee..3fa840c 100644 --- a/internal/gitlab/release_test.go +++ b/internal/gitlab/release_test.go @@ -79,7 +79,7 @@ func TestClient_CreateRelease(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mhc := &mockHttpClient{} - mhc.On("Do", mock.Anything).Return(tt.res, nil) + mhc.On("Do", mock.Anything).Return(tt.res, nil).Once() gc := &Client{ baseURL: "http://127.0.0.1", @@ -142,7 +142,7 @@ func TestClient_CreateRelease_NonAPIErrors(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mhc := &mockHttpClient{} - mhc.On("Do", mock.Anything).Return(tt.res, tt.err) + mhc.On("Do", mock.Anything).Return(tt.res, tt.err).Once() client := New(tt.baseURL, "token", "projectID", mhc) res, err := client.CreateRelease(context.Background(), &CreateReleaseRequest{}) -- GitLab From b473f8ce5eb1151502b358c535f71fd0b38eeb6c Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Thu, 5 Mar 2020 12:27:51 +1100 Subject: [PATCH 7/8] Makfiles formatting --- Makefile.build.mk | 1 + Makefile.util.mk | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile.build.mk b/Makefile.build.mk index 328b7a3..d4003a4 100644 --- a/Makefile.build.mk +++ b/Makefile.build.mk @@ -25,6 +25,7 @@ build: setup run: setup $Q go run cmd/$(PROJECT_NAME)/main.go + clean: $Q rm -rf bin/* $Q rm -rf cover/* diff --git a/Makefile.util.mk b/Makefile.util.mk index ffc9b57..9a70a9d 100644 --- a/Makefile.util.mk +++ b/Makefile.util.mk @@ -1,9 +1,7 @@ GOLANGCI_LINT_IMAGE := registry.gitlab.com/gitlab-org/gitlab-build-images:golangci-lint-alpine - MOCKERY = mockery DEVELOPMENT_TOOLS = $(MOCKERY) - .PHONY: lint test cover list deps-check mocks lint: @@ -30,7 +28,6 @@ cover: setup list: @echo $(allpackages) - deps-check: go mod tidy @if git diff --color=always --exit-code -- go.mod go.sum; then \ -- GitLab From 0b8f6df685b19ce642e730e8caa97820f176e08b Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Tue, 10 Mar 2020 09:56:45 +1100 Subject: [PATCH 8/8] Expose HTTPClient interface Export the HTTPClient interface since it's being exposed in the New func. Regenerate mocks and use snake_case for file name. Update ErrorResponse in client to DRY a bit. --- Makefile.util.mk | 2 +- internal/gitlab/client.go | 17 +++++++++++------ internal/gitlab/client_test.go | 2 +- ...pClient_test.go => mock_http_client_test.go} | 6 +++--- internal/gitlab/release_test.go | 4 ++-- 5 files changed, 18 insertions(+), 13 deletions(-) rename internal/gitlab/{mock_httpClient_test.go => mock_http_client_test.go} (78%) diff --git a/Makefile.util.mk b/Makefile.util.mk index 9a70a9d..d6969b0 100644 --- a/Makefile.util.mk +++ b/Makefile.util.mk @@ -50,7 +50,7 @@ mocks-check: mocks # development tools mocks: $(MOCKERY) find . -type f ! -path '*vendor/*' -name 'mock_*' -delete && \ - ${GOPATH}/bin/mockery -dir=./internal/gitlab/ -all -inpkg -testonly + ${GOPATH}/bin/mockery -dir=./internal/gitlab/ -all -inpkg -testonly -case snake $(MOCKERY): go get github.com/vektra/mockery/cmd/mockery diff --git a/internal/gitlab/client.go b/internal/gitlab/client.go index 33c8eda..24eb7a2 100644 --- a/internal/gitlab/client.go +++ b/internal/gitlab/client.go @@ -20,27 +20,32 @@ type ErrorResponse struct { // Error implements the error interface. Wraps an error message from the API into an error type func (er *ErrorResponse) Error() string { + err := fmt.Sprintf("API Error Response status_code: %d message: %s", er.statusCode, er.Message) + if er.Err != "" { - return fmt.Sprintf("API Error Response status_code: %d message: %s error: %s", er.statusCode, er.Message, er.Err) + return fmt.Sprintf("%s error %s", err, er.Err) } - return fmt.Sprintf("API Error Response status_code: %d message: %s", er.statusCode, er.Message) + return err } -type httpClient interface { +// HTTPClient is an interface that describes the available actions of the client. +// Use http.Client during runtime. +// See mock_httpClient_test.go for a testing implementation +type HTTPClient interface { Do(*http.Request) (*http.Response, error) } -// Client contains the GitLab client configuration +// Client is used to send requests to the GitLab API. Normally created with the `New` function type Client struct { baseURL string token string projectID string - httpClient httpClient + httpClient HTTPClient } // New creates a new GitLab Client -func New(baseURL, token, projectID string, httpClient httpClient) *Client { +func New(baseURL, token, projectID string, httpClient HTTPClient) *Client { return &Client{ baseURL: baseURL, token: token, diff --git a/internal/gitlab/client_test.go b/internal/gitlab/client_test.go index 0a031ac..c92781a 100644 --- a/internal/gitlab/client_test.go +++ b/internal/gitlab/client_test.go @@ -30,7 +30,7 @@ func TestClient_request(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gc := New(tt.baseURL, tt.token, tt.projectID, &mockHttpClient{}) + gc := New(tt.baseURL, tt.token, tt.projectID, &MockHTTPClient{}) got, err := gc.request(context.Background(), tt.method, tt.baseURL, nil) if tt.wantErrMsg != "" { require.EqualError(t, err, tt.wantErrMsg) diff --git a/internal/gitlab/mock_httpClient_test.go b/internal/gitlab/mock_http_client_test.go similarity index 78% rename from internal/gitlab/mock_httpClient_test.go rename to internal/gitlab/mock_http_client_test.go index 116500b..2ffb8d5 100644 --- a/internal/gitlab/mock_httpClient_test.go +++ b/internal/gitlab/mock_http_client_test.go @@ -5,13 +5,13 @@ package gitlab import http "net/http" import mock "github.com/stretchr/testify/mock" -// mockHttpClient is an autogenerated mock type for the httpClient type -type mockHttpClient struct { +// MockHTTPClient is an autogenerated mock type for the HTTPClient type +type MockHTTPClient struct { mock.Mock } // Do provides a mock function with given fields: _a0 -func (_m *mockHttpClient) Do(_a0 *http.Request) (*http.Response, error) { +func (_m *MockHTTPClient) Do(_a0 *http.Request) (*http.Response, error) { ret := _m.Called(_a0) var r0 *http.Response diff --git a/internal/gitlab/release_test.go b/internal/gitlab/release_test.go index 3fa840c..4ed7bd6 100644 --- a/internal/gitlab/release_test.go +++ b/internal/gitlab/release_test.go @@ -78,7 +78,7 @@ func TestClient_CreateRelease(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mhc := &mockHttpClient{} + mhc := &MockHTTPClient{} mhc.On("Do", mock.Anything).Return(tt.res, nil).Once() gc := &Client{ @@ -141,7 +141,7 @@ func TestClient_CreateRelease_NonAPIErrors(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mhc := &mockHttpClient{} + mhc := &MockHTTPClient{} mhc.On("Do", mock.Anything).Return(tt.res, tt.err).Once() client := New(tt.baseURL, "token", "projectID", mhc) -- GitLab