From a84148c620eb858d2f981f1446911d9a2a40ddd7 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 29 Dec 2017 17:07:06 -0200 Subject: [PATCH 01/15] feat: make goreleaser timeoutable --- context/context.go | 13 ++++++++- goreleaserlib/goreleaser.go | 41 ++++++++++++++++++++++------ main.go | 5 ++++ pipeline/build/build.go | 14 +++++----- pipeline/docker/docker.go | 16 +++++------ pipeline/fpm/fpm.go | 6 ++-- pipeline/fpm/fpm_test.go | 7 ++--- pipeline/release/body.go | 2 +- pipeline/sign/sign.go | 2 +- pipeline/snapcraft/snapcraft.go | 2 +- pipeline/snapcraft/snapcraft_test.go | 22 +++++++-------- 11 files changed, 85 insertions(+), 45 deletions(-) diff --git a/context/context.go b/context/context.go index b42177211..3423ee31b 100644 --- a/context/context.go +++ b/context/context.go @@ -10,6 +10,7 @@ import ( ctx "context" "os" "strings" + "time" "github.com/goreleaser/goreleaser/config" "github.com/goreleaser/goreleaser/internal/artifact" @@ -41,8 +42,18 @@ type Context struct { // New context func New(config config.Project) *Context { + return wrap(ctx.Background(), config) +} + +// NewWithTimeout new context with the given timeout +func NewWithTimeout(config config.Project, timeout time.Duration) (*Context, ctx.CancelFunc) { + ctx, cancel := ctx.WithTimeout(ctx.Background(), timeout) + return wrap(ctx, config), cancel +} + +func wrap(ctx ctx.Context, config config.Project) *Context { return &Context{ - Context: ctx.Background(), + Context: ctx, Config: config, Env: splitEnv(os.Environ()), Parallelism: 4, diff --git a/goreleaserlib/goreleaser.go b/goreleaserlib/goreleaser.go index b856eb39a..bdb9b9402 100644 --- a/goreleaserlib/goreleaser.go +++ b/goreleaserlib/goreleaser.go @@ -4,7 +4,10 @@ import ( "fmt" "io/ioutil" "os" + "os/signal" "strings" + "syscall" + "time" "github.com/apex/log" "github.com/apex/log/handlers/cli" @@ -65,6 +68,7 @@ type Flags interface { String(s string) string Int(s string) int Bool(s string) bool + Duration(s string) time.Duration } // Release runs the release process with the given flags @@ -84,7 +88,8 @@ func Release(flags Flags) error { } log.WithField("file", file).Warn("could not load config, using defaults") } - var ctx = context.New(cfg) + ctx, cancel := context.NewWithTimeout(cfg, flags.Duration("timeout")) + defer cancel() ctx.Parallelism = flags.Int("parallelism") ctx.Debug = flags.Bool("debug") log.Debugf("parallelism: %v", ctx.Parallelism) @@ -105,16 +110,36 @@ func Release(flags Flags) error { ctx.Publish = false } ctx.RmDist = flags.Bool("rm-dist") - for _, pipe := range pipes { - cli.Default.Padding = normalPadding - log.Infof("\033[1m%s\033[0m", strings.ToUpper(pipe.String())) - cli.Default.Padding = increasedPadding - if err := handle(pipe.Run(ctx)); err != nil { - return err + var errs = make(chan error) + go func() { + for _, pipe := range pipes { + restoreOutputPadding() + log.Infof("\033[1m%s\033[0m", strings.ToUpper(pipe.String())) + cli.Default.Padding = increasedPadding + if err := handle(pipe.Run(ctx)); err != nil { + errs <- err + return + } } + errs <- nil + }() + defer restoreOutputPadding() + var signals = make(chan os.Signal) + signal.Notify(signals, os.Interrupt, syscall.SIGTERM) + select { + case err := <-errs: + return err + case <-ctx.Done(): + return ctx.Err() + case sig := <-signals: + restoreOutputPadding() + log.Infof("stopping: %s", sig) + return nil } +} + +func restoreOutputPadding() { cli.Default.Padding = normalPadding - return nil } func handle(err error) error { diff --git a/main.go b/main.go index 7ced1dd0a..7255a9f98 100644 --- a/main.go +++ b/main.go @@ -63,6 +63,11 @@ func main() { Name: "debug", Usage: "Enable debug mode", }, + cli.DurationFlag{ + Name: "timeout", + Usage: "How much time the entire release process is allowed to take", + Value: 30 * time.Minute, + }, } app.Action = func(c *cli.Context) error { start := time.Now() diff --git a/pipeline/build/build.go b/pipeline/build/build.go index 4a6add2fd..c32772860 100644 --- a/pipeline/build/build.go +++ b/pipeline/build/build.go @@ -74,7 +74,7 @@ func buildWithDefaults(ctx *context.Context, build config.Build) config.Build { } func runPipeOnBuild(ctx *context.Context, build config.Build) error { - if err := runHook(build.Env, build.Hooks.Pre); err != nil { + if err := runHook(ctx, build.Env, build.Hooks.Pre); err != nil { return errors.Wrap(err, "pre hook failed") } sem := make(chan bool, ctx.Parallelism) @@ -93,16 +93,16 @@ func runPipeOnBuild(ctx *context.Context, build config.Build) error { if err := g.Wait(); err != nil { return err } - return errors.Wrap(runHook(build.Env, build.Hooks.Post), "post hook failed") + return errors.Wrap(runHook(ctx, build.Env, build.Hooks.Post), "post hook failed") } -func runHook(env []string, hook string) error { +func runHook(ctx *context.Context, env []string, hook string) error { if hook == "" { return nil } log.WithField("hook", hook).Info("running hook") cmd := strings.Fields(hook) - return run(buildtarget.Runtime, cmd, env) + return run(ctx, buildtarget.Runtime, cmd, env) } func doBuild(ctx *context.Context, build config.Build, target buildtarget.Target) error { @@ -119,7 +119,7 @@ func doBuild(ctx *context.Context, build config.Build, target buildtarget.Target return err } cmd = append(cmd, "-ldflags="+flags, "-o", binary, build.Main) - if err := run(target, cmd, build.Env); err != nil { + if err := run(ctx, target, cmd, build.Env); err != nil { return errors.Wrapf(err, "failed to build for %s", target) } ctx.Artifacts.Add(artifact.Artifact{ @@ -137,9 +137,9 @@ func doBuild(ctx *context.Context, build config.Build, target buildtarget.Target return nil } -func run(target buildtarget.Target, command, env []string) error { +func run(ctx *context.Context, target buildtarget.Target, command, env []string) error { /* #nosec */ - var cmd = exec.Command(command[0], command[1:]...) + var cmd = exec.CommandContext(ctx, command[0], command[1:]...) env = append(env, target.Env()...) var log = log.WithField("target", target.PrettyString()). WithField("env", env). diff --git a/pipeline/docker/docker.go b/pipeline/docker/docker.go index 737e0a0c5..fa99c6766 100644 --- a/pipeline/docker/docker.go +++ b/pipeline/docker/docker.go @@ -141,11 +141,11 @@ func process(ctx *context.Context, docker config.Docker, artifact artifact.Artif return errors.Wrapf(err, "failed to link extra file '%s'", file) } } - if err := dockerBuild(root, dockerfile, image); err != nil { + if err := dockerBuild(ctx, root, dockerfile, image); err != nil { return err } if docker.Latest { - if err := dockerTag(image, latest); err != nil { + if err := dockerTag(ctx, image, latest); err != nil { return err } } @@ -187,16 +187,16 @@ func publish(ctx *context.Context, docker config.Docker, image, latest string) e if !docker.Latest { return nil } - if err := dockerTag(image, latest); err != nil { + if err := dockerTag(ctx, image, latest); err != nil { return err } return dockerPush(ctx, docker, latest) } -func dockerBuild(root, dockerfile, image string) error { +func dockerBuild(ctx *context.Context, root, dockerfile, image string) error { log.WithField("image", image).Info("building docker image") /* #nosec */ - var cmd = exec.Command("docker", "build", "-f", dockerfile, "-t", image, root) + var cmd = exec.CommandContext(ctx, "docker", "build", "-f", dockerfile, "-t", image, root) log.WithField("cmd", cmd).Debug("executing") out, err := cmd.CombinedOutput() if err != nil { @@ -206,10 +206,10 @@ func dockerBuild(root, dockerfile, image string) error { return nil } -func dockerTag(image, tag string) error { +func dockerTag(ctx *context.Context, image, tag string) error { log.WithField("image", image).WithField("tag", tag).Info("tagging docker image") /* #nosec */ - var cmd = exec.Command("docker", "tag", image, tag) + var cmd = exec.CommandContext(ctx, "docker", "tag", image, tag) log.WithField("cmd", cmd).Debug("executing") out, err := cmd.CombinedOutput() if err != nil { @@ -222,7 +222,7 @@ func dockerTag(image, tag string) error { func dockerPush(ctx *context.Context, docker config.Docker, image string) error { log.WithField("image", image).Info("pushing docker image") /* #nosec */ - var cmd = exec.Command("docker", "push", image) + var cmd = exec.CommandContext(ctx, "docker", "push", image) log.WithField("cmd", cmd).Debug("executing") out, err := cmd.CombinedOutput() if err != nil { diff --git a/pipeline/fpm/fpm.go b/pipeline/fpm/fpm.go index 871e10fe5..df0c3e40b 100644 --- a/pipeline/fpm/fpm.go +++ b/pipeline/fpm/fpm.go @@ -128,7 +128,7 @@ func create(ctx *context.Context, format, arch string, binaries []artifact.Artif } log.WithField("args", options).Debug("creating fpm package") - if out, err := cmd(options).CombinedOutput(); err != nil { + if out, err := cmd(ctx, options).CombinedOutput(); err != nil { return errors.Wrap(err, string(out)) } ctx.Artifacts.Add(artifact.Artifact{ @@ -142,9 +142,9 @@ func create(ctx *context.Context, format, arch string, binaries []artifact.Artif return nil } -func cmd(options []string) *exec.Cmd { +func cmd(ctx *context.Context, options []string) *exec.Cmd { /* #nosec */ - var cmd = exec.Command("fpm", options...) + var cmd = exec.CommandContext(ctx, "fpm", options...) cmd.Env = []string{fmt.Sprintf("PATH=%s:%s", gnuTarPath, os.Getenv("PATH"))} for _, env := range os.Environ() { if strings.HasPrefix(env, "PATH=") { diff --git a/pipeline/fpm/fpm_test.go b/pipeline/fpm/fpm_test.go index 6c108d58f..5d2dbb7b9 100644 --- a/pipeline/fpm/fpm_test.go +++ b/pipeline/fpm/fpm_test.go @@ -96,7 +96,7 @@ func TestInvalidNameTemplate(t *testing.T) { Config: config.Project{ FPM: config.FPM{ NameTemplate: "{{.Foo}", - Formats: []string{"deb"}, + Formats: []string{"deb"}, }, }, } @@ -109,7 +109,6 @@ func TestInvalidNameTemplate(t *testing.T) { assert.Contains(t, Pipe{}.Run(ctx).Error(), `template: {{.Foo}:1: unexpected "}" in operand`) } - func TestCreateFileDoesntExist(t *testing.T) { folder, err := ioutil.TempDir("", "archivetest") assert.NoError(t, err) @@ -141,7 +140,7 @@ func TestCreateFileDoesntExist(t *testing.T) { } func TestCmd(t *testing.T) { - cmd := cmd([]string{"--help"}) + cmd := cmd(context.New(config.Project{}), []string{"--help"}) assert.NotEmpty(t, cmd.Env) assert.Contains(t, cmd.Env[0], gnuTarPath) } @@ -161,7 +160,7 @@ func TestDefaultSet(t *testing.T) { var ctx = &context.Context{ Config: config.Project{ FPM: config.FPM{ - Bindir: "/bin", + Bindir: "/bin", NameTemplate: "foo", }, }, diff --git a/pipeline/release/body.go b/pipeline/release/body.go index 0a130d299..75c6ab2d8 100644 --- a/pipeline/release/body.go +++ b/pipeline/release/body.go @@ -25,7 +25,7 @@ Built with {{ .GoVersion }}` func describeBody(ctx *context.Context) (bytes.Buffer, error) { /* #nosec */ - bts, err := exec.Command("go", "version").CombinedOutput() + bts, err := exec.CommandContext(ctx, "go", "version").CombinedOutput() if err != nil { return bytes.Buffer{}, err } diff --git a/pipeline/sign/sign.go b/pipeline/sign/sign.go index 18e641e80..a56251557 100644 --- a/pipeline/sign/sign.go +++ b/pipeline/sign/sign.go @@ -92,7 +92,7 @@ func signone(ctx *context.Context, artifact artifact.Artifact) (string, error) { // However, this works as intended. The nosec annotation // tells the scanner to ignore this. // #nosec - cmd := exec.Command(cfg.Cmd, args...) + cmd := exec.CommandContext(ctx, cfg.Cmd, args...) output, err := cmd.CombinedOutput() if err != nil { return "", fmt.Errorf("sign: %s failed with %q", cfg.Cmd, string(output)) diff --git a/pipeline/snapcraft/snapcraft.go b/pipeline/snapcraft/snapcraft.go index ae0bec32a..45663d633 100644 --- a/pipeline/snapcraft/snapcraft.go +++ b/pipeline/snapcraft/snapcraft.go @@ -163,7 +163,7 @@ func create(ctx *context.Context, arch string, binaries []artifact.Artifact) err var snap = filepath.Join(ctx.Config.Dist, folder+".snap") /* #nosec */ - var cmd = exec.Command("snapcraft", "snap", primeDir, "--output", snap) + var cmd = exec.CommandContext(ctx, "snapcraft", "snap", primeDir, "--output", snap) if out, err = cmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to generate snap package: %s", string(out)) } diff --git a/pipeline/snapcraft/snapcraft_test.go b/pipeline/snapcraft/snapcraft_test.go index 6c56f46ca..c104c329b 100644 --- a/pipeline/snapcraft/snapcraft_test.go +++ b/pipeline/snapcraft/snapcraft_test.go @@ -54,8 +54,8 @@ func TestRunPipe(t *testing.T) { Dist: dist, Snapcraft: config.Snapcraft{ NameTemplate: "foo_{{.Arch}}", - Summary: "test summary", - Description: "test description", + Summary: "test summary", + Description: "test description", }, }, } @@ -77,8 +77,8 @@ func TestRunPipeInvalidNameTemplate(t *testing.T) { Dist: dist, Snapcraft: config.Snapcraft{ NameTemplate: "foo_{{.Arch}", - Summary: "test summary", - Description: "test description", + Summary: "test summary", + Description: "test description", }, }, } @@ -100,9 +100,9 @@ func TestRunPipeWithName(t *testing.T) { Dist: dist, Snapcraft: config.Snapcraft{ NameTemplate: "foo_{{.Arch}}", - Name: "testsnapname", - Summary: "test summary", - Description: "test description", + Name: "testsnapname", + Summary: "test summary", + Description: "test description", }, }, } @@ -130,8 +130,8 @@ func TestRunPipeWithPlugsAndDaemon(t *testing.T) { Dist: dist, Snapcraft: config.Snapcraft{ NameTemplate: "foo_{{.Arch}}", - Summary: "test summary", - Description: "test description", + Summary: "test summary", + Description: "test description", Apps: map[string]config.SnapcraftAppMetadata{ "mybin": { Plugs: []string{"home", "network"}, @@ -171,7 +171,7 @@ func TestNoSnapcraftInPath(t *testing.T) { func TestDefault(t *testing.T) { var ctx = context.New(config.Project{}) - assert.NoError(t,Pipe{}.Default(ctx)) + assert.NoError(t, Pipe{}.Default(ctx)) assert.Equal(t, defaultNameTemplate, ctx.Config.Snapcraft.NameTemplate) } @@ -181,7 +181,7 @@ func TestDefaultSet(t *testing.T) { NameTemplate: "foo", }, }) - assert.NoError(t,Pipe{}.Default(ctx)) + assert.NoError(t, Pipe{}.Default(ctx)) assert.Equal(t, "foo", ctx.Config.Snapcraft.NameTemplate) } From 1d0911ba1f4904b918b062a6604cff721720f6c0 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 29 Dec 2017 17:12:25 -0200 Subject: [PATCH 02/15] style: improved output when canceled --- goreleaserlib/goreleaser.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/goreleaserlib/goreleaser.go b/goreleaserlib/goreleaser.go index bdb9b9402..4d7be7b01 100644 --- a/goreleaserlib/goreleaser.go +++ b/goreleaserlib/goreleaser.go @@ -132,9 +132,7 @@ func Release(flags Flags) error { case <-ctx.Done(): return ctx.Err() case sig := <-signals: - restoreOutputPadding() - log.Infof("stopping: %s", sig) - return nil + return fmt.Errorf("canceled due to %s", sig) } } From 6b0e6686a5b6d64ea344c331bbd3fdfbbcfe458e Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 29 Dec 2017 17:35:22 -0200 Subject: [PATCH 03/15] test: fixed tests --- goreleaserlib/goreleaser_test.go | 15 +++++++++++ pipeline/build/build_test.go | 14 ++++++++-- pipeline/docker/docker_test.go | 45 ++++++++++++++++---------------- 3 files changed, 50 insertions(+), 24 deletions(-) diff --git a/goreleaserlib/goreleaser_test.go b/goreleaserlib/goreleaser_test.go index 1f3ca29c1..0f0a142a7 100644 --- a/goreleaserlib/goreleaser_test.go +++ b/goreleaserlib/goreleaser_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strconv" "testing" + "time" "github.com/goreleaser/goreleaser/config" "github.com/goreleaser/goreleaser/internal/testlib" @@ -21,6 +22,7 @@ func TestRelease(t *testing.T) { _, back := setup(t) defer back() var flags = fakeFlags{ + t: t, flags: map[string]string{ "skip-publish": "true", "skip-validate": "true", @@ -35,6 +37,7 @@ func TestSnapshotRelease(t *testing.T) { _, back := setup(t) defer back() var flags = fakeFlags{ + t: t, flags: map[string]string{ "snapshot": "true", "parallelism": "4", @@ -45,6 +48,7 @@ func TestSnapshotRelease(t *testing.T) { func TestConfigFileIsSetAndDontExist(t *testing.T) { var flags = fakeFlags{ + t: t, flags: map[string]string{ "config": "/this/wont/exist", }, @@ -70,6 +74,7 @@ func TestConfigFlagNotSetButExists(t *testing.T) { ), ) var flags = fakeFlags{ + t: t, flags: map[string]string{}, } assert.Equal(t, name, getConfigFile(flags)) @@ -79,6 +84,7 @@ func TestConfigFlagNotSetButExists(t *testing.T) { func TestReleaseNotesFileDontExist(t *testing.T) { var flags = fakeFlags{ + t: t, flags: map[string]string{ "release-notes": "/this/also/wont/exist", }, @@ -92,6 +98,7 @@ func TestCustomReleaseNotesFile(t *testing.T) { var releaseNotes = filepath.Join(folder, "notes.md") createFile(t, releaseNotes, "nothing important at all") var flags = fakeFlags{ + t: t, flags: map[string]string{ "release-notes": releaseNotes, "skip-publish": "true", @@ -107,6 +114,7 @@ func TestBrokenPipe(t *testing.T) { defer back() createFile(t, "main.go", "not a valid go file") var flags = fakeFlags{ + t: t, flags: map[string]string{ "skip-publish": "true", "skip-validate": "true", @@ -149,6 +157,7 @@ func TestInitProjectDefaultPipeFails(t *testing.T) { // fakeFlags is a mock of the cli flags type fakeFlags struct { + t *testing.T flags map[string]string } @@ -169,6 +178,12 @@ func (f fakeFlags) Bool(s string) bool { return f.flags[s] == "true" } +func (f fakeFlags) Duration(s string) time.Duration { + result, err := time.ParseDuration(f.flags[s]) + assert.NoError(f.t, err) + return result +} + func setup(t *testing.T) (current string, back func()) { folder, err := ioutil.TempDir("", "goreleaser") assert.NoError(t, err) diff --git a/pipeline/build/build_test.go b/pipeline/build/build_test.go index adbbc4f0f..bc0e6e8ba 100644 --- a/pipeline/build/build_test.go +++ b/pipeline/build/build_test.go @@ -21,11 +21,21 @@ func TestPipeDescription(t *testing.T) { } func TestRun(t *testing.T) { - assert.NoError(t, run(buildtarget.Runtime, []string{"go", "list", "./..."}, emptyEnv)) + assert.NoError(t, run( + context.New(config.Project{}), + buildtarget.Runtime, + []string{"go", "list", "./..."}, + emptyEnv, + )) } func TestRunInvalidCommand(t *testing.T) { - assert.Error(t, run(buildtarget.Runtime, []string{"gggggo", "nope"}, emptyEnv)) + assert.Error(t, run( + context.New(config.Project{}), + buildtarget.Runtime, + []string{"gggggo", "nope"}, + emptyEnv, + )) } func TestBuild(t *testing.T) { diff --git a/pipeline/docker/docker_test.go b/pipeline/docker/docker_test.go index 997736ef7..3e51f1686 100644 --- a/pipeline/docker/docker_test.go +++ b/pipeline/docker/docker_test.go @@ -203,30 +203,28 @@ func TestRunPipe(t *testing.T) { for name, docker := range table { t.Run(name, func(tt *testing.T) { folder, err := ioutil.TempDir("", "archivetest") - assert.NoError(t, err) + assert.NoError(tt, err) var dist = filepath.Join(folder, "dist") - assert.NoError(t, os.Mkdir(dist, 0755)) - assert.NoError(t, os.Mkdir(filepath.Join(dist, "mybin"), 0755)) + assert.NoError(tt, os.Mkdir(dist, 0755)) + assert.NoError(tt, os.Mkdir(filepath.Join(dist, "mybin"), 0755)) var binPath = filepath.Join(dist, "mybin", "mybin") _, err = os.Create(binPath) - assert.NoError(t, err) + assert.NoError(tt, err) - var ctx = &context.Context{ - Version: "1.0.0", - Publish: docker.publish, - Parallelism: 4, - Artifacts: artifact.New(), - Git: context.GitInfo{ - CurrentTag: "v1.0.0", + var ctx = context.New(config.Project{ + ProjectName: "mybin", + Dist: dist, + Dockers: []config.Docker{ + docker.docker, }, - Config: config.Project{ - ProjectName: "mybin", - Dist: dist, - Dockers: []config.Docker{ - docker.docker, - }, - }, - Env: map[string]string{"FOO": "123"}, + }) + ctx.Publish = true + ctx.Env = map[string]string{ + "FOO": "123", + } + ctx.Version = "1.0.0" + ctx.Git = context.GitInfo{ + CurrentTag: "v1.0.0", } for _, os := range []string{"linux", "darwin"} { for _, arch := range []string{"amd64", "386"} { @@ -252,14 +250,17 @@ func TestRunPipe(t *testing.T) { if docker.err == "" { assert.NoError(tt, err) } else { - assert.Contains(tt, err.Error(), docker.err) + assert.Error(tt, err) + if err != nil { + assert.Contains(tt, err.Error(), docker.err) + } } // this might should not fail as the image should have been created when // the step ran for _, img := range docker.expect { - t.Log("removing docker image", img) - assert.NoError(t, exec.Command("docker", "rmi", img).Run(), "could not delete image %s", img) + tt.Log("removing docker image", img) + assert.NoError(tt, exec.Command("docker", "rmi", img).Run(), "could not delete image %s", img) } }) From a751da59092bfe87794b47e346cbb874a29bb879 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 29 Dec 2017 17:51:02 -0200 Subject: [PATCH 04/15] test: fixed tests --- goreleaserlib/goreleaser_test.go | 14 ++++++-- pipeline/fpm/fpm_test.go | 55 +++++++++++++------------------- 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/goreleaserlib/goreleaser_test.go b/goreleaserlib/goreleaser_test.go index 0f0a142a7..98d56f516 100644 --- a/goreleaserlib/goreleaser_test.go +++ b/goreleaserlib/goreleaser_test.go @@ -24,6 +24,7 @@ func TestRelease(t *testing.T) { var flags = fakeFlags{ t: t, flags: map[string]string{ + "timeout": "1m", "skip-publish": "true", "skip-validate": "true", "debug": "true", @@ -39,6 +40,7 @@ func TestSnapshotRelease(t *testing.T) { var flags = fakeFlags{ t: t, flags: map[string]string{ + "timeout": "1m", "snapshot": "true", "parallelism": "4", }, @@ -50,7 +52,8 @@ func TestConfigFileIsSetAndDontExist(t *testing.T) { var flags = fakeFlags{ t: t, flags: map[string]string{ - "config": "/this/wont/exist", + "timeout": "1m", + "config": "/this/wont/exist", }, } assert.Error(t, Release(flags)) @@ -74,8 +77,10 @@ func TestConfigFlagNotSetButExists(t *testing.T) { ), ) var flags = fakeFlags{ - t: t, - flags: map[string]string{}, + t: t, + flags: map[string]string{ + "timeout": "1m", + }, } assert.Equal(t, name, getConfigFile(flags)) }) @@ -86,6 +91,7 @@ func TestReleaseNotesFileDontExist(t *testing.T) { var flags = fakeFlags{ t: t, flags: map[string]string{ + "timeout": "1m", "release-notes": "/this/also/wont/exist", }, } @@ -100,6 +106,7 @@ func TestCustomReleaseNotesFile(t *testing.T) { var flags = fakeFlags{ t: t, flags: map[string]string{ + "timeout": "1m", "release-notes": releaseNotes, "skip-publish": "true", "skip-validate": "true", @@ -116,6 +123,7 @@ func TestBrokenPipe(t *testing.T) { var flags = fakeFlags{ t: t, flags: map[string]string{ + "timeout": "1m", "skip-publish": "true", "skip-validate": "true", "parallelism": "4", diff --git a/pipeline/fpm/fpm_test.go b/pipeline/fpm/fpm_test.go index 5d2dbb7b9..263aa18c7 100644 --- a/pipeline/fpm/fpm_test.go +++ b/pipeline/fpm/fpm_test.go @@ -36,27 +36,22 @@ func TestRunPipe(t *testing.T) { var binPath = filepath.Join(dist, "mybin", "mybin") _, err = os.Create(binPath) assert.NoError(t, err) - var ctx = &context.Context{ - Version: "1.0.0", - Parallelism: runtime.NumCPU(), - Debug: true, - Artifacts: artifact.New(), - Config: config.Project{ - ProjectName: "mybin", - Dist: dist, - FPM: config.FPM{ - NameTemplate: defaultNameTemplate, - Formats: []string{"deb", "rpm"}, - Dependencies: []string{"make"}, - Conflicts: []string{"git"}, - Description: "Some description", - License: "MIT", - Maintainer: "me@me", - Vendor: "asdf", - Homepage: "https://goreleaser.github.io", - }, + var ctx = context.New(config.Project{ + ProjectName: "mybin", + Dist: dist, + FPM: config.FPM{ + NameTemplate: defaultNameTemplate, + Formats: []string{"deb", "rpm"}, + Dependencies: []string{"make"}, + Conflicts: []string{"git"}, + Description: "Some description", + License: "MIT", + Maintainer: "me@me", + Vendor: "asdf", + Homepage: "https://goreleaser.github.io", }, - } + }) + ctx.Version = "1.0.0" for _, goos := range []string{"linux", "darwin"} { for _, goarch := range []string{"amd64", "386"} { ctx.Artifacts.Add(artifact.Artifact{ @@ -115,20 +110,16 @@ func TestCreateFileDoesntExist(t *testing.T) { var dist = filepath.Join(folder, "dist") assert.NoError(t, os.Mkdir(dist, 0755)) assert.NoError(t, os.Mkdir(filepath.Join(dist, "mybin"), 0755)) - var ctx = &context.Context{ - Version: "1.0.0", - Parallelism: runtime.NumCPU(), - Artifacts: artifact.New(), - Config: config.Project{ - Dist: dist, - FPM: config.FPM{ - Formats: []string{"deb", "rpm"}, - Files: map[string]string{ - "testdata/testfile.txt": "/var/lib/test/testfile.txt", - }, + var ctx = context.New(config.Project{ + Dist: dist, + FPM: config.FPM{ + Formats: []string{"deb", "rpm"}, + Files: map[string]string{ + "testdata/testfile.txt": "/var/lib/test/testfile.txt", }, }, - } + }) + ctx.Version = "1.0.0" ctx.Artifacts.Add(artifact.Artifact{ Name: "mybin", Path: filepath.Join(dist, "mybin", "mybin"), From 12321ec0e845c109c9d1ee3861da2b5385e17148 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 29 Dec 2017 20:19:55 -0200 Subject: [PATCH 05/15] test: fixed snap tests --- pipeline/snapcraft/snapcraft_test.go | 106 ++++++++++++--------------- 1 file changed, 46 insertions(+), 60 deletions(-) diff --git a/pipeline/snapcraft/snapcraft_test.go b/pipeline/snapcraft/snapcraft_test.go index c104c329b..5a31c5698 100644 --- a/pipeline/snapcraft/snapcraft_test.go +++ b/pipeline/snapcraft/snapcraft_test.go @@ -46,19 +46,16 @@ func TestRunPipe(t *testing.T) { var dist = filepath.Join(folder, "dist") assert.NoError(t, os.Mkdir(dist, 0755)) assert.NoError(t, err) - var ctx = &context.Context{ - Version: "testversion", - Artifacts: artifact.New(), - Config: config.Project{ - ProjectName: "mybin", - Dist: dist, - Snapcraft: config.Snapcraft{ - NameTemplate: "foo_{{.Arch}}", - Summary: "test summary", - Description: "test description", - }, + var ctx = context.New(config.Project{ + ProjectName: "mybin", + Dist: dist, + Snapcraft: config.Snapcraft{ + NameTemplate: "foo_{{.Arch}}", + Summary: "test summary", + Description: "test description", }, - } + }) + ctx.Version = "testversion" addBinaries(t, ctx, "mybin", dist) assert.NoError(t, Pipe{}.Run(ctx)) } @@ -69,19 +66,16 @@ func TestRunPipeInvalidNameTemplate(t *testing.T) { var dist = filepath.Join(folder, "dist") assert.NoError(t, os.Mkdir(dist, 0755)) assert.NoError(t, err) - var ctx = &context.Context{ - Version: "testversion", - Artifacts: artifact.New(), - Config: config.Project{ - ProjectName: "mybin", - Dist: dist, - Snapcraft: config.Snapcraft{ - NameTemplate: "foo_{{.Arch}", - Summary: "test summary", - Description: "test description", - }, + var ctx = context.New(config.Project{ + ProjectName: "mybin", + Dist: dist, + Snapcraft: config.Snapcraft{ + NameTemplate: "foo_{{.Arch}", + Summary: "test summary", + Description: "test description", }, - } + }) + ctx.Version = "testversion" addBinaries(t, ctx, "mybin", dist) assert.EqualError(t, Pipe{}.Run(ctx), `template: foo_{{.Arch}:1: unexpected "}" in operand`) } @@ -92,20 +86,17 @@ func TestRunPipeWithName(t *testing.T) { var dist = filepath.Join(folder, "dist") assert.NoError(t, os.Mkdir(dist, 0755)) assert.NoError(t, err) - var ctx = &context.Context{ - Version: "testversion", - Artifacts: artifact.New(), - Config: config.Project{ - ProjectName: "testprojectname", - Dist: dist, - Snapcraft: config.Snapcraft{ - NameTemplate: "foo_{{.Arch}}", - Name: "testsnapname", - Summary: "test summary", - Description: "test description", - }, + var ctx = context.New(config.Project{ + ProjectName: "testprojectname", + Dist: dist, + Snapcraft: config.Snapcraft{ + NameTemplate: "foo_{{.Arch}}", + Name: "testsnapname", + Summary: "test summary", + Description: "test description", }, - } + }) + ctx.Version = "testversion" addBinaries(t, ctx, "testprojectname", dist) assert.NoError(t, Pipe{}.Run(ctx)) yamlFile, err := ioutil.ReadFile(filepath.Join(dist, "foo_amd64", "prime", "meta", "snap.yaml")) @@ -122,25 +113,22 @@ func TestRunPipeWithPlugsAndDaemon(t *testing.T) { var dist = filepath.Join(folder, "dist") assert.NoError(t, os.Mkdir(dist, 0755)) assert.NoError(t, err) - var ctx = &context.Context{ - Version: "testversion", - Artifacts: artifact.New(), - Config: config.Project{ - ProjectName: "mybin", - Dist: dist, - Snapcraft: config.Snapcraft{ - NameTemplate: "foo_{{.Arch}}", - Summary: "test summary", - Description: "test description", - Apps: map[string]config.SnapcraftAppMetadata{ - "mybin": { - Plugs: []string{"home", "network"}, - Daemon: "simple", - }, + var ctx = context.New(config.Project{ + ProjectName: "mybin", + Dist: dist, + Snapcraft: config.Snapcraft{ + NameTemplate: "foo_{{.Arch}}", + Summary: "test summary", + Description: "test description", + Apps: map[string]config.SnapcraftAppMetadata{ + "mybin": { + Plugs: []string{"home", "network"}, + Daemon: "simple", }, }, }, - } + }) + ctx.Version = "testversion" addBinaries(t, ctx, "mybin", dist) assert.NoError(t, Pipe{}.Run(ctx)) yamlFile, err := ioutil.ReadFile(filepath.Join(dist, "foo_amd64", "prime", "meta", "snap.yaml")) @@ -158,14 +146,12 @@ func TestNoSnapcraftInPath(t *testing.T) { assert.NoError(t, os.Setenv("PATH", path)) }() assert.NoError(t, os.Setenv("PATH", "")) - var ctx = &context.Context{ - Config: config.Project{ - Snapcraft: config.Snapcraft{ - Summary: "dummy", - Description: "dummy", - }, + var ctx = context.New(config.Project{ + Snapcraft: config.Snapcraft{ + Summary: "dummy", + Description: "dummy", }, - } + }) assert.EqualError(t, Pipe{}.Run(ctx), ErrNoSnapcraft.Error()) } From 8e94fa79751e1d3e0826aac91392d00f764621ca Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 29 Dec 2017 20:34:09 -0200 Subject: [PATCH 06/15] test: fixed release tests --- pipeline/release/body_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pipeline/release/body_test.go b/pipeline/release/body_test.go index 5a28fa370..3855b7cae 100644 --- a/pipeline/release/body_test.go +++ b/pipeline/release/body_test.go @@ -60,9 +60,9 @@ func TestDescribeBodyNoDockerImagesNoBrews(t *testing.T) { func TestDontEscapeHTML(t *testing.T) { var changelog = "

test

" - var ctx = &context.Context{ - ReleaseNotes: changelog, - } + var ctx = context.New(config.Project{}) + ctx.ReleaseNotes = changelog + out, err := describeBody(ctx) assert.NoError(t, err) assert.Contains(t, out.String(), changelog) From bfa79275bc90e6cc07832dce9f9dedd8da903cae Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 29 Dec 2017 22:19:33 -0200 Subject: [PATCH 07/15] fix: buffering channels --- goreleaserlib/goreleaser.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/goreleaserlib/goreleaser.go b/goreleaserlib/goreleaser.go index 4d7be7b01..df74f76d9 100644 --- a/goreleaserlib/goreleaser.go +++ b/goreleaserlib/goreleaser.go @@ -110,7 +110,11 @@ func Release(flags Flags) error { ctx.Publish = false } ctx.RmDist = flags.Bool("rm-dist") - var errs = make(chan error) + return doRelease(ctx) +} + +func doRelease(ctx *context.Context) error { + var errs = make(chan error, 1) go func() { for _, pipe := range pipes { restoreOutputPadding() @@ -124,7 +128,7 @@ func Release(flags Flags) error { errs <- nil }() defer restoreOutputPadding() - var signals = make(chan os.Signal) + var signals = make(chan os.Signal, 1) signal.Notify(signals, os.Interrupt, syscall.SIGTERM) select { case err := <-errs: From f422e7734de911f0fc8a6d6e95192848c317ad7c Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 29 Dec 2017 23:01:17 -0200 Subject: [PATCH 08/15] test: cover context with timeout --- context/context_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/context/context_test.go b/context/context_test.go index 50d298dac..1ebc71dd1 100644 --- a/context/context_test.go +++ b/context/context_test.go @@ -2,6 +2,7 @@ package context import ( "testing" + "time" "github.com/goreleaser/goreleaser/config" "github.com/stretchr/testify/assert" @@ -12,3 +13,12 @@ func TestNew(t *testing.T) { assert.NotEmpty(t, ctx.Env) assert.Equal(t, 4, ctx.Parallelism) } + +func TestNewWithTimeout(t *testing.T) { + ctx, cancel := NewWithTimeout(config.Project{}, time.Second) + assert.NotEmpty(t, ctx.Env) + assert.Equal(t, 4, ctx.Parallelism) + cancel() + <-ctx.Done() + assert.EqualError(t, ctx.Err(), `context canceled`) +} From ed14fe7b996af7c72de8b697ccc012353de2924b Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Sat, 30 Dec 2017 00:55:44 -0200 Subject: [PATCH 09/15] fix: move code to new pkg so we can better cover it with tests --- goreleaserlib/goreleaser.go | 25 ++------ goreleaserlib/goreleaser_test.go | 104 ++++++++++++------------------- internal/handler/handler.go | 53 ++++++++++++++++ internal/handler/handler_test.go | 57 +++++++++++++++++ 4 files changed, 156 insertions(+), 83 deletions(-) create mode 100644 internal/handler/handler.go create mode 100644 internal/handler/handler_test.go diff --git a/goreleaserlib/goreleaser.go b/goreleaserlib/goreleaser.go index df74f76d9..f58145877 100644 --- a/goreleaserlib/goreleaser.go +++ b/goreleaserlib/goreleaser.go @@ -4,9 +4,7 @@ import ( "fmt" "io/ioutil" "os" - "os/signal" "strings" - "syscall" "time" "github.com/apex/log" @@ -15,6 +13,7 @@ import ( "github.com/goreleaser/goreleaser/config" "github.com/goreleaser/goreleaser/context" + "github.com/goreleaser/goreleaser/internal/handler" "github.com/goreleaser/goreleaser/pipeline" "github.com/goreleaser/goreleaser/pipeline/archive" "github.com/goreleaser/goreleaser/pipeline/artifactory" @@ -114,30 +113,18 @@ func Release(flags Flags) error { } func doRelease(ctx *context.Context) error { - var errs = make(chan error, 1) - go func() { + defer restoreOutputPadding() + return handler.New().Run(ctx, func() error { for _, pipe := range pipes { restoreOutputPadding() log.Infof("\033[1m%s\033[0m", strings.ToUpper(pipe.String())) cli.Default.Padding = increasedPadding if err := handle(pipe.Run(ctx)); err != nil { - errs <- err - return + return err } } - errs <- nil - }() - defer restoreOutputPadding() - var signals = make(chan os.Signal, 1) - signal.Notify(signals, os.Interrupt, syscall.SIGTERM) - select { - case err := <-errs: - return err - case <-ctx.Done(): - return ctx.Err() - case sig := <-signals: - return fmt.Errorf("canceled due to %s", sig) - } + return nil + }) } func restoreOutputPadding() { diff --git a/goreleaserlib/goreleaser_test.go b/goreleaserlib/goreleaser_test.go index 98d56f516..a0e242f50 100644 --- a/goreleaserlib/goreleaser_test.go +++ b/goreleaserlib/goreleaser_test.go @@ -21,42 +21,29 @@ func init() { func TestRelease(t *testing.T) { _, back := setup(t) defer back() - var flags = fakeFlags{ - t: t, - flags: map[string]string{ - "timeout": "1m", - "skip-publish": "true", - "skip-validate": "true", - "debug": "true", - "parallelism": "4", - }, - } - assert.NoError(t, Release(flags)) + assert.NoError(t, Release(newFlags(t, testParams()))) +} + +func TestReleaseTimeout(t *testing.T) { + _, back := setup(t) + defer back() + params := testParams() + params["timeout"] = "1s" + assert.EqualError(t, Release(newFlags(t, params)), `context deadline exceeded`) } func TestSnapshotRelease(t *testing.T) { _, back := setup(t) defer back() - var flags = fakeFlags{ - t: t, - flags: map[string]string{ - "timeout": "1m", - "snapshot": "true", - "parallelism": "4", - }, - } - assert.NoError(t, Release(flags)) + params := testParams() + params["snapshot"] = "true" + assert.NoError(t, Release(newFlags(t, params))) } func TestConfigFileIsSetAndDontExist(t *testing.T) { - var flags = fakeFlags{ - t: t, - flags: map[string]string{ - "timeout": "1m", - "config": "/this/wont/exist", - }, - } - assert.Error(t, Release(flags)) + params := testParams() + params["config"] = "/this/wont/exist" + assert.Error(t, Release(newFlags(t, params))) } func TestConfigFlagNotSetButExists(t *testing.T) { @@ -76,26 +63,15 @@ func TestConfigFlagNotSetButExists(t *testing.T) { filepath.Join(folder, name), ), ) - var flags = fakeFlags{ - t: t, - flags: map[string]string{ - "timeout": "1m", - }, - } - assert.Equal(t, name, getConfigFile(flags)) + assert.Equal(t, name, getConfigFile(newFlags(t, testParams()))) }) } } func TestReleaseNotesFileDontExist(t *testing.T) { - var flags = fakeFlags{ - t: t, - flags: map[string]string{ - "timeout": "1m", - "release-notes": "/this/also/wont/exist", - }, - } - assert.Error(t, Release(flags)) + params := testParams() + params["release-notes"] = "/this/also/wont/exist" + assert.Error(t, Release(newFlags(t, params))) } func TestCustomReleaseNotesFile(t *testing.T) { @@ -103,33 +79,16 @@ func TestCustomReleaseNotesFile(t *testing.T) { defer back() var releaseNotes = filepath.Join(folder, "notes.md") createFile(t, releaseNotes, "nothing important at all") - var flags = fakeFlags{ - t: t, - flags: map[string]string{ - "timeout": "1m", - "release-notes": releaseNotes, - "skip-publish": "true", - "skip-validate": "true", - "parallelism": "4", - }, - } - assert.NoError(t, Release(flags)) + var params = testParams() + params["release-notes"] = releaseNotes + assert.NoError(t, Release(newFlags(t, params))) } func TestBrokenPipe(t *testing.T) { _, back := setup(t) defer back() createFile(t, "main.go", "not a valid go file") - var flags = fakeFlags{ - t: t, - flags: map[string]string{ - "timeout": "1m", - "skip-publish": "true", - "skip-validate": "true", - "parallelism": "4", - }, - } - assert.Error(t, Release(flags)) + assert.Error(t, Release(newFlags(t, testParams()))) } func TestInitProject(t *testing.T) { @@ -169,6 +128,13 @@ type fakeFlags struct { flags map[string]string } +func newFlags(t *testing.T, params map[string]string) Flags { + return fakeFlags{ + t: t, + flags: params, + } +} + func (f fakeFlags) IsSet(s string) bool { return f.flags[s] != "" } @@ -192,6 +158,16 @@ func (f fakeFlags) Duration(s string) time.Duration { return result } +func testParams() map[string]string { + return map[string]string{ + "debug": "true", + "parallelism": "4", + "skip-publish": "true", + "skip-validate": "true", + "timeout": "1m", + } +} + func setup(t *testing.T) (current string, back func()) { folder, err := ioutil.TempDir("", "goreleaser") assert.NoError(t, err) diff --git a/internal/handler/handler.go b/internal/handler/handler.go new file mode 100644 index 000000000..84a979a25 --- /dev/null +++ b/internal/handler/handler.go @@ -0,0 +1,53 @@ +// Package handler provides a way of having a task that is context-aware and +// that also deals with interrup and term signals. +// It was externalized mostly because it would be easier to test it this way. +// The name is not ideal but I couldn't think in a better one. +package handler + +import ( + "context" + "fmt" + "os" + "os/signal" + "syscall" +) + +// Task is function that can be executed by a Handler +type Task func() error + +// Handler is the task handler +type Handler struct { + signals chan os.Signal + errs chan error +} + +// New returns a new handler with its internals setup. +func New() *Handler { + return &Handler{ + signals: make(chan os.Signal, 1), + errs: make(chan error, 1), + } +} + +// Run executes a given task with a given context, dealing with its timeouts, +// cancels and SIGTERM and SIGINT signals. +// It will return an error if the context is canceled, if deadline exceeds, +// if a SIGTERM or SIGINT is received and of course if the task itself fails. +func (h *Handler) Run(ctx context.Context, task Task) error { + go func() { + if err := task(); err != nil { + h.errs <- err + return + } + h.errs <- nil + }() + signal.Notify(h.signals, syscall.SIGINT, syscall.SIGTERM) + select { + case err := <-h.errs: + return err + case <-ctx.Done(): + return ctx.Err() + case sig := <-h.signals: + return fmt.Errorf("received: %s", sig) + } +} diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go new file mode 100644 index 000000000..6099921df --- /dev/null +++ b/internal/handler/handler_test.go @@ -0,0 +1,57 @@ +package handler + +import ( + "context" + "errors" + "fmt" + "os" + "syscall" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestHandlerOK(t *testing.T) { + assert.NoError(t, New().Run(context.Background(), func() error { + return nil + })) +} + +func TestHandlerErrors(t *testing.T) { + var err = errors.New("some error") + assert.EqualError(t, New().Run(context.Background(), func() error { + return err + }), err.Error()) +} + +func TestHandlerTimeout(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond) + defer cancel() + assert.EqualError(t, New().Run(ctx, func() error { + t.Log("slow task...") + time.Sleep(time.Minute) + return nil + }), "context deadline exceeded") +} + +func TestHandlerSignals(t *testing.T) { + for _, signal := range []os.Signal{syscall.SIGINT, syscall.SIGTERM} { + signal := signal + t.Run(signal.String(), func(tt *testing.T) { + tt.Parallel() + var h = New() + var errs = make(chan error, 1) + go func() { + errs <- h.Run(context.Background(), func() error { + tt.Log("slow task...") + time.Sleep(time.Minute) + return nil + }) + }() + h.signals <- signal + assert.EqualError(tt, <-errs, fmt.Sprintf("received: %s", signal)) + }) + + } +} From e07caf3718134641c350ac6111843c1d16a09959 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Sat, 30 Dec 2017 00:58:17 -0200 Subject: [PATCH 10/15] style: rm empty line --- internal/handler/handler_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go index 6099921df..97fde652c 100644 --- a/internal/handler/handler_test.go +++ b/internal/handler/handler_test.go @@ -52,6 +52,5 @@ func TestHandlerSignals(t *testing.T) { h.signals <- signal assert.EqualError(tt, <-errs, fmt.Sprintf("received: %s", signal)) }) - } } From 686f5d2cd218e1c8bc55575af4e6861404ce292c Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Sat, 30 Dec 2017 01:05:39 -0200 Subject: [PATCH 11/15] test: added benchmark as well --- internal/handler/handler_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go index 97fde652c..0f8c8361f 100644 --- a/internal/handler/handler_test.go +++ b/internal/handler/handler_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "golang.org/x/sync/errgroup" ) func TestHandlerOK(t *testing.T) { @@ -54,3 +55,18 @@ func TestHandlerSignals(t *testing.T) { }) } } + +func BenchmarkHandler(b *testing.B) { + var task Task = func() error { + return nil + } + var h = New() + var ctx = context.Background() + var wg errgroup.Group + for i := 0; i < 10000; i++ { + wg.Go(func() error { + return h.Run(ctx, task) + }) + } + assert.NoError(b, wg.Wait()) +} From f537091757112223495c89eb569654454a2aac92 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Sat, 30 Dec 2017 09:59:08 -0200 Subject: [PATCH 12/15] test: removed useless test --- goreleaserlib/goreleaser_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/goreleaserlib/goreleaser_test.go b/goreleaserlib/goreleaser_test.go index a0e242f50..08153f381 100644 --- a/goreleaserlib/goreleaser_test.go +++ b/goreleaserlib/goreleaser_test.go @@ -24,14 +24,6 @@ func TestRelease(t *testing.T) { assert.NoError(t, Release(newFlags(t, testParams()))) } -func TestReleaseTimeout(t *testing.T) { - _, back := setup(t) - defer back() - params := testParams() - params["timeout"] = "1s" - assert.EqualError(t, Release(newFlags(t, params)), `context deadline exceeded`) -} - func TestSnapshotRelease(t *testing.T) { _, back := setup(t) defer back() From 60eb9d5f22fa6ef5beb0b20967078d0e4bf29292 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Sat, 30 Dec 2017 10:16:54 -0200 Subject: [PATCH 13/15] style: simplified code --- internal/handler/handler.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 84a979a25..c16d42a5a 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -35,11 +35,7 @@ func New() *Handler { // if a SIGTERM or SIGINT is received and of course if the task itself fails. func (h *Handler) Run(ctx context.Context, task Task) error { go func() { - if err := task(); err != nil { - h.errs <- err - return - } - h.errs <- nil + h.errs <- task() }() signal.Notify(h.signals, syscall.SIGINT, syscall.SIGTERM) select { From 6a9da7881306b0b99b654a78d90b2989dee9e006 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Tue, 2 Jan 2018 12:47:54 -0200 Subject: [PATCH 14/15] fix: using lib --- Gopkg.lock | 48 +++++++++++++++++---- Gopkg.toml | 4 ++ goreleaserlib/goreleaser.go | 4 +- internal/handler/handler.go | 49 ---------------------- internal/handler/handler_test.go | 72 -------------------------------- 5 files changed, 45 insertions(+), 132 deletions(-) delete mode 100644 internal/handler/handler.go delete mode 100644 internal/handler/handler_test.go diff --git a/Gopkg.lock b/Gopkg.lock index 55c2ff749..f9594a55b 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -2,10 +2,19 @@ [[projects]] + branch = "master" name = "github.com/apex/log" - packages = [".","handlers/cli"] - revision = "0296d6eb16bb28f8a0c55668affcf4876dc269be" - version = "v1.0.0" + packages = [ + ".", + "handlers/cli" + ] + revision = "bf81de4b2280680c3ccdf5520047f055cdafc7e0" + +[[projects]] + branch = "master" + name = "github.com/caarlos0/ctrlc" + packages = ["."] + revision = "6fe1e9620de6169f9b7a2caacb96598dd48315ce" [[projects]] name = "github.com/davecgh/go-spew" @@ -33,14 +42,21 @@ [[projects]] name = "github.com/goreleaser/archive" - packages = [".","tar","zip"] + packages = [ + ".", + "tar", + "zip" + ] revision = "caa5f3f5742eb0535631e94fa5e171c74c0144b7" version = "v1.0.0" [[projects]] branch = "master" name = "github.com/mattn/go-zglob" - packages = [".","fastwalk"] + packages = [ + ".", + "fastwalk" + ] revision = "4ecb59231939b2e499b1f2fd8f075565977d2452" [[projects]] @@ -70,13 +86,19 @@ [[projects]] branch = "master" name = "golang.org/x/net" - packages = ["context","context/ctxhttp"] + packages = [ + "context", + "context/ctxhttp" + ] revision = "cd69bc3fc700721b709c3a59e16e24c67b58f6ff" [[projects]] branch = "master" name = "golang.org/x/oauth2" - packages = [".","internal"] + packages = [ + ".", + "internal" + ] revision = "bb50c06baba3d0c76f9d125c0719093e315b5b44" [[projects]] @@ -87,7 +109,15 @@ [[projects]] name = "google.golang.org/appengine" - packages = ["internal","internal/base","internal/datastore","internal/log","internal/remote_api","internal/urlfetch","urlfetch"] + packages = [ + "internal", + "internal/base", + "internal/datastore", + "internal/log", + "internal/remote_api", + "internal/urlfetch", + "urlfetch" + ] revision = "150dc57a1b433e64154302bdc40b6bb8aefa313a" version = "v1.0.0" @@ -100,6 +130,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "963ab95030179004b910d862fc0048503002cb1029912dbe437ee08e87b31446" + inputs-digest = "a9900479518274dcc57ff328eacbb4c91f7f1e88c243d52aa792a9e6986f8de0" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index f76a52c0b..d71047f56 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -38,3 +38,7 @@ branch = "master" name = "github.com/apex/log" + +[[constraint]] + branch = "master" + name = "github.com/caarlos0/ctrlc" diff --git a/goreleaserlib/goreleaser.go b/goreleaserlib/goreleaser.go index f58145877..571b0fe5c 100644 --- a/goreleaserlib/goreleaser.go +++ b/goreleaserlib/goreleaser.go @@ -9,11 +9,11 @@ import ( "github.com/apex/log" "github.com/apex/log/handlers/cli" + "github.com/caarlos0/ctrlc" yaml "gopkg.in/yaml.v2" "github.com/goreleaser/goreleaser/config" "github.com/goreleaser/goreleaser/context" - "github.com/goreleaser/goreleaser/internal/handler" "github.com/goreleaser/goreleaser/pipeline" "github.com/goreleaser/goreleaser/pipeline/archive" "github.com/goreleaser/goreleaser/pipeline/artifactory" @@ -114,7 +114,7 @@ func Release(flags Flags) error { func doRelease(ctx *context.Context) error { defer restoreOutputPadding() - return handler.New().Run(ctx, func() error { + return ctrlc.Default.Run(ctx, func() error { for _, pipe := range pipes { restoreOutputPadding() log.Infof("\033[1m%s\033[0m", strings.ToUpper(pipe.String())) diff --git a/internal/handler/handler.go b/internal/handler/handler.go deleted file mode 100644 index c16d42a5a..000000000 --- a/internal/handler/handler.go +++ /dev/null @@ -1,49 +0,0 @@ -// Package handler provides a way of having a task that is context-aware and -// that also deals with interrup and term signals. -// It was externalized mostly because it would be easier to test it this way. -// The name is not ideal but I couldn't think in a better one. -package handler - -import ( - "context" - "fmt" - "os" - "os/signal" - "syscall" -) - -// Task is function that can be executed by a Handler -type Task func() error - -// Handler is the task handler -type Handler struct { - signals chan os.Signal - errs chan error -} - -// New returns a new handler with its internals setup. -func New() *Handler { - return &Handler{ - signals: make(chan os.Signal, 1), - errs: make(chan error, 1), - } -} - -// Run executes a given task with a given context, dealing with its timeouts, -// cancels and SIGTERM and SIGINT signals. -// It will return an error if the context is canceled, if deadline exceeds, -// if a SIGTERM or SIGINT is received and of course if the task itself fails. -func (h *Handler) Run(ctx context.Context, task Task) error { - go func() { - h.errs <- task() - }() - signal.Notify(h.signals, syscall.SIGINT, syscall.SIGTERM) - select { - case err := <-h.errs: - return err - case <-ctx.Done(): - return ctx.Err() - case sig := <-h.signals: - return fmt.Errorf("received: %s", sig) - } -} diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go deleted file mode 100644 index 0f8c8361f..000000000 --- a/internal/handler/handler_test.go +++ /dev/null @@ -1,72 +0,0 @@ -package handler - -import ( - "context" - "errors" - "fmt" - "os" - "syscall" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "golang.org/x/sync/errgroup" -) - -func TestHandlerOK(t *testing.T) { - assert.NoError(t, New().Run(context.Background(), func() error { - return nil - })) -} - -func TestHandlerErrors(t *testing.T) { - var err = errors.New("some error") - assert.EqualError(t, New().Run(context.Background(), func() error { - return err - }), err.Error()) -} - -func TestHandlerTimeout(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond) - defer cancel() - assert.EqualError(t, New().Run(ctx, func() error { - t.Log("slow task...") - time.Sleep(time.Minute) - return nil - }), "context deadline exceeded") -} - -func TestHandlerSignals(t *testing.T) { - for _, signal := range []os.Signal{syscall.SIGINT, syscall.SIGTERM} { - signal := signal - t.Run(signal.String(), func(tt *testing.T) { - tt.Parallel() - var h = New() - var errs = make(chan error, 1) - go func() { - errs <- h.Run(context.Background(), func() error { - tt.Log("slow task...") - time.Sleep(time.Minute) - return nil - }) - }() - h.signals <- signal - assert.EqualError(tt, <-errs, fmt.Sprintf("received: %s", signal)) - }) - } -} - -func BenchmarkHandler(b *testing.B) { - var task Task = func() error { - return nil - } - var h = New() - var ctx = context.Background() - var wg errgroup.Group - for i := 0; i < 10000; i++ { - wg.Go(func() error { - return h.Run(ctx, task) - }) - } - assert.NoError(b, wg.Wait()) -} From b0ef91c49c2fefd16c3329bcdd00e521d9f782cd Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Tue, 2 Jan 2018 12:51:40 -0200 Subject: [PATCH 15/15] fix: fixed version --- Gopkg.lock | 6 +++--- Gopkg.toml | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index f9594a55b..e36a3eb40 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -11,10 +11,10 @@ revision = "bf81de4b2280680c3ccdf5520047f055cdafc7e0" [[projects]] - branch = "master" name = "github.com/caarlos0/ctrlc" packages = ["."] - revision = "6fe1e9620de6169f9b7a2caacb96598dd48315ce" + revision = "70dc48d5d792f20f684a8f1d29bbac298f4b2ef4" + version = "v1.0.0" [[projects]] name = "github.com/davecgh/go-spew" @@ -130,6 +130,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "a9900479518274dcc57ff328eacbb4c91f7f1e88c243d52aa792a9e6986f8de0" + inputs-digest = "7c274a0e93e3a9ae90db19694e64c254f25763565b80c965de42dda3eacf77e8" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index d71047f56..6dcfe5315 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -40,5 +40,6 @@ [[constraint]] - branch = "master" name = "github.com/caarlos0/ctrlc" + version = "1.0.0" +