From c477dec8a1307baf942a74ae6d5be98292f1759c Mon Sep 17 00:00:00 2001 From: Noam Gal Date: Thu, 8 Sep 2022 15:16:10 +0300 Subject: [PATCH 1/2] removed cloneURL from createrRepo returned values --- pkg/git/provider.go | 2 +- pkg/git/provider_ado.go | 8 +-- pkg/git/provider_ado_test.go | 34 ++++++------- pkg/git/provider_bitbucket-server.go | 15 ++---- pkg/git/provider_bitbucket-server_test.go | 52 ++++---------------- pkg/git/provider_bitbucket.go | 21 ++------ pkg/git/provider_bitbucket_test.go | 34 ++++--------- pkg/git/provider_gitea.go | 12 ++--- pkg/git/provider_gitea_test.go | 18 +++---- pkg/git/provider_github.go | 16 +++--- pkg/git/provider_github_test.go | 40 +++------------ pkg/git/provider_gitlab.go | 16 +++--- pkg/git/provider_gitlab_test.go | 59 ++++++----------------- pkg/git/repository.go | 7 ++- pkg/git/repository_test.go | 36 ++++++-------- 15 files changed, 112 insertions(+), 258 deletions(-) diff --git a/pkg/git/provider.go b/pkg/git/provider.go index c1396e2b..d9474493 100644 --- a/pkg/git/provider.go +++ b/pkg/git/provider.go @@ -13,7 +13,7 @@ type ( Provider interface { // CreateRepository creates the repository in the remote provider and returns a // clone url - CreateRepository(ctx context.Context, orgRepo string) (cloneURL, defaultBranch string, err error) + CreateRepository(ctx context.Context, orgRepo string) (defaultBranch string, err error) // GetDefaultBranch returns the default branch of the repository GetDefaultBranch(ctx context.Context, orgRepo string) (string, error) diff --git a/pkg/git/provider_ado.go b/pkg/git/provider_ado.go index 11230b71..41f7f570 100644 --- a/pkg/git/provider_ado.go +++ b/pkg/git/provider_ado.go @@ -62,9 +62,9 @@ func newAdo(opts *ProviderOptions) (Provider, error) { }, nil } -func (g *adoGit) CreateRepository(ctx context.Context, orgRepo string) (cloneURL, defaultBranch string, err error) { +func (g *adoGit) CreateRepository(ctx context.Context, orgRepo string) (defaultBranch string, err error) { if orgRepo == "" { - return "", "", fmt.Errorf("name needs to be provided to create an azure devops repository. name: '%s'", orgRepo) + return "", fmt.Errorf("name needs to be provided to create an azure devops repository. name: '%s'", orgRepo) } project := g.adoUrl.GetProjectName() @@ -76,10 +76,10 @@ func (g *adoGit) CreateRepository(ctx context.Context, orgRepo string) (cloneURL } repository, err := g.adoClient.CreateRepository(ctx, createRepositoryArgs) if err != nil { - return "", "", err + return "", err } - return *repository.RemoteUrl, *repository.DefaultBranch, nil + return *repository.DefaultBranch, nil } func (g *adoGit) GetDefaultBranch(ctx context.Context, orgRepo string) (string, error) { diff --git a/pkg/git/provider_ado_test.go b/pkg/git/provider_ado_test.go index e8bb3347..445e754a 100644 --- a/pkg/git/provider_ado_test.go +++ b/pkg/git/provider_ado_test.go @@ -15,18 +15,16 @@ import ( func Test_adoGit_CreateRepository(t *testing.T) { emptyFunc := func(client *adoMock.MockAdoClient, url *adoMock.MockAdoUrl) {} tests := []struct { - name string - mockClient func(client *adoMock.MockAdoClient, url *adoMock.MockAdoUrl) - repoName string - wantCloneURL string - wantDefaultBranch string - wantErr assert.ErrorAssertionFunc + name string + mockClient func(client *adoMock.MockAdoClient, url *adoMock.MockAdoUrl) + repoName string + want string + wantErr assert.ErrorAssertionFunc }{ { - name: "Empty Name", - mockClient: emptyFunc, - repoName: "", - wantCloneURL: "", + name: "Empty Name", + mockClient: emptyFunc, + repoName: "", wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { return true }, @@ -41,8 +39,7 @@ func Test_adoGit_CreateRepository(t *testing.T) { Times(1). Return("blah") }, - repoName: "name", - wantCloneURL: "", + repoName: "name", wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { return true }, @@ -50,7 +47,6 @@ func Test_adoGit_CreateRepository(t *testing.T) { { name: "Success creating repo", mockClient: func(client *adoMock.MockAdoClient, url *adoMock.MockAdoUrl) { - remoteURL := "https://dev.azure.com/SUB/PROJECT/_git/REPO" defaultBranch := "main" url.EXPECT().GetProjectName(). Times(1). @@ -65,7 +61,7 @@ func Test_adoGit_CreateRepository(t *testing.T) { Name: nil, ParentRepository: nil, Project: nil, - RemoteUrl: &remoteURL, + RemoteUrl: nil, Size: nil, SshUrl: nil, Url: nil, @@ -73,9 +69,8 @@ func Test_adoGit_CreateRepository(t *testing.T) { WebUrl: nil, }, nil) }, - repoName: "name", - wantCloneURL: "https://dev.azure.com/SUB/PROJECT/_git/REPO", - wantDefaultBranch: "main", + repoName: "name", + want: "main", wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { return false }, @@ -91,13 +86,12 @@ func Test_adoGit_CreateRepository(t *testing.T) { adoClient: mockClient, adoUrl: mockUrl, } - gotCloneURL, gotDefaultBranch, err := g.CreateRepository(context.Background(), tt.repoName) + got, err := g.CreateRepository(context.Background(), tt.repoName) if !tt.wantErr(t, err, fmt.Sprintf("CreateRepository - %s", tt.repoName)) { return } - assert.Equalf(t, tt.wantCloneURL, gotCloneURL, "CreateRepository - %s", tt.name) - assert.Equalf(t, tt.wantDefaultBranch, gotDefaultBranch, "CreateRepository - %s", tt.name) + assert.Equalf(t, tt.want, got, "CreateRepository - %s", tt.name) }) } } diff --git a/pkg/git/provider_bitbucket-server.go b/pkg/git/provider_bitbucket-server.go index 6a75f5ec..7e961219 100644 --- a/pkg/git/provider_bitbucket-server.go +++ b/pkg/git/provider_bitbucket-server.go @@ -94,10 +94,10 @@ func newBitbucketServer(opts *ProviderOptions) (Provider, error) { return g, nil } -func (bbs *bitbucketServer) CreateRepository(ctx context.Context, orgRepo string) (cloneURL, defaultBranch string, err error) { +func (bbs *bitbucketServer) CreateRepository(ctx context.Context, orgRepo string) (defaultBranch string, err error) { noun, owner, name, err := splitOrgRepo(orgRepo) if err != nil { - return "", "", err + return "", err } path := fmt.Sprintf("%s/%s/repos", noun, owner) @@ -107,17 +107,10 @@ func (bbs *bitbucketServer) CreateRepository(ctx context.Context, orgRepo string Scm: "git", }, repo) if err != nil { - return "", "", err - } - - for _, link := range repo.Links.Clone { - if link.Name == bbs.baseURL.Scheme { - return link.Href, repo.DefaultBranch, nil - - } + return "", err } - return "", "", fmt.Errorf("created repo did not contain a valid %s clone url", bbs.baseURL.Scheme) + return repo.DefaultBranch, nil } func (bbs *bitbucketServer) GetDefaultBranch(ctx context.Context, orgRepo string) (string, error) { diff --git a/pkg/git/provider_bitbucket-server_test.go b/pkg/git/provider_bitbucket-server_test.go index 49b956c3..0223d121 100644 --- a/pkg/git/provider_bitbucket-server_test.go +++ b/pkg/git/provider_bitbucket-server_test.go @@ -36,11 +36,10 @@ func createBody(obj interface{}) io.ReadCloser { func Test_bitbucketServer_CreateRepository(t *testing.T) { tests := map[string]struct { - orgRepo string - wantCloneURL string - wantDefaultBranch string - wantErr string - beforeFn func(t *testing.T, c *mocks.MockHttpClient) + orgRepo string + want string + wantErr string + beforeFn func(t *testing.T, c *mocks.MockHttpClient) }{ "Should fail if orgRepo is invalid": { orgRepo: "no-scm/project/repo", @@ -53,44 +52,15 @@ func Test_bitbucketServer_CreateRepository(t *testing.T) { c.EXPECT().Do(gomock.AssignableToTypeOf(&http.Request{})).Times(1).Return(nil, errors.New("some error")) }, }, - "Should fail if returned repo doesn't have clone url": { - orgRepo: "scm/project/repo", - wantErr: "created repo did not contain a valid https clone url", - beforeFn: func(_ *testing.T, c *mocks.MockHttpClient) { - repo := &repoResponse{ - Links: Links{ - Clone: []Link{ - { - Name: "ssh", - Href: "ssh@some.server/scm/project/repo.git", - }, - }, - }, - } - body := createBody(repo) - res := &http.Response{ - StatusCode: 200, - Body: body, - } - c.EXPECT().Do(gomock.AssignableToTypeOf(&http.Request{})).Times(1).Return(res, nil) - }, - }, "Should create a valid project repo": { - orgRepo: "scm/project/repo", - wantCloneURL: "https://some.server/scm/project/repo.git", + orgRepo: "scm/project/repo", + want: "main", beforeFn: func(t *testing.T, c *mocks.MockHttpClient) { c.EXPECT().Do(gomock.AssignableToTypeOf(&http.Request{})).Times(1).DoAndReturn(func(req *http.Request) (*http.Response, error) { assert.Equal(t, "POST", req.Method) assert.Equal(t, "https://some.server/rest/api/1.0/projects/project/repos", req.URL.String()) repo := &repoResponse{ - Links: Links{ - Clone: []Link{ - { - Name: "https", - Href: "https://some.server/scm/project/repo.git", - }, - }, - }, + DefaultBranch: "main", } body := createBody(repo) res := &http.Response{ @@ -102,8 +72,7 @@ func Test_bitbucketServer_CreateRepository(t *testing.T) { }, }, "Should create a valid user repo": { - orgRepo: "scm/~user/repo", - wantCloneURL: "https://some.server/scm/~user/repo.git", + orgRepo: "scm/~user/repo", beforeFn: func(t *testing.T, c *mocks.MockHttpClient) { c.EXPECT().Do(gomock.AssignableToTypeOf(&http.Request{})).Times(1).DoAndReturn(func(req *http.Request) (*http.Response, error) { assert.Equal(t, "POST", req.Method) @@ -142,14 +111,13 @@ func Test_bitbucketServer_CreateRepository(t *testing.T) { c: mockClient, opts: providerOptions, } - gotCloneURL, gotDefaultBranch, err := bbs.CreateRepository(context.Background(), tt.orgRepo) + got, err := bbs.CreateRepository(context.Background(), tt.orgRepo) if err != nil || tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) return } - assert.Equalf(t, tt.wantCloneURL, gotCloneURL, "CreateRepository - %s", name) - assert.Equalf(t, tt.wantDefaultBranch, gotDefaultBranch, "CreateRepository - %s", name) + assert.Equalf(t, tt.want, got, "CreateRepository - %s", name) }) } } diff --git a/pkg/git/provider_bitbucket.go b/pkg/git/provider_bitbucket.go index e8d2af50..135d598d 100644 --- a/pkg/git/provider_bitbucket.go +++ b/pkg/git/provider_bitbucket.go @@ -43,10 +43,10 @@ func newBitbucket(opts *ProviderOptions) (Provider, error) { } -func (g *bitbucket) CreateRepository(ctx context.Context, orgRepo string) (cloneURL, defaultBranch string, err error) { +func (g *bitbucket) CreateRepository(ctx context.Context, orgRepo string) (defaultBranch string, err error) { opts, err := getDefaultRepoOptions(orgRepo) if err != nil { - return "", "", err + return "", err } createOpts := &bb.RepositoryOptions{ @@ -62,23 +62,10 @@ func (g *bitbucket) CreateRepository(ctx context.Context, orgRepo string) (clone p, err := g.Repository.Create(createOpts) if err != nil { - return "", "", fmt.Errorf("failed creating the repository \"%s\" under \"%s\": %w", opts.Name, opts.Owner, err) - } - - var cloneUrl string - cloneLinksObj := p.Links["clone"] - for _, cloneLink := range cloneLinksObj.([]interface{}) { - link := cloneLink.(map[string]interface{}) - if link["name"].(string) == "https" { - cloneUrl = link["href"].(string) - } - } - - if cloneUrl == "" { - return "", "", fmt.Errorf("clone url is empty") + return "", fmt.Errorf("failed creating the repository \"%s\" under \"%s\": %w", opts.Name, opts.Owner, err) } - return cloneUrl, p.Mainbranch.Name, err + return p.Mainbranch.Name, err } func (g *bitbucket) GetDefaultBranch(ctx context.Context, orgRepo string) (string, error) { diff --git a/pkg/git/provider_bitbucket_test.go b/pkg/git/provider_bitbucket_test.go index 8571ba8f..e7380474 100644 --- a/pkg/git/provider_bitbucket_test.go +++ b/pkg/git/provider_bitbucket_test.go @@ -13,20 +13,18 @@ import ( func Test_bitbucket_CreateRepository(t *testing.T) { tests := map[string]struct { - orgRepo string - wantCloneURL string - wantDefaultBranch string - wantErr string - beforeRepoFn func(*bbmocks.MockbbRepo) + orgRepo string + want string + wantErr string + beforeRepoFn func(*bbmocks.MockbbRepo) }{ "Should fail if orgRepo is invalid": { orgRepo: "invalid", wantErr: "failed parsing organization and repo from 'invalid'", }, "Creates repository under user": { - orgRepo: "username/repoName", - wantCloneURL: "https://username@bitbucket.org/username/repoName.git", - wantDefaultBranch: "main", + orgRepo: "username/repoName", + want: "main", beforeRepoFn: func(c *bbmocks.MockbbRepo) { createOpts := bb.RepositoryOptions{ Owner: "username", @@ -35,24 +33,11 @@ func Test_bitbucket_CreateRepository(t *testing.T) { IsPrivate: "true", } - links := map[string]interface{}{ - "self": map[string]string{ - "href": "https://api.bitbucket.org/2.0/repositories/userName/repoName", - }, - "clone": []interface{}{ - map[string]interface{}{ - "name": "https", - "href": "https://username@bitbucket.org/username/repoName.git", - }, - }, - } - repo := &bb.Repository{ - Name: "userName", + Name: "userName", Mainbranch: bb.RepositoryBranch{ Name: "main", }, - Links: links, } c.EXPECT().Create(&createOpts). @@ -109,14 +94,13 @@ func Test_bitbucket_CreateRepository(t *testing.T) { Repository: mockRepoClient, User: mockUserClient, } - gotCloneURL, gotDefaultBranch, err := g.CreateRepository(context.Background(), tt.orgRepo) + got, err := g.CreateRepository(context.Background(), tt.orgRepo) if err != nil || tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) return } - assert.Equalf(t, tt.wantCloneURL, gotCloneURL, "CreateRepository - %s", name) - assert.Equalf(t, tt.wantDefaultBranch, gotDefaultBranch, "CreateRepository - %s",name) + assert.Equalf(t, tt.want, got, "CreateRepository - %s", name) }) } } diff --git a/pkg/git/provider_gitea.go b/pkg/git/provider_gitea.go index ff520143..32d19c4b 100644 --- a/pkg/git/provider_gitea.go +++ b/pkg/git/provider_gitea.go @@ -35,15 +35,15 @@ func newGitea(opts *ProviderOptions) (Provider, error) { return g, nil } -func (g *gitea) CreateRepository(_ context.Context, orgRepo string) (cloneURL, defaultBranch string, err error) { +func (g *gitea) CreateRepository(_ context.Context, orgRepo string) (defaultBranch string, err error) { opts, err := getDefaultRepoOptions(orgRepo) if err != nil { - return "", "", err + return "", err } authUser, err := g.getAuthenticatedUser() if err != nil { - return "", "", err + return "", err } createOpts := gt.CreateRepoOption{ @@ -63,13 +63,13 @@ func (g *gitea) CreateRepository(_ context.Context, orgRepo string) (cloneURL, d if err != nil { if res.StatusCode == 404 { - return "", "", fmt.Errorf("owner %s not found: %w", opts.Owner, err) + return "", fmt.Errorf("owner %s not found: %w", opts.Owner, err) } - return "", "", err + return "", err } - return r.CloneURL, r.DefaultBranch, nil + return r.DefaultBranch, nil } func (g *gitea) GetDefaultBranch(_ context.Context, orgRepo string) (string, error) { diff --git a/pkg/git/provider_gitea_test.go b/pkg/git/provider_gitea_test.go index b8b40700..5bde6313 100644 --- a/pkg/git/provider_gitea_test.go +++ b/pkg/git/provider_gitea_test.go @@ -14,11 +14,10 @@ import ( func Test_gitea_CreateRepository(t *testing.T) { tests := map[string]struct { - orgRepo string - beforeFn func(*gtmocks.MockClient) - wantCloneURL string - wantDefaultBranch string - wantErr string + orgRepo string + beforeFn func(*gtmocks.MockClient) + want string + wantErr string }{ "Should fail if credentials are wrong": { orgRepo: "username/repo", @@ -96,7 +95,6 @@ func Test_gitea_CreateRepository(t *testing.T) { Times(1). Return(u, nil, nil) r := >.Repository{ - CloneURL: "http://gitea.com/username/repo", DefaultBranch: "main", } createOpts := gt.CreateRepoOption{ @@ -107,8 +105,7 @@ func Test_gitea_CreateRepository(t *testing.T) { Times(1). Return(r, nil, nil) }, - wantCloneURL: "http://gitea.com/username/repo", - wantDefaultBranch: "main", + want: "main", }, } for name, tt := range tests { @@ -118,15 +115,14 @@ func Test_gitea_CreateRepository(t *testing.T) { g := &gitea{ client: mockClient, } - gotCloneURL, gotDefaultBranch, err := g.CreateRepository(context.Background(), tt.orgRepo) + got, err := g.CreateRepository(context.Background(), tt.orgRepo) if err != nil || tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) return } - assert.Equalf(t, tt.wantCloneURL, gotCloneURL, "CreateRepository - %s", name) - assert.Equalf(t, tt.wantDefaultBranch, gotDefaultBranch, "CreateRepository - %s", name) + assert.Equalf(t, tt.want, got, "CreateRepository - %s", name) }) } } diff --git a/pkg/git/provider_github.go b/pkg/git/provider_github.go index 5e131375..ce60f133 100644 --- a/pkg/git/provider_github.go +++ b/pkg/git/provider_github.go @@ -54,15 +54,15 @@ func newGithub(opts *ProviderOptions) (Provider, error) { return g, nil } -func (g *github) CreateRepository(ctx context.Context, orgRepo string) (cloneURL, defaultBranch string, err error) { +func (g *github) CreateRepository(ctx context.Context, orgRepo string) (defaultBranch string, err error) { opts, err := getDefaultRepoOptions(orgRepo) if err != nil { - return "", "", err + return "", err } authUser, err := g.getAuthenticatedUser(ctx) if err != nil { - return "", "", err + return "", err } org := "" @@ -76,17 +76,13 @@ func (g *github) CreateRepository(ctx context.Context, orgRepo string) (cloneURL }) if err != nil { if res.StatusCode == 404 { - return "", "", fmt.Errorf("owner %s not found: %w", opts.Owner, err) + return "", fmt.Errorf("owner %s not found: %w", opts.Owner, err) } - return "", "", err - } - - if r.CloneURL == nil { - return "", "", fmt.Errorf("repo clone url is nil") + return "", err } - return *r.CloneURL, *r.DefaultBranch, err + return *r.DefaultBranch, err } func (g *github) GetDefaultBranch(ctx context.Context, orgRepo string) (string, error) { diff --git a/pkg/git/provider_github_test.go b/pkg/git/provider_github_test.go index af528bea..f8ead307 100644 --- a/pkg/git/provider_github_test.go +++ b/pkg/git/provider_github_test.go @@ -14,11 +14,10 @@ import ( func Test_github_CreateRepository(t *testing.T) { tests := map[string]struct { - orgRepo string - beforeFn func(*mocks.MockRepositories, *mocks.MockUsers) - wantCloneURL string - wantDefaultBranch string - wantErr string + orgRepo string + beforeFn func(*mocks.MockRepositories, *mocks.MockUsers) + want string + wantErr string }{ "Error getting user": { orgRepo: "owner/name", @@ -61,7 +60,6 @@ func Test_github_CreateRepository(t *testing.T) { }, nil, nil) repo := &gh.Repository{ - CloneURL: gh.String("https://github.com/owner/repo"), DefaultBranch: gh.String("main"), } res := &gh.Response{Response: &http.Response{ @@ -74,8 +72,7 @@ func Test_github_CreateRepository(t *testing.T) { Times(1). Return(repo, res, nil) }, - wantCloneURL: "https://github.com/owner/repo", - wantDefaultBranch: "main", + want: "main", }, "Creates with org": { orgRepo: "org/name", @@ -87,7 +84,6 @@ func Test_github_CreateRepository(t *testing.T) { }, nil, nil) repo := &gh.Repository{ - CloneURL: gh.String("https://github.com/org/repo"), DefaultBranch: gh.String("main"), } res := &gh.Response{Response: &http.Response{ @@ -100,26 +96,7 @@ func Test_github_CreateRepository(t *testing.T) { Times(1). Return(repo, res, nil) }, - wantCloneURL: "https://github.com/org/repo", - wantDefaultBranch: "main", - }, - "Error when no cloneURL": { - orgRepo: "org/name", - beforeFn: func(mr *mocks.MockRepositories, mu *mocks.MockUsers) { - mu.EXPECT().Get(context.Background(), ""). - Times(1). - Return(&gh.User{ - Login: gh.String("owner"), - }, nil, nil) - - mr.EXPECT().Create(context.Background(), "org", &gh.Repository{ - Name: gh.String("name"), - Private: gh.Bool(true), - }). - Times(1). - Return(&gh.Repository{}, &gh.Response{Response: &http.Response{StatusCode: 200}}, nil) - }, - wantErr: "repo clone url is nil", + want: "main", }, } for name, tt := range tests { @@ -135,14 +112,13 @@ func Test_github_CreateRepository(t *testing.T) { Repositories: mockRepo, Users: mockUsers, } - gotCloneURL, gotDefaultBranch, err := g.CreateRepository(ctx, tt.orgRepo) + got, err := g.CreateRepository(ctx, tt.orgRepo) if err != nil || tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) return } - assert.Equalf(t, tt.wantCloneURL, gotCloneURL, "CreateRepository - %s", name) - assert.Equalf(t, tt.wantDefaultBranch, gotDefaultBranch, "CreateRepository - %s", name) + assert.Equalf(t, tt.want, got, "CreateRepository - %s", name) }) } } diff --git a/pkg/git/provider_gitlab.go b/pkg/git/provider_gitlab.go index 734d74fc..862fbbf9 100644 --- a/pkg/git/provider_gitlab.go +++ b/pkg/git/provider_gitlab.go @@ -49,15 +49,15 @@ func newGitlab(opts *ProviderOptions) (Provider, error) { return g, nil } -func (g *gitlab) CreateRepository(ctx context.Context, orgRepo string) (cloneURL, defaultBranch string, err error) { +func (g *gitlab) CreateRepository(ctx context.Context, orgRepo string) (defaultBranch string, err error) { opts, err := getDefaultRepoOptions(orgRepo) if err != nil { - return "", "", err + return "", err } authUser, err := g.getAuthenticatedUser() if err != nil { - return "", "", err + return "", err } createOpts := gl.CreateProjectOptions{ @@ -72,7 +72,7 @@ func (g *gitlab) CreateRepository(ctx context.Context, orgRepo string) (cloneURL if authUser.Username != opts.Owner { groupId, err := g.getGroupIdByName(opts.Owner) if err != nil { - return "", "", err + return "", err } createOpts.NamespaceID = gl.Int(groupId) @@ -80,14 +80,10 @@ func (g *gitlab) CreateRepository(ctx context.Context, orgRepo string) (cloneURL p, _, err := g.client.CreateProject(&createOpts) if err != nil { - return "", "", fmt.Errorf("failed creating the project \"%s\" under \"%s\": %w", opts.Name, opts.Owner, err) - } - - if p.HTTPURLToRepo == "" { - return "", "", fmt.Errorf("clone url is empty") + return "", fmt.Errorf("failed creating the project \"%s\" under \"%s\": %w", opts.Name, opts.Owner, err) } - return p.HTTPURLToRepo, p.DefaultBranch, err + return p.DefaultBranch, err } func (g *gitlab) GetDefaultBranch(ctx context.Context, orgRepo string) (string, error) { diff --git a/pkg/git/provider_gitlab_test.go b/pkg/git/provider_gitlab_test.go index f93ea788..00ec2b1d 100644 --- a/pkg/git/provider_gitlab_test.go +++ b/pkg/git/provider_gitlab_test.go @@ -14,11 +14,10 @@ import ( func Test_gitlab_CreateRepository(t *testing.T) { tests := map[string]struct { - orgRepo string - beforeFn func(*glmocks.MockGitlabClient) - wantCloneURL string - wantDefaultBranch string - wantErr string + orgRepo string + beforeFn func(*glmocks.MockGitlabClient) + want string + wantErr string }{ "Fails if credentials are wrong": { orgRepo: "username/projectName", @@ -83,13 +82,11 @@ func Test_gitlab_CreateRepository(t *testing.T) { }, }, "Creates project under user": { - orgRepo: "username/projectName", - wantCloneURL: "http://gitlab.com/username/projectName", - wantDefaultBranch: "main", + orgRepo: "username/projectName", + want: "main", beforeFn: func(c *glmocks.MockGitlabClient) { u := &gl.User{Username: "username"} p := &gl.Project{ - HTTPURLToRepo: "http://gitlab.com/username/projectName", DefaultBranch: "main", } createOpts := gl.CreateProjectOptions{ @@ -106,14 +103,12 @@ func Test_gitlab_CreateRepository(t *testing.T) { }, }, "Creates project under group": { - orgRepo: "org/projectName", - wantCloneURL: "http://gitlab.com/org/projectName", - wantDefaultBranch: "main", + orgRepo: "org/projectName", + want: "main", beforeFn: func(c *glmocks.MockGitlabClient) { u := &gl.User{Username: "username"} c.EXPECT().CurrentUser().Return(u, nil, nil) p := &gl.Project{ - HTTPURLToRepo: "http://gitlab.com/org/projectName", DefaultBranch: "main", } g := &gl.Group{FullPath: "org", ID: 1} @@ -133,14 +128,12 @@ func Test_gitlab_CreateRepository(t *testing.T) { }, }, "Creates project under sub group": { - orgRepo: "org/subOrg/projectName", - wantCloneURL: "http://gitlab.com/org/subOrg/projectName", - wantDefaultBranch: "main", + orgRepo: "org/subOrg/projectName", + want: "main", beforeFn: func(c *glmocks.MockGitlabClient) { u := &gl.User{Username: "username"} c.EXPECT().CurrentUser().Return(u, nil, nil) p := &gl.Project{ - HTTPURLToRepo: "http://gitlab.com/org/subOrg/projectName", DefaultBranch: "main", } g := &gl.Group{FullPath: "org/subOrg", ID: 1} @@ -160,13 +153,11 @@ func Test_gitlab_CreateRepository(t *testing.T) { }, }, "Creates private project": { - orgRepo: "username/projectName", - wantCloneURL: "http://gitlab.com/username/projectName", - wantDefaultBranch: "main", + orgRepo: "username/projectName", + want: "main", beforeFn: func(c *glmocks.MockGitlabClient) { u := &gl.User{Username: "username"} p := &gl.Project{ - HTTPURLToRepo: "http://gitlab.com/username/projectName", DefaultBranch: "main", } createOpts := gl.CreateProjectOptions{ @@ -185,27 +176,6 @@ func Test_gitlab_CreateRepository(t *testing.T) { Return(p, res, nil) }, }, - "Fails when no HTTPURLToRepo": { - orgRepo: "username/projectName", - wantErr: "clone url is empty", - beforeFn: func(c *glmocks.MockGitlabClient) { - u := &gl.User{Username: "username"} - p := &gl.Project{} - createOpts := gl.CreateProjectOptions{ - Name: gl.String("projectName"), - Visibility: gl.Visibility(gl.PrivateVisibility), - } - res := &gl.Response{ - Response: &http.Response{}, - } - c.EXPECT().CurrentUser(). - Times(1). - Return(u, nil, nil) - c.EXPECT().CreateProject(&createOpts). - Times(1). - Return(p, res, nil) - }, - }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { @@ -214,14 +184,13 @@ func Test_gitlab_CreateRepository(t *testing.T) { g := &gitlab{ client: mockClient, } - gotCloneURL, gotDefaultBranch, err := g.CreateRepository(context.Background(), tt.orgRepo) + got, err := g.CreateRepository(context.Background(), tt.orgRepo) if err != nil || tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) return } - assert.Equalf(t, tt.wantCloneURL, gotCloneURL, "CreateRepository - %s", name) - assert.Equalf(t, tt.wantDefaultBranch, gotDefaultBranch, "CreateRepository - %s", name) + assert.Equalf(t, tt.want, got, "CreateRepository - %s", name) }) } } diff --git a/pkg/git/repository.go b/pkg/git/repository.go index 6fcad8e0..3d2ee50d 100644 --- a/pkg/git/repository.go +++ b/pkg/git/repository.go @@ -226,12 +226,11 @@ func (o *CloneOptions) GetRepo(ctx context.Context) (Repository, fs.FS, error) { } log.G(ctx).Infof("repository '%s' was not found, trying to create it...", o.Repo) - o.Repo, defaultBranch, err = createRepo(ctx, o) + defaultBranch, err = createRepo(ctx, o) if err != nil { return nil, nil, fmt.Errorf("failed to create repository: %w", err) } - o.Parse() fallthrough // a new repo will always start as empty - we need to init it locally case transport.ErrEmptyRemoteRepository: log.G(ctx).Info("empty repository, initializing a new one with specified remote") @@ -469,10 +468,10 @@ var clone = func(ctx context.Context, opts *CloneOptions) (*repo, error) { return repo, nil } -var createRepo = func(ctx context.Context, opts *CloneOptions) (cloneURL, defaultBranch string, err error) { +var createRepo = func(ctx context.Context, opts *CloneOptions) (defaultBranch string, err error) { provider, _ := getProvider(opts.Provider, opts.Repo, &opts.Auth) if provider == nil { - return "", "", errors.New("failed creating repository - no git provider supplied") + return "", errors.New("failed creating repository - no git provider supplied") } _, orgRepo, _, _, _, _, _ := util.ParseGitUrl(opts.Repo) diff --git a/pkg/git/repository_test.go b/pkg/git/repository_test.go index 4db72546..22995ba3 100644 --- a/pkg/git/repository_test.go +++ b/pkg/git/repository_test.go @@ -27,14 +27,14 @@ import ( ) type mockProvider struct { - createRepository func(orgRepo string) (cloneURL, defaultBranch string, err error) + createRepository func(orgRepo string) (defaultBranch string, err error) getDefaultBranch func(orgRepo string) (string, error) getAuthor func() (string, string, error) } -func (p *mockProvider) CreateRepository(_ context.Context, orgRepo string) (cloneURL, defaultBranch string, err error) { +func (p *mockProvider) CreateRepository(_ context.Context, orgRepo string) (defaultBranch string, err error) { return p.createRepository(orgRepo) } @@ -599,7 +599,7 @@ func TestGetRepo(t *testing.T) { opts *CloneOptions wantErr string cloneFn func(context.Context, *CloneOptions) (*repo, error) - createRepoFn func(context.Context, *CloneOptions) (cloneURL, defaultBranch string, err error) + createRepoFn func(context.Context, *CloneOptions) (defaultBranch string, err error) initRepoFn func(context.Context, *CloneOptions, string) (*repo, error) assertFn func(*testing.T, Repository, fs.FS, error) }{ @@ -661,8 +661,8 @@ func TestGetRepo(t *testing.T) { cloneFn: func(_ context.Context, opts *CloneOptions) (*repo, error) { return nil, transport.ErrRepositoryNotFound }, - createRepoFn: func(c context.Context, co *CloneOptions) (cloneURL, defaultBranch string, err error) { - return "", "", errors.New("some error") + createRepoFn: func(c context.Context, co *CloneOptions) (defaultBranch string, err error) { + return "", errors.New("some error") }, assertFn: func(t *testing.T, r Repository, f fs.FS, e error) { assert.Nil(t, r) @@ -697,8 +697,8 @@ func TestGetRepo(t *testing.T) { cloneFn: func(_ context.Context, opts *CloneOptions) (*repo, error) { return nil, transport.ErrRepositoryNotFound }, - createRepoFn: func(c context.Context, co *CloneOptions) (cloneURL, defaultBranch string, err error) { - return "", "", nil + createRepoFn: func(c context.Context, co *CloneOptions) (defaultBranch string, err error) { + return "", nil }, initRepoFn: func(c context.Context, _ *CloneOptions, _ string) (*repo, error) { return &repo{}, nil @@ -1310,10 +1310,9 @@ func TestAddFlags(t *testing.T) { func Test_createRepo(t *testing.T) { tests := map[string]struct { - opts *CloneOptions - wantCloneURL string - wantDefaultBranch string - wantErr string + opts *CloneOptions + want string + wantErr string }{ "Should create new repository": { opts: &CloneOptions{ @@ -1324,15 +1323,13 @@ func Test_createRepo(t *testing.T) { Password: "password", }, }, - wantCloneURL: "https://github.com/owner/name.git", - wantDefaultBranch: "main", + want: "main", }, "Should infer correct provider type from repo url": { opts: &CloneOptions{ Repo: "https://github.com/owner/name.git", }, - wantCloneURL: "https://github.com/owner/name.git", - wantDefaultBranch: "main", + want: "main", }, } @@ -1340,18 +1337,17 @@ func Test_createRepo(t *testing.T) { defer func() { getProvider = orgGetProvider }() for name, tt := range tests { t.Run(name, func(t *testing.T) { - mockProvider := &mockProvider{func(orgRepo string) (cloneURL, defaultBranch string, err error) { - return "https://github.com/owner/name.git", "main", nil + mockProvider := &mockProvider{func(orgRepo string) (defaultBranch string, err error) { + return "main", nil }, nil, nil} getProvider = func(providerType, repoURL string, auth *Auth) (Provider, error) { return mockProvider, nil } - gotCloneURL, gotDefaultBranch, err := createRepo(context.Background(), tt.opts) + got, err := createRepo(context.Background(), tt.opts) if err != nil || tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) return } - assert.Equalf(t, tt.wantCloneURL, gotCloneURL, "CreateRepository - %s", name) - assert.Equalf(t, tt.wantDefaultBranch, gotDefaultBranch, "CreateRepository - %s", name) + assert.Equalf(t, tt.want, got, "CreateRepository - %s", name) }) } } From 2de9064ac5c52dbdc9a2b4eb8211d2305a3babbc Mon Sep 17 00:00:00 2001 From: Noam Gal Date: Thu, 8 Sep 2022 15:26:52 +0300 Subject: [PATCH 2/2] ran codegen --- pkg/git/mocks/provider.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/git/mocks/provider.go b/pkg/git/mocks/provider.go index 6df06107..3c447ded 100644 --- a/pkg/git/mocks/provider.go +++ b/pkg/git/mocks/provider.go @@ -35,13 +35,12 @@ func (m *MockProvider) EXPECT() *MockProviderMockRecorder { } // CreateRepository mocks base method. -func (m *MockProvider) CreateRepository(ctx context.Context, orgRepo string) (string, string, error) { +func (m *MockProvider) CreateRepository(ctx context.Context, orgRepo string) (string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateRepository", ctx, orgRepo) ret0, _ := ret[0].(string) - ret1, _ := ret[1].(string) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 + ret1, _ := ret[1].(error) + return ret0, ret1 } // CreateRepository indicates an expected call of CreateRepository.