From 529af6fe72a391207ab25ef6ca7676eaf4c45954 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Sun, 15 Oct 2017 17:46:21 -0200 Subject: [PATCH] refactor: improved git error handling Improved the error handling in git code, mostly in the defaults pipe. The idea is to output better error messages, hopefully avoiding confusion on "whats wrong". refs #356 --- config/config.go | 3 +++ config/config_test.go | 11 ++++++-- pipeline/defaults/defaults.go | 2 +- pipeline/defaults/defaults_test.go | 40 +++++++++++++++++++++--------- pipeline/defaults/remote.go | 8 ++++-- pipeline/defaults/remote_test.go | 6 +++++ pipeline/git/git.go | 3 ++- 7 files changed, 55 insertions(+), 18 deletions(-) diff --git a/config/config.go b/config/config.go index ee2922d3f..93c8f5569 100644 --- a/config/config.go +++ b/config/config.go @@ -31,6 +31,9 @@ type Repo struct { // String of the repo, e.g. owner/name func (r Repo) String() string { + if r.Owner == "" && r.Name == "" { + return "" + } return r.Owner + "/" + r.Name } diff --git a/config/config_test.go b/config/config_test.go index 65bacd166..5defb42ee 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -12,8 +12,15 @@ import ( ) func TestRepo(t *testing.T) { - r := Repo{Owner: "goreleaser", Name: "godownloader"} - assert.Equal(t, "goreleaser/godownloader", r.String(), "not equal") + assert.Equal( + t, + "goreleaser/godownloader", + Repo{Owner: "goreleaser", Name: "godownloader"}.String(), + ) +} + +func TestEmptyRepoNameAndOwner(t *testing.T) { + assert.Empty(t, Repo{}.String()) } func TestLoadReader(t *testing.T) { diff --git a/pipeline/defaults/defaults.go b/pipeline/defaults/defaults.go index e94ee24bb..bf9df74f0 100644 --- a/pipeline/defaults/defaults.go +++ b/pipeline/defaults/defaults.go @@ -115,7 +115,7 @@ func setReleaseDefaults(ctx *context.Context) error { } repo, err := remoteRepo() if err != nil { - return fmt.Errorf("failed reading repo from git: %v", err.Error()) + return err } ctx.Config.Release.GitHub = repo return nil diff --git a/pipeline/defaults/defaults_test.go b/pipeline/defaults/defaults_test.go index 045fe8cf4..5b168ab11 100644 --- a/pipeline/defaults/defaults_test.go +++ b/pipeline/defaults/defaults_test.go @@ -1,12 +1,11 @@ package defaults import ( - "io/ioutil" - "os" "testing" "github.com/goreleaser/goreleaser/config" "github.com/goreleaser/goreleaser/context" + "github.com/goreleaser/goreleaser/internal/testlib" "github.com/stretchr/testify/assert" ) @@ -15,6 +14,11 @@ func TestDescription(t *testing.T) { } func TestFillBasicData(t *testing.T) { + _, back := testlib.Mktmp(t) + defer back() + testlib.GitInit(t) + testlib.GitRemoteAdd(t, "git@github.com:goreleaser/goreleaser.git") + var ctx = &context.Context{ Config: config.Project{}, } @@ -32,8 +36,6 @@ func TestFillBasicData(t *testing.T) { assert.Equal(t, "tar.gz", ctx.Config.Archive.Format) assert.Contains(t, ctx.Config.Brew.Install, "bin.install \"goreleaser\"") assert.Empty(t, ctx.Config.Dockers) - assert.Equal(t, ctx.Config.Brew.CommitAuthor.Name, "goreleaserbot") - assert.Equal(t, ctx.Config.Brew.CommitAuthor.Email, "goreleaser@carlosbecker.com") assert.NotEmpty( t, ctx.Config.Archive.NameTemplate, @@ -43,6 +45,10 @@ func TestFillBasicData(t *testing.T) { } func TestFillPartial(t *testing.T) { + _, back := testlib.Mktmp(t) + defer back() + testlib.GitInit(t) + testlib.GitRemoteAdd(t, "git@github.com:goreleaser/goreleaser.git") var ctx = &context.Context{ Config: config.Project{ @@ -83,6 +89,10 @@ func TestFillPartial(t *testing.T) { } func TestFillSingleBuild(t *testing.T) { + _, back := testlib.Mktmp(t) + defer back() + testlib.GitInit(t) + testlib.GitRemoteAdd(t, "git@github.com:goreleaser/goreleaser.git") var ctx = &context.Context{ Config: config.Project{ @@ -97,16 +107,22 @@ func TestFillSingleBuild(t *testing.T) { } func TestNotAGitRepo(t *testing.T) { - folder, err := ioutil.TempDir("", "goreleasertest") - assert.NoError(t, err) - previous, err := os.Getwd() - assert.NoError(t, err) - assert.NoError(t, os.Chdir(folder)) - defer func() { - assert.NoError(t, os.Chdir(previous)) - }() + _, back := testlib.Mktmp(t) + defer back() + testlib.GitInit(t) var ctx = &context.Context{ Config: config.Project{}, } assert.Error(t, Pipe{}.Run(ctx)) + assert.Empty(t, ctx.Config.Release.GitHub.String()) +} + +func TestGitRepoWithoutRemote(t *testing.T) { + _, back := testlib.Mktmp(t) + defer back() + var ctx = &context.Context{ + Config: config.Project{}, + } + assert.Error(t, Pipe{}.Run(ctx)) + assert.Empty(t, ctx.Config.Release.GitHub.String()) } diff --git a/pipeline/defaults/remote.go b/pipeline/defaults/remote.go index b89b87f83..b5fb7488d 100644 --- a/pipeline/defaults/remote.go +++ b/pipeline/defaults/remote.go @@ -1,19 +1,23 @@ package defaults import ( - "errors" + "os" "os/exec" "strings" "github.com/goreleaser/goreleaser/config" + "github.com/pkg/errors" ) // remoteRepo gets the repo name from the Git config. func remoteRepo() (result config.Repo, err error) { + if _, err = os.Stat(".git"); os.IsNotExist(err) { + return result, errors.Wrap(err, "current folder is not a git repository") + } cmd := exec.Command("git", "config", "--get", "remote.origin.url") bts, err := cmd.CombinedOutput() if err != nil { - return result, errors.New(err.Error() + ": " + string(bts)) + return result, errors.Wrap(err, "repository doesn't have an `origin` remote") } return extractRepoFromURL(string(bts)), nil } diff --git a/pipeline/defaults/remote_test.go b/pipeline/defaults/remote_test.go index 297ad9b76..4ecd8bcf2 100644 --- a/pipeline/defaults/remote_test.go +++ b/pipeline/defaults/remote_test.go @@ -3,10 +3,16 @@ package defaults import ( "testing" + "github.com/goreleaser/goreleaser/internal/testlib" + "github.com/stretchr/testify/assert" ) func TestRepoName(t *testing.T) { + _, back := testlib.Mktmp(t) + defer back() + testlib.GitInit(t) + testlib.GitRemoteAdd(t, "git@github.com:goreleaser/goreleaser.git") repo, err := remoteRepo() assert.NoError(t, err) assert.Equal(t, "goreleaser/goreleaser", repo.String()) diff --git a/pipeline/git/git.go b/pipeline/git/git.go index 4549b26e1..c0b5df6ff 100644 --- a/pipeline/git/git.go +++ b/pipeline/git/git.go @@ -14,6 +14,7 @@ import ( "github.com/goreleaser/goreleaser/context" "github.com/goreleaser/goreleaser/internal/git" "github.com/goreleaser/goreleaser/pipeline" + "github.com/pkg/errors" ) // Pipe for brew deployment @@ -54,7 +55,7 @@ func setVersion(ctx *context.Context, tag, commit string) (err error) { if ctx.Snapshot { snapshotName, err := getSnapshotName(ctx, tag, commit) if err != nil { - return fmt.Errorf("failed to generate snapshot name: %s", err.Error()) + return errors.Wrap(err, "failed to generate snapshot name") } ctx.Version = snapshotName return nil