From 27f3d62d02b39b618f2bf34cd5f30d05b5bf1a02 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 5 Apr 2024 12:23:47 +0200 Subject: [PATCH 1/4] Remove naked returns from git ParseURL --- git/url.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/git/url.go b/git/url.go index 1a3e97fd6..05517f9b3 100644 --- a/git/url.go +++ b/git/url.go @@ -26,7 +26,7 @@ func isPossibleProtocol(u string) bool { } // ParseURL normalizes git remote urls -func ParseURL(rawURL string) (u *url.URL, err error) { +func ParseURL(rawURL string) (*url.URL, error) { if !isPossibleProtocol(rawURL) && strings.ContainsRune(rawURL, ':') && // not a Windows path @@ -35,9 +35,9 @@ func ParseURL(rawURL string) (u *url.URL, err error) { rawURL = "ssh://" + strings.Replace(rawURL, ":", "/", 1) } - u, err = url.Parse(rawURL) + u, err := url.Parse(rawURL) if err != nil { - return + return nil, err } if u.Scheme == "git+ssh" { @@ -49,7 +49,7 @@ func ParseURL(rawURL string) (u *url.URL, err error) { } if u.Scheme != "ssh" { - return + return u, nil } if strings.HasPrefix(u.Path, "//") { @@ -60,5 +60,5 @@ func ParseURL(rawURL string) (u *url.URL, err error) { u.Host = u.Host[0:idx] } - return + return u, nil } From 2c2a09c73e00e1e96abdc1043bfa6ef295aecd66 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 5 Apr 2024 12:25:10 +0200 Subject: [PATCH 2/4] Test for parsing error in git ParseURL --- git/url_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/git/url_test.go b/git/url_test.go index f5b3b50d0..569a086ee 100644 --- a/git/url_test.go +++ b/git/url_test.go @@ -196,13 +196,22 @@ func TestParseURL(t *testing.T) { Path: "", }, }, + { + name: "fails to parse", + url: "ssh://git@[/tmp/git-repo", + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { u, err := ParseURL(tt.url) - if (err != nil) != tt.wantErr { - t.Fatalf("got error: %v", err) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + return } + if u.Scheme != tt.want.Scheme { t.Errorf("expected scheme %q, got %q", tt.want.Scheme, u.Scheme) } From 1b57b0f917a81057a80d9de580112abd30d89c32 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 5 Apr 2024 12:29:27 +0200 Subject: [PATCH 3/4] Minorly refactor scheme normalization in git ParseURL --- git/url.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/git/url.go b/git/url.go index 05517f9b3..f11e86c80 100644 --- a/git/url.go +++ b/git/url.go @@ -40,12 +40,11 @@ func ParseURL(rawURL string) (*url.URL, error) { return nil, err } - if u.Scheme == "git+ssh" { - u.Scheme = "ssh" - } - - if u.Scheme == "git+https" { + switch u.Scheme { + case "git+https": u.Scheme = "https" + case "git+ssh": + u.Scheme = "ssh" } if u.Scheme != "ssh" { From b0b147e60c58464c9a541a20442de77e1b3409a3 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 5 Apr 2024 16:05:50 +0200 Subject: [PATCH 4/4] Use testify in git url tests --- git/url_test.go | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/git/url_test.go b/git/url_test.go index 569a086ee..c2cfe9657 100644 --- a/git/url_test.go +++ b/git/url_test.go @@ -1,6 +1,11 @@ package git -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) func TestIsURL(t *testing.T) { tests := []struct { @@ -56,9 +61,7 @@ func TestIsURL(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := IsURL(tt.url); got != tt.want { - t.Errorf("IsURL() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.want, IsURL(tt.url)) }) } } @@ -206,24 +209,14 @@ func TestParseURL(t *testing.T) { t.Run(tt.name, func(t *testing.T) { u, err := ParseURL(tt.url) if tt.wantErr { - if err == nil { - t.Fatalf("expected error, got nil") - } + require.Error(t, err) return } - if u.Scheme != tt.want.Scheme { - t.Errorf("expected scheme %q, got %q", tt.want.Scheme, u.Scheme) - } - if u.User.Username() != tt.want.User { - t.Errorf("expected user %q, got %q", tt.want.User, u.User.Username()) - } - if u.Host != tt.want.Host { - t.Errorf("expected host %q, got %q", tt.want.Host, u.Host) - } - if u.Path != tt.want.Path { - t.Errorf("expected path %q, got %q", tt.want.Path, u.Path) - } + assert.Equal(t, u.Scheme, tt.want.Scheme) + assert.Equal(t, u.User.Username(), tt.want.User) + assert.Equal(t, u.Host, tt.want.Host) + assert.Equal(t, u.Path, tt.want.Path) }) } }