From a6b944570256b2d36ffecf723367ebd96582ff7e Mon Sep 17 00:00:00 2001 From: monopole Date: Wed, 19 May 2021 17:35:24 -0700 Subject: [PATCH] Fix repospec test --- api/internal/git/repospec.go | 8 ++- api/internal/git/repospec_test.go | 103 ++++++++++++++---------------- 2 files changed, 53 insertions(+), 58 deletions(-) diff --git a/api/internal/git/repospec.go b/api/internal/git/repospec.go index 29e2f4e95..7560fa92f 100644 --- a/api/internal/git/repospec.go +++ b/api/internal/git/repospec.go @@ -78,6 +78,7 @@ func (x *RepoSpec) Cleaner(fSys filesys.FileSystem) func() error { return func() error { return fSys.RemoveAll(x.Dir.String()) } } +// NewRepoSpecFromUrl parses git-like urls. // From strings like git@github.com:someOrg/someRepo.git or // https://github.com/someOrg/someRepo?ref=someHash, extract // the parts. @@ -85,7 +86,7 @@ func NewRepoSpecFromUrl(n string) (*RepoSpec, error) { if filepath.IsAbs(n) { return nil, fmt.Errorf("uri looks like abs path: %s", n) } - host, orgRepo, path, gitRef, gitSubmodules, gitSuffix, gitTimeout := parseGitUrl(n) + host, orgRepo, path, gitRef, gitSubmodules, suffix, gitTimeout := parseGitUrl(n) if orgRepo == "" { return nil, fmt.Errorf("url lacks orgRepo: %s", n) } @@ -94,7 +95,7 @@ func NewRepoSpecFromUrl(n string) (*RepoSpec, error) { } return &RepoSpec{ raw: n, Host: host, OrgRepo: orgRepo, - Dir: notCloned, Path: path, Ref: gitRef, GitSuffix: gitSuffix, + Dir: notCloned, Path: path, Ref: gitRef, GitSuffix: suffix, Submodules: gitSubmodules, Timeout: gitTimeout}, nil } @@ -124,6 +125,9 @@ func parseGitUrl(n string) ( index := strings.Index(n, gitSuffix) orgRepo = n[0:index] n = n[index+len(gitSuffix):] + if n[0] == '/' { + n = n[1:] + } path, gitRef, gitTimeout, gitSubmodules = peelQuery(n) return } diff --git a/api/internal/git/repospec_test.go b/api/internal/git/repospec_test.go index befed131a..e84a35249 100644 --- a/api/internal/git/repospec_test.go +++ b/api/internal/git/repospec_test.go @@ -9,6 +9,8 @@ import ( "strings" "testing" "time" + + "github.com/stretchr/testify/assert" ) var orgRepos = []string{"someOrg/someRepo", "kubernetes/website"} @@ -109,96 +111,87 @@ func TestNewRepoSpecFromUrlErrors(t *testing.T) { } func TestNewRepoSpecFromUrl_CloneSpecs(t *testing.T) { - testcases := []struct { + testcases := map[string]struct { input string cloneSpec string absPath string ref string }{ - { + "t1": { input: "https://git-codecommit.us-east-2.amazonaws.com/someorg/somerepo/somedir", cloneSpec: "https://git-codecommit.us-east-2.amazonaws.com/someorg/somerepo", absPath: notCloned.Join("somedir"), ref: "", }, - { + "t2": { input: "https://git-codecommit.us-east-2.amazonaws.com/someorg/somerepo/somedir?ref=testbranch", cloneSpec: "https://git-codecommit.us-east-2.amazonaws.com/someorg/somerepo", absPath: notCloned.Join("somedir"), ref: "testbranch", }, - { + "t3": { input: "https://fabrikops2.visualstudio.com/someorg/somerepo?ref=master", cloneSpec: "https://fabrikops2.visualstudio.com/someorg/somerepo", absPath: notCloned.String(), ref: "master", }, - { + "t4": { input: "http://github.com/someorg/somerepo/somedir", cloneSpec: "https://github.com/someorg/somerepo.git", absPath: notCloned.Join("somedir"), ref: "", }, - { + "t5": { input: "git@github.com:someorg/somerepo/somedir", cloneSpec: "git@github.com:someorg/somerepo.git", absPath: notCloned.Join("somedir"), ref: "", }, - { + "t6": { input: "git@gitlab2.sqtools.ru:10022/infra/kubernetes/thanos-base.git?ref=v0.1.0", cloneSpec: "git@gitlab2.sqtools.ru:10022/infra/kubernetes/thanos-base.git", absPath: notCloned.String(), ref: "v0.1.0", }, - { + "t7": { input: "git@bitbucket.org:company/project.git//path?ref=branch", cloneSpec: "git@bitbucket.org:company/project.git", absPath: notCloned.Join("path"), ref: "branch", }, - { + "t8": { input: "https://itfs.mycompany.com/collection/project/_git/somerepos", cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos", absPath: notCloned.String(), ref: "", }, - { + "t9": { input: "https://itfs.mycompany.com/collection/project/_git/somerepos?version=v1.0.0", cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos", absPath: notCloned.String(), ref: "v1.0.0", }, - { + "t10": { input: "https://itfs.mycompany.com/collection/project/_git/somerepos/somedir?version=v1.0.0", cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos", absPath: notCloned.Join("somedir"), ref: "v1.0.0", }, - { + "t11": { input: "git::https://itfs.mycompany.com/collection/project/_git/somerepos", cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos", absPath: notCloned.String(), ref: "", }, } - for _, testcase := range testcases { - rs, err := NewRepoSpecFromUrl(testcase.input) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if rs.CloneSpec() != testcase.cloneSpec { - t.Errorf("CloneSpec expected to be %v, but got %v on %s", - testcase.cloneSpec, rs.CloneSpec(), testcase.input) - } - if rs.AbsPath() != testcase.absPath { - t.Errorf("AbsPath expected to be %v, but got %v on %s", - testcase.absPath, rs.AbsPath(), testcase.input) - } - if rs.Ref != testcase.ref { - t.Errorf("ref expected to be %v, but got %v on %s", - testcase.ref, rs.Ref, testcase.input) - } + for tn, tc := range testcases { + t.Run(tn, func(t *testing.T) { + rs, err := NewRepoSpecFromUrl(tc.input) + assert.NoError(t, err) + assert.Equal(t, tc.cloneSpec, rs.CloneSpec(), "cloneSpec mismatch") + assert.Equal(t, tc.absPath, rs.AbsPath(), "absPath mismatch") + assert.Equal(t, tc.ref, rs.Ref, "ref mismatch") + }) } } @@ -233,15 +226,14 @@ func TestIsAzureHost(t *testing.T) { } func TestPeelQuery(t *testing.T) { - testcases := []struct { - input string - + testcases := map[string]struct { + input string path string ref string submodules bool timeout time.Duration }{ - { + "t1": { // All empty. input: "somerepos", path: "somerepos", @@ -249,21 +241,21 @@ func TestPeelQuery(t *testing.T) { submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t2": { input: "somerepos?ref=v1.0.0", path: "somerepos", ref: "v1.0.0", submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t3": { input: "somerepos?version=master", path: "somerepos", ref: "master", submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t4": { // A ref value takes precedence over a version value. input: "somerepos?version=master&ref=v1.0.0", path: "somerepos", @@ -271,7 +263,7 @@ func TestPeelQuery(t *testing.T) { submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t5": { // Empty submodules value uses default. input: "somerepos?version=master&submodules=", path: "somerepos", @@ -279,7 +271,7 @@ func TestPeelQuery(t *testing.T) { submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t6": { // Malformed submodules value uses default. input: "somerepos?version=master&submodules=maybe", path: "somerepos", @@ -287,21 +279,21 @@ func TestPeelQuery(t *testing.T) { submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t7": { input: "somerepos?version=master&submodules=true", path: "somerepos", ref: "master", submodules: true, timeout: defaultTimeout, }, - { + "t8": { input: "somerepos?version=master&submodules=false", path: "somerepos", ref: "master", submodules: false, timeout: defaultTimeout, }, - { + "t9": { // Empty timeout value uses default. input: "somerepos?version=master&timeout=", path: "somerepos", @@ -309,7 +301,7 @@ func TestPeelQuery(t *testing.T) { submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t10": { // Malformed timeout value uses default. input: "somerepos?version=master&timeout=jiffy", path: "somerepos", @@ -317,7 +309,7 @@ func TestPeelQuery(t *testing.T) { submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t11": { // Zero timeout value uses default. input: "somerepos?version=master&timeout=0", path: "somerepos", @@ -325,28 +317,28 @@ func TestPeelQuery(t *testing.T) { submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t12": { input: "somerepos?version=master&timeout=0s", path: "somerepos", ref: "master", submodules: defaultSubmodules, timeout: defaultTimeout, }, - { + "t13": { input: "somerepos?version=master&timeout=61", path: "somerepos", ref: "master", submodules: defaultSubmodules, timeout: 61 * time.Second, }, - { + "t14": { input: "somerepos?version=master&timeout=1m1s", path: "somerepos", ref: "master", submodules: defaultSubmodules, timeout: 61 * time.Second, }, - { + "t15": { input: "somerepos?version=master&submodules=false&timeout=1m1s", path: "somerepos", ref: "master", @@ -354,15 +346,14 @@ func TestPeelQuery(t *testing.T) { timeout: 61 * time.Second, }, } - - for _, testcase := range testcases { - path, ref, timeout, submodules := peelQuery(testcase.input) - if path != testcase.path || ref != testcase.ref || timeout != testcase.timeout || submodules != testcase.submodules { - t.Errorf("peelQuery: expected (%s, %s, %v, %v) got (%s, %s, %v, %v) on %s", - testcase.path, testcase.ref, testcase.timeout, testcase.submodules, - path, ref, timeout, submodules, - testcase.input) - } + for tn, tc := range testcases { + t.Run(tn, func(t *testing.T) { + path, ref, timeout, submodules := peelQuery(tc.input) + assert.Equal(t, tc.path, path, "path mismatch") + assert.Equal(t, tc.ref, ref, "ref mismatch") + assert.Equal(t, tc.timeout, timeout, "timeout mismatch") + assert.Equal(t, tc.submodules, submodules, "submodules mismatch") + }) } }